Add PKCE support ?

Though on adding PKCE support for dani-garcia#3899

Even for offline clients it's still recommended by Kanidm

As Kanidm aims for "secure by default" design, even with confidential clients, we deem it important to raise the bar for attackers. For example an attacker may have access to the client_id and client_secret of a confidential client as it was mishandled by a system administrator.

It's acknowledged that:

For confidential clients (refered to as a basic client in Kanidm due to the use of HTTP Basic for client_id+secret presentation) PKCE may optionally be disabled. This can allow authorisation code attacks to be carried out - however if TLS is used and the client_secret never leaks, then these attacks will not be possible.

On the nonce of Openid:

While the nonce can assist with certain attack mitigations, authorisation code interception is not prevented by the presence or validation of the nonce value.

When searching on nonce and pkce found this article from https://danielfett.de/2020/05/16/pkce-vs-nonce-equivalent-or-not/ which is quite detailed but discuss mainly CSRF which is not the attack discussed I believe.

The attacker would have the client_id and client_secret and the authorization code (And probably the nonce).
But without the verifier the SSO will not return the id_token when calling the exchange_code.
Note: even if the attacker did not have the nonce since the validation is done by the attacker it's no protection as opposed to the verifier sent to the SSO.

One could argue that if the attacker has access to all of those you probably have other way more important problems (probably something along the lines of reading all traffic post TLS termination, but in this case there is no use to doing the exchange himself ?)

But at the same time pkce challenge is supported by openidconnect and we are already storing the nonce in DB so adding the verifier alongside has almost no complexity/code and since we already retrieve the nonce before calling the exchange (using the state) we would easily have access to the verifier.

My conclusion ATM is to add it behind a config parameter in case some SSO do not support it correctly.
Still wondering on the default value of the parameter, maybe deactivated initially (Even if no obvious issues in https://github.com/ramosbugs/openidconnect-rs/issues?q=pkce ) and if people have no issues with it switching later on.

Some additional information:

OAuth 2.0 Security Best Current Practice recommend use of PKCE for authorization code flow even for confidential clients:

For confidential clients, the use of PKCE [RFC7636] is RECOMMENDED, as it provides a strong protection against misuse and injection of authorization codes as described in Section 4.5.3.1 and, as a side-effect, prevents CSRF even in presence of strong attackers as described in Section 4.7.1.

It also states that OIDC's nonce can be used instead if the client implements it correctly.

With additional precautions, described in Section 4.5.3.2, confidential OpenID Connect [OpenID.Core] clients MAY use the nonce parameter and the respective Claim in the ID Token instead.
Regarding whether to hide it behind a config parameter:

My conclusion ATM is to add it behind a config parameter in case some SSO do not support it correctly.

The draft also states:

Authorization servers MUST support PKCE [RFC7636].

Therefore, I think it makes sense to enable PKCE by default and not provide any means to disable it - if it is implemented in vaultwarden. If the authorization server does not support it correctly it is not OAuth compliant (at least not following best practices) and probably shouldn't be used.
Upcoming OAuth 2.1 makes PKCE mandatory for all clients using authorization code flow:

PKCE is required for all OAuth clients using the authorization code flow

The draft document states:
To prevent injection of authorization codes into the client, using code_challenge and code_verifier is REQUIRED for clients, and authorization servers MUST enforce their use, unless both of the following criteria are met:
- The client is a confidential client.
- In the specific deployment and the specific request, there is reasonable assurance by the authorization server that the client implements the OpenID Connect nonce mechanism properly.
In this case, using and enforcing code_challenge and code_verifier is still RECOMMENDED.
Not sure if this one exception above applies to vaultwarden.

Hey,

Thanks for the additional information :).

On the nonce correct implementation looking at Section 4.5.3.2 I believe it's the case.

The nonce is stored in DB and not available to the client or anyone else outside of when creating the initial request to the OpenID Provider (OP)
MUST validate the nonce in the ID Token obtained from the token endpoint It's the case.
MUST ensure that, unless and until that check succeeds, all tokens are disregarded and not used for any other purpose it's the case

With this I believe that Vaultwarden would qualify for the exception.

Therefore, I think it makes sense to enable PKCE by default and not provide any means to disable it

In general I would agree, but then you have stuff like this : https://learn.microsoft.com/en-us/answers/questions/218113/openid-connect-authorization-code-flow-with-proof (which create issues such as : github.com/argoproj/argo-cd/issues/16649).

probably shouldn't be used.

Not always so simple and in the end the security requirements are not the same for all deployments, if for example you are running only in a VPN. So as long as it does not introduce vulnerabilities the config allow for broader support without potentially having to add providers idiosyncrasies.

You're right. I failed to state more clearly that the best current practice document is also still a draft, not an official RFC. So authorization servers are currently not required to support PKCE, and a flag to disable PKCE might be useful.

But it seems there is a general consensus that PKCE is a good thing, and has good chances to become mandatory in the next specs. IMO it's a good idea to add PKCE support to vaultwarden already now, even if it is technically not required at the moment.

Added config SSO_PKCE.
Will be in 1.30.3-1, build usually take around ~1h.

I was able to enable PKCE support with Zitadel. The only issue is that SSO_CLIENT_SECRET is not provided in Zitadel if PKCE is used and currently vaultwarden checks if SSO_CLIENT_SECRET is presented when starting up. I had to write something to make vaultwarden happy. ~~but due to the ongoing issue I'm experiencing, I'm not sure if refresh_token will work.~~

Updated: Refresh token is working correctly in PKCE. The client no longer gets logged out like before.

@Halyul not a security expert but I would not activate PKCE if means you have to disable client secret.

I was able to enable PKCE support with Zitadel. The only issue is that SSO_CLIENT_SECRET is not provided in Zitadel if PKCE is used and currently vaultwarden checks if SSO_CLIENT_SECRET is presented when starting up. I had to write something to make vaultwarden happy. ~~but due to the ongoing issue I'm experiencing, I'm not sure if refresh_token will work.~~

Updated: Refresh token is working correctly in PKCE. The client no longer gets logged out like before.

Hi @Halyul ,
I am a begginer with Zitadel and Vaultwarden OIDC. Could you please share your Vaultwarden OIDC Configuration and tell me what settings you used in Zitadel ? I would like to achieve the same thing but I am a bit lost between all the oidc related settings in this fork of vaultwarden.

Thanks in advance for any answer and have a great day

Hi @Halyul , I am a begginer with Zitadel and Vaultwarden OIDC. Could you please share your Vaultwarden OIDC Configuration and tell me what settings you used in Zitadel ? I would like to achieve the same thing but I am a bit lost between all the oidc related settings in this fork of vaultwarden.

Thanks in advance for any answer and have a great day

For Vaultwarden OIDC, I have used the following envs:

SSO_ENABLED=true
SSO_PKCE=true
SSO_ONLY=true
SSO_AUTHORITY=https://zitadel.example.com
SSO_AUDIENCE_TRUSTED='^<project id>$'
SSO_CLIENT_ID
SSO_CLIENT_SECRET=<placeholder vaule>
SSO_SCOPES="email profile offline_access"

For Zitadel, I created a PKCE application with refresh token enabled and changed Auth Token Type to JWT under Token Settings

@Halyul Wonderful, thanks a lot ! I have some additional questions.

May I ask how you found out that Auth Token Type needed to be JWT and not Bearer token ?

I used SSO_FRONTEND=override in addition to your settings. How will login work with non-web clients like Android app, browser addon... ?

I am having the following error :
Could not read id_token claims, Invalid audiences: `264080136108834819@server` is not a trusted audience
What is very weird is that the ID in this error in not my Project ID, not my Organization ID, not my User ID, not my client ID. So I tried putting it in SSO_AUDIENCE_TRUSTED but I still have the same error. I'm curious about what it could be.

May I ask how you found out that Auth Token Type needed to be JWT and not Bearer token ?

JWT allows Vaultwarden to extract token expiry time set from Ztiadel, otherwise the expiry time is set by Vaultwarden if I remember correctly.

I used SSO_FRONTEND=override in addition to your settings. How will login work with non-web clients like Android app, browser addon... ?

I missed that when I copied and pasted those envs :).

I am having the following error : Could not read id_token claims, Invalid audiences: `264080136108834819@server` is not a trusted audience What is very weird is that the ID in this error in not my Project ID, not my Organization ID, not my User ID, not my client ID. So I tried putting it in SSO_AUDIENCE_TRUSTED but I still have the same error. I'm curious about what it could be.

Im not sure. In my case, the invalid audience is Resource Id shown in the project page (which I called it Project Id in my last comment). You can try oidcdebugger.com to find out what data is included in the payload. Another possibility is that the regular expression used in SSO_AUDIENCE_TRUSTED is incorrect, which prevents Vaultwarden trusting the Id

@Halyul Thanks for your answers.

I just tried OIDC debugger (after adding it to allowed redirect URIs) and it says it is successful. It says this :

I didn't understand if that means that PKCE challenge has already been done and that the successful state includes it, or if further action is required to test that as well.

This is probably a question not specific to Zitadel and more general about VW OIDC, but I didn't understand how OIDC will work in apps like the Android app or browser addon. Will Bitwarden show the SSO button instead of email field like on VW OIDC web ui when SSO_FRONTEND=override is set ?

In my case, the invalid audience is Resource Id shown in the project page

You mean that you also have an invalid audience error ? You can't access the web vault with Zitadel either ?
This doc

vaultwarden/SSO.md

Line 200 in f72bd26

    
           Additionnaly Zitadel include the `Project id` and the `Client Id` in the audience of the Id Token.

explicitely says that it is the Project ID that should be added to trusted audience. I think you are right and that it should be the resource id under project settings. Do you think the doc could be wrong ?

Will Bitwarden show the SSO button instead of email field like on VW OIDC web ui when SSO_FRONTEND=override is set ?

Bitwarden client will detect whether the server supports sso. When supported, a button named Enterprise Login will show in the page where the user enters their master password after entering the email.

Do you think the doc could be wrong ?

It's probably due to here, which I explicitly said project id referring to Resource Id in zitadel. Thus, @Timshel uses Project id in the doc.

@Timshel could you update the doc so confusion won't happen again? thanks

Thanks for your explanations. So it is sure that the ID that should be added to trusted audience is the project IDs? Would you have an idea about what could be wrong with my setup that leads to my error?

So it is sure that the ID that should be added to trusted audience is the project IDs?

In my case, yes. I'm not sure about your case, as it seems that anything with @ in zitadel is referring to client id. But you said you can't find this anywhere in zitadel :(.

My recommendation is to use oidc debugger to find out what's inside id_token.
For using oidc debugger, you need to:

In step 1, use to option Use PKCE? and note down Code Verifier
In step 2, make a post request to zitadel using curl or other tools with the state and Authorization Code vaule, e.g.

$ curl  -X POST \
  'https://zitadel.example.com/oauth/v2/token' \
  --header 'Content-Type: application/x-www-form-urlencoded' \
  --data-urlencode 'grant_type=authorization_code' \
  --data-urlencode 'code=<authorization code in step 2>' \
  --data-urlencode 'client_id=<client id>' \
  --data-urlencode 'redirect_uri=https%3A%2F%2Foidcdebugger.com%2Fdebug' \
  --data-urlencode 'code_verifier=<code verifier from steo 1>' \
  --data-urlencode 'state=<state value in step 2>'

Then you will get a response, e.g.

{
  "access_token": "...",
  "token_type": "Bearer",
  "expires_in": 43199,
  "id_token": "..."
}

Copy the value in id_token, paste to jwt.io, and find out what's inside your payload, e.g.,

{
  "iss": "https://zitadel.example.com",
  "sub": "...",
  "aud": [
    "<client id>",
    "<Resource Id located at project page>"
  ],
  "exp": ...,
  "iat": ...,
  "auth_time": ...,
  "nonce": "...",
  "amr": [...],
  "azp": "<client id>",
  "client_id": "<client id>",
  "at_hash": "...",
  "c_hash": "..."
}

To view the tokens you should be able to just activate SSO_DEBUG_TOKENS=true then try a login flow.

Hi @Halyul , I made an issue about my problem with Zitadel + Vaultwarden : #54 I would like to ask if you could show me your whole Zitadel configuration for the Vaultwarden application, as I suspect my Zitadel is not sending some informations to Vaultwarden. Here Is mine:

I have simply created an app with the PKCE profile, enabled Refresh Token and changed auth token type from Bearer to JWT.
Is yours different from this ?

Thanks in advance for any answer, have a nice day