Add proper identity propagation to WebSockets

[stuartwdouglas]

Fixes #16602

Draft for now, as it needs quarkusio/quarkus-http#85 and a release of quarkus-http.

[sberyozkin]

Hi Stuart @stuartwdouglas It looks very good, thanks.

How often is BearerTokenClientEndpointConfigurator called ? I.e is it used only at the initial web sockets channel set up time to provide a token ? Or will it (also) be called every time a request is coming over an already established channel to emulate HTTP Bearer request ?
I'm trying to figure out if the token will be verified once and then be only reused to recreate the identity or it will be verified every time. If it will only be verified once then I think there should be some way to connect to (OIDC)IdentityProvider which will verify it...

[stuartwdouglas]

It's just a way of propagating a token to the client, basically the world's most roundabout addHeader call.

[sberyozkin]

@stuartwdouglas it sounds good :-) but what I don't get is how this token is verified, since OidcIdentityProvider would be activated when HttpAuthenticationMechanims are in action and here it is not HTTP... I must be missing something, sorry :-)

[stuartwdouglas]

The websocket handshake is HTTP, so the token is verified as part of the handshake.

[sberyozkin]

@stuartwdouglas Yes that bit I understand, thanks, but what happens when the web socket channel is on and the messages are flowing through it into Quarkus... The token is bound to it I believe, which is why SecurityIdentiy is still working but the token is not verified again ? (I was suggesting that in this case IdentityProvider mechanism should be activated as well by bypassing HttpAuthentocationMechanism)
For ex, OIDC would verify code flow id token on every request even after the authentication has happened as it is kept (by default) as a cookie.

[stuartwdouglas]

Why would it need to be verified again? In case the user was disabled while the websocket connection is open?

Refresh may be an issue if you want to propagate the token other servers.

[sberyozkin]

@stuartwdouglas

Why would it need to be verified again? In case the user was disabled while the websocket connection is open?

For the bearer access tokens, OidcIdentityProvider (and smallrye-jwt one) checks its expiry date, access tokens are usually short lived, default is 10 mins in Keycloak (may have changed recently, CC @pedroigor ) and can be lower in other providers so accepting it in say 20 mins would be a problem.
And the signature, it is wss but like with https there will always be at least a theoretical risk of someone on the client side changing a user name or other claims since a JWT payload is just encoded JSON, so a signature check protects against it.
I'd not be surprised if in both cases we got some security vulnerability issues opened without these checks.

Refresh may be an issue if you want to propagate the token other servers.

Good point as well - though for the bearer tokens it is not possible since we don't have a refresh token - but for the one returned alongside the code flow id token it would be possible - but this PR does not cover picking up that access token yet - it would be fine to do it later - CodeAuthenticationMechanism sends the security events, when the relogin based on a refresh happens, so we can probably update that event to include a refreshed access token with the WSS code replacing the old one, something like that

[stuartwdouglas]

That is not how websockets work though, they are only authenticated on open, there is no per message authentication.

[sberyozkin]

Hi Stuart @stuartwdouglas

That is not how websockets work though, they are only authenticated on open, there is no per message authentication.

True, which is why everyone does some workarounds to have a per message authentication working :-) (for example, how would it work for Basic auth.. etc), so it is great a bearer token binding will work in Quarkus out of the box.
I've been thinking for a while what can be done after your last comment.

One thing which I believe we can't really allow is to have @Inject SecurityIdentity identity; and have it backed up by a token which expired 30 mins/1 hour ago. IMHO the transport constraints should not affect the consumption of the security identity in Quarkus endpoints if we'll let the users write such a code.

I'd like to propose that an expiry check is done somewhere in the WSS code for the bearer token. For ex you can copy a simple code from OidcUtils which will give a Vert.x JsonObject representing a token payload and the exp claim value can be retrieved. If the token has expired then it is logged and an anonymous SecurityIdentity is used instead from now on. This should be easy to do and will ensure the token will not go stale.

Re the signature check, since it is wss: one can assume that it won't be intercepted. However - IMHO there should be a property, I'd say in the websockets namespace, something like quarkus.websockets.allow-identity-propagation (true by default) for the users be able to block the bearer token propagation from the HTTPS to WSS session for whatever reasons. For ex, if someone says, and what if someone bad will intercept the token then we'd say 1) it must be wss: and 2) if you'd like you can block the token propagation.

IMHO these 2 simple tools would be enough.
thanks

[devauxbr]

Hi,
Thanks for working on this matter! 🙏 For the record I think this MR should resolve #13565 and #16847 as well 🤞

[stuartwdouglas]

Per message authentication is not a thing, this is not like HTTP where you can have a different users for different messages. Once the session is established there is a 1:1 relationship of identity->connection. If you wanted to do per message authentication then you would need to add your own framing/headers mechanism, which is not something we can help with and just generally a bad idea.

This will also work fine for basic auth, the identity is created on connection establishment.

Regarding expiry this is only really an issue if you actually need to propagate the token, in which case we could have a setting to simply close the connection on token expiration (or ideally a bit before token expiration).

[sberyozkin]

@stuartwdouglas

Per message authentication is not a thing, this is not like HTTP where you can have a different users for different messages. Once the session is established there is a 1:1 relationship of identity->connection. If you wanted to do per message authentication then you would need to add your own framing/headers mechanism, which is not something we can help with and just generally a bad idea.

I understand trying to address all these technical problems can probably cause side-effects etc. But presenting a kind of valid Security Identity to the endpoint which is in fact no longer valid for the last hour is a major problem of its own. There is a code in this PR preparing a WebSocket principal - why can't the expiry be checked there and what is fundamentally wrong with it ?

Regarding expiry this is only really an issue if you actually need to propagate the token, in which case we could have a setting to simply close the connection on token expiration (or ideally a bit before token expiration).

What do you mean by propagate the token ? Is it the propagation from this endpoint to some remote target ? In this case the target will be validating it anyway, not a problem of this endpoint, we can't refresh it anyway if it is expired. Or do you mean something else ?

thanks

[sberyozkin]

Hi Stuart @stuartwdouglas I have another argument in favor of the expiry check (after a good coffee break :-) ):

• Expiry check is a fundamental token verification check. Good access tokens are always time scoped, the shorter the time to live, the better.
• The fact that typically Web sockets only authenticate during the upgrade from HTTP is irrelevant when it comes to expecting that a user code will only have access to a valid access token or indeed for such an access token to continue be used as a proof that the client is authorized to access this endpoint.
• If we don't do the expiry check then we'd send an indirect message to anyone who would be interested: so you have got a nearly expired token and you know you will fail accessing the endpoint over HTTP very soon; but don't worry, just transit to WSS with this token and continue providing it to Quarkus for as long as you want and IMHO it won't be sound and will inevitably lead to a vulnerability report.

Here you go, another try to convince you :-)

[stuartwdouglas]

Hi Stuart @stuartwdouglas I have another argument in favor of the expiry check (after a good coffee break :-) ):

* Expiry check is a fundamental token verification check. Good access tokens are always time scoped, the shorter the time to live, the better.

If the application cares about expiry then we could add an option to close the connection on expiry, as this is the only course of action that makes sense. That said I don't think this should be the default, as most people will not care. A websocket is different to HTTP, once the connection is established so is the identity, it is impossible to change the identity after the establishment.

The closed HTTP based analogy would be if you had an administrative task that took one minute to run, but your token expired in 30s. Should the server about the processing 30s into the request when the token expires?

* The fact that typically Web sockets only authenticate during the upgrade from HTTP is irrelevant when it comes to expecting that a user code will only have access to a `valid` access token or indeed for such an access token to continue be used as a proof that the client is authorized to access this endpoint.

It's not an endpoint, its an established authenticated session. If there is a requirement for a valid token (e.g. you are going to call another service using the token), then you will need to close the session and reconnect to the endpoint every time the token expires.

* If we don't do the expiry check then we'd send an indirect message to anyone who would be interested: so you have got a nearly expired token and you know you will fail accessing the endpoint over HTTP very soon; but don't worry, just transit to WSS with this token and continue providing it to Quarkus for as long as you want and IMHO it won't be sound and will inevitably lead to a vulnerability report.

That is not how websockets work, you can't 'just transition to WSS' and do the same things you would do over HTTP.

[sberyozkin]

Hi @stuartwdouglas

Let me start in the reverse order :-)

Sergey: If we don't do the expiry check then we'd send an indirect message to anyone who would be interested: so you have got a nearly expired token and you know you will fail accessing the endpoint over HTTP very soon; but don't worry, just transit to WSS with this token and continue providing it to Quarkus for as long as you want and IMHO it won't be sound and will inevitably lead to a vulnerability report.)
Stuart: That is not how websockets work, you can't 'just transition to WSS' and do the same things you would do over HTTP.

This is not what I implied, while I don't know all the mechanics I have a good enough idea of how WSS works :-) . My problem is about implicitly extending a lifetime of the time scoped credential without any control over it.

It's not an endpoint, its an established authenticated session.

During this session a user code will get access to a security identity which may have been already invalid, the longer the session lasts the longer it will be invalid for. I believe RBAC decisions will be made based on this security identity. It should work the same from Quarkus Security's point of view - access to the resource should fail if the token has expired, be it with HTTP or WSS, esp send (though it is probably simpler to apply it to the in an out).

If there is a requirement for a valid token (e.g. you are going to call another service using the token), then you will need to close the session and reconnect to the endpoint every time the token expires.

Token propagation from within a WSS session is a different and likely very advanced use case. We can support it with OidcClient in the future with auto-refreshing the token before it expires (would only be possible for a token acquired originally via the code flow as a refresh token will also be available - which is not the case with the bearer token).

If the application cares about expiry then we could add an option to close the connection on expiry, as this is the only course of action that makes sense.

+1, yes please :-), all I'm asking for is to let the users control what happens when the token has expired.

That said I don't think this should be the default, as most people will not care.

But someone has to be paranoid at the Quarkus end, and I don't mind to be the one, most people won't but by having a simple tool like an option you've proposed would let us support anyone who would be concerned to control the session lifetime and respond to possibly any, even if academic, vulnerability report. Non default as you suggest is fine for a start for sure.

A websocket is different to HTTP, once the connection is established so is the identity, it is impossible to change the identity after the establishment.

I understand, the problem is about making a possibly expired identity available. In theory at least, the user code can do some other side-effects with it as well without propagating the token but making some decisions based on the user name, its roles, etc. Also if we go totally paranoid, then we can assume the WSS session can be hijacked and the token modified - hence another earlier proposal to block the identity propagation via a property (instead of the signature check) - but I'm fine if we won't add it right now, the expiry check will most likely do.

The closed HTTP based analogy would be if you had an administrative task that took one minute to run, but your token expired in 30s. Should the server about the processing 30s into the request when the token expires ?

Good example, but I think the token is about verifying the token bearer can access the resource for whatever reasons (and start an admin task in the background if needed) - there is no requirement for the token to last for as long as the task itself runs. In general we can not control the misuse of the bearer token - taking your example, the user configures the background task with this token and this tasks keeps using this token long after it expired - but it is not something Quarkus can do anything about - as opposed to this case where it is what Quarkus controls. As I said the concern is about a possibly expired identity being visible to the endpoint code with the possible side-effects, so making it possible to control it would be great.

Thanks (for your patience :-) )

[stuartwdouglas]

@gsmet @geoand @cescoffier this will require a release of quarkus-http and AFAIK I don't have permission to release.

[geoand]

Interesting... I'll give it a shot and let you know.

[geoand]

I just released 4.1.2 so it should shortly be available in Central.

[stuartwdouglas]

Should be good to go now.

[quarkus-bot]

Failing Jobs - Building d6a421a

Status	Name	Step	Failures	Logs	Raw logs
⌛	Maven Tests - JDK 11 Windows	Build	⚠️ Check →	Logs	Raw logs

[sberyozkin]

@stuartwdouglas thanks :-)

[sberyozkin]

Given that Maven Tests - JDK 11 (pull_request) has passed, I think it is safe to assume Maven Tests - JDK 11 Windows (pull_request) build cancellation is not related