Re: [keycloak-dev] PKCE support for Keycloak Adapters (OAuthRequestAuthenticator)

There's a JavaScript adapter test in the base testsuite. On my phone now so can't find the correct name. Probably called JavaScriptAdapterTest or something like that. On Fri, 8 Jun 2018, 18:35 Thomas Darimont, <thomas.darimont(a)googlemail.com&gt; wrote: > Hi Marek, > > thanks for your helpful feedback! > I extracted the generateRandomData() function from the > generateCodeVerifier() function, which could be > reused for state and UUID generation. I also added a fallback in case > Uint8Array isn't available. > > I created a PR with the current implementation for KEYCLOAK-1033 (the > issue subject should be adapted to this) > https://github.com/keycloak/keycloak/pull/5255 > > What's the best way to implement an automated for this? > > Cheers, > Thomas > > On Wed, Jun 6, 2018 at 9:12 AM Marek Posolda <mposolda(a)redhat.com&gt; wrote: > >> Hi Thomas, >> >> I think it is great and will be good to support this in our javascript >> adapter OOTB. >> >> Few comments: >> - In keycloak.js, we use "createUUID()" function to generate "nonce" and >> "state" . IMO it will be good if all the things (nonce, state, >> codeVerifier...) are generated through same mechanism. I guess that we >> can use "window.crypto" or "window.msCrypto", but if none of them is >> available, then just fallback to what is currently used in >> "createUUID()" ? Also change the current generating of "nonce" and >> "state" to this as I suppose that "window.crypto" or "window.msCrypto" >> are safer alternatives than our current "createUUID()" ? >> >> - I have same concern as you about localStorage, but don't have any >> better alternative... No idea if sessionStorage is better. >> >> - We should document what browsers supports it. I guess that due to >> fallback for crypto/msCrypto, the "plain" method shouldn't be an issue >> for any browser as codeVerifier will be always generated just fine? But >> I guess that S256, which requires this "Uint8Array" stuff may have >> compatibility issues? Or am I wrong here? The issue is, that people can >> still use very old browsers (For example Windows XP with IE6) and not >> sure how bad it is if those people with old browsers can't >> authenticate... >> >> Marek >> >> >> On 05/06/18 21:33, Thomas Darimont wrote: >> > Hello, >> > >> > I gave it another spin and tested it with a whole bunch of browsers >> > - Windows (IE11, Edge, Firefox, Chrome) >> > - Linux (Chrome, Firefox) >> > >> > Current changes: (Added IE11 Support) >> > >> https://github.com/keycloak/keycloak/compare/master...thomasdarimont:poc/... >> > >> > If you want to give it a try, you could use the js-console example from >> > >> > >> https://github.com/keycloak/keycloak/blob/master/examples/js-console/src/... >> > and just copy the keycloak.js with the PKCE support into the webapp >> folder >> > an include the script from there. >> > >> > In the initOptions for Keycloak, just add "pkceMethod: 'S256'", e.g.: >> > >> > var initOptions = { >> > responseMode: 'fragment', >> > flow: 'standard', >> > pkceMethod: 'S256' >> > }; >> > >> > If you look at your browser dev-tools, you should now see, >> code_challenge >> > and code_challenge_method parameters added to your >> > /auth url. Additionally a code_verifier parameter should be sent with >> the >> > /token POST request. >> > >> > Some additional remarks: >> > >> > In order to support IE11 I had to use the window.msCrypto APIs and I >> added >> > a fallback if even that is not available. >> > Currently the generated code_verifier is stored in localStorage >> similar to >> > the 'kc-callback-...' values. >> > I think that localStorage is not the best place to store a temporary >> secret >> > ... I'm open to alternatives. >> > >> > The PKCE code_verifier is generated and stored in localStorage when >> > building the auth URL, where either >> > the plain value or an url-safe base64 encoded sha256 has of the >> > code_verifier is used as the code_challenge, depending on the chosen >> method. >> > Instead of localStorage one could probably also just use >> sessionStorage, >> > which would have the advantage of automatic deletion when the tab / >> browser >> > is closed. >> > >> > The two included libraries could be embedded / scoped into the >> keycloak JS >> > scope to not interfere with other included versions of the libraries. >> > >> > Cheers, >> > Thomas >> > >> > On Thu, May 31, 2018 at 1:09 PM Thomas Darimont < >> > thomas.darimont(a)googlemail.com&gt; wrote: >> > >> >> Hi folks, >> >> >> >> I've got a working PoC for PKCE support in the Keycloak JS adapter >> that >> >> supports `plain` and `S256` code_challenge_methods. >> >> Works very well with Keycloak 4.0.0.beta3 and Keycloak 3.4.3.Final. >> >> Perhaps this could serve as a base for a real PR. >> >> >> >> I needed to use two small js libraries to have proper support for >> sha256 >> >> and base64 encoding for Unit8Arrays in order to >> >> produce a codeChallenge in the same way the Keycloak server does it, >> for >> >> details see commit. >> >> >> >> Branch: >> >> >> https://github.com/thomasdarimont/keycloak/tree/poc/keycloak-js-pkce-support >> >> Commit: >> >> >> https://github.com/thomasdarimont/keycloak/commit/f05066d430f6504246f7e51... >> >> >> >> Looks like this would solve the issue >> >> https://issues.jboss.org/browse/KEYCLOAK-1033 (which would probably >> need >> >> to be renamed). >> >> >> >> Cheers, >> >> Thomas >> >> >> >> On Wed, May 30, 2018 at 11:57 PM Thomas Darimont < >> >> thomas.darimont(a)googlemail.com&gt; wrote: >> >> >> >>> Good point. Yes the main use case for PKCE are public clients / >> native >> >>> apps. >> >>> >> >>> However the recently published OAuth 2.0 Security Best Current >> Practice >> >>> (Draft 06 2018) [1] states: >> >>> " >> >>> 2.1. Protecting redirect-based flows >> >>> ... >> >>> Clients shall use PKCE [RFC7636] in order to (with the help of the >> >>> authorization server) detect and prevent attempts to inject >> (replay) >> >>> authorization codes into the authorization response. The PKCE >> >>> challenges must be transaction-specific and securely bound to the >> >>> user agent, in which the transaction was started. OpenID Connect >> >>> clients may use the "nonce" parameter of the OpenID Connect >> >>> authentication request as specified in [OpenID] in conjunction >> with >> >>> the corresponding ID Token claim for the same purpose. >> >>> >> >>> Note: although PKCE so far was recommended as mechanism to protect >> >>> native apps, this advice applies to all kinds of OAuth clients, >> >>> including web applications. >> >>> " >> >>> The last "Note" section was what inspired me to look into PKCE >> support >> >>> for server-side adapters as well. >> >>> But I generally agree that this is probably better suited for the JS >> / >> >>> CLI/ keycloak-installed adapters. >> >>> >> >>> [1] https://tools.ietf.org/html/draft-ietf-oauth-security-topics-06 >> >>> >> >>> On Wed, May 30, 2018 at 8:38 AM Stian Thorgersen < >> sthorger(a)redhat.com&gt; >> >>> wrote: >> >>> >> >>>> As PKCE is aimed at public clients why is there a need to add >> support >> >>>> for this to the Java adapters? Makes more sense to add this to the >> >>>> JavaScript adapter and CLI/desktop adapter. >> >>>> >> >>>> On 30 May 2018 at 07:47, 乗松隆志 / NORIMATSU，TAKASHI < >> >>>> takashi.norimatsu.ws(a)hitachi.com&gt; wrote: >> >>>> >> >>>>> Hello, >> >>>>> >> >>>>> I've encountered the same problem and gave up. >> >>>>> >> >>>>> At that time, the naive idea had hit on me. >> >>>>> * prepare some concurrently accessible singleton (line >> >>>>> KeycloakDeployment) from OAuthRequestAuthenticator >> >>>>> * store generated codeVerifier on it with state parameter value as >> its >> >>>>> key. >> >>>>> >> >>>>> But, considering the nature of codeVerifier, the followings are >> >>>>> required for such the store >> >>>>> * codeVerifier should be treated the same secure levels as client >> >>>>> credentials >> >>>>> * codeVerifier should be short-lived and deleted after its life the >> >>>>> same as Authorization Code >> >>>>> >> >>>>> Therefore, It might be better to create an tentative instance whose >> >>>>> lifetime is between issuing Authorization Code Request and issuing >> Token >> >>>>> Request. And, it should be identified and only accessible from the >> session >> >>>>> instance who issued Authorization Code Request. >> >>>>> >> >>>>> However, I'm afraid it might be difficult to accomplish it in >> generic >> >>>>> fashion. We need to implement the above each type of client >> adapter. >> >>>>> >> >>>>> Best regards, >> >>>>> Takashi Norimatsu >> >>>>> Hitachi Ltd., >> >>>>> >> >>>>> -----Original Message----- >> >>>>> From: keycloak-dev-bounces(a)lists.jboss.org < >> >>>>> keycloak-dev-bounces(a)lists.jboss.org&gt; On Behalf Of Thomas Darimont >> >>>>> Sent: Wednesday, May 30, 2018 9:02 AM >> >>>>> To: keycloak-dev <keycloak-dev(a)lists.jboss.org&gt; >> >>>>> Subject: [!][keycloak-dev] PKCE support for Keycloak Adapters >> >>>>> (OAuthRequestAuthenticator) >> >>>>> >> >>>>> Hi there, >> >>>>> >> >>>>> I was recently playing with the PKCE support in Keycloak (server) >> which >> >>>>> worked quite well. >> >>>>> However the support for client / adapters seems to be quite >> limited at >> >>>>> the moment... >> >>>>> >> >>>>> I think support for PKCE to all? java adapters could be added quite >> >>>>> easily >> >>>>> - I could provide a >> >>>>> PR but I'm currently stuck with finding a generic way to store the >> >>>>> codeVerifier generated for the login redirect for later retrival >> for the >> >>>>> code2token exchange. >> >>>>> >> >>>>> Do you have any recommendations for this? >> >>>>> >> >>>>> I created the following JIRA issue (with some comments) to track >> this: >> >>>>> >> >>>>> >> https://clicktime.symantec.com/a/1/bkUjActRvyW1Ds3zoQSu7mjr4Nabixm_1YJAW4... >> >>>>> >> >>>>> Cheers, >> >>>>> Thomas >> >>>>> _______________________________________________ >> >>>>> keycloak-dev mailing list >> >>>>> keycloak-dev(a)lists.jboss.org >> >>>>> >> >>>>> >> https://clicktime.symantec.com/a/1/Xn2ffdZIVPL_UA8_cnNApcirkcZZdsnyb6SpUd... >> >>>>> >> >>>>> _______________________________________________ >> >>>>> keycloak-dev mailing list >> >>>>> keycloak-dev(a)lists.jboss.org >> >>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> >>>>> >> >>>> >> > _______________________________________________ >> > keycloak-dev mailing list >> > keycloak-dev(a)lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/keycloak-dev >> >> >>