Implement RFC 7636 PKCE in OAuth client and fix redirect URI bug #102
Conversation
Implement the Proof Key for Code Exchange (RFC 7636) specification in the OAuth2 client.
| // | ||
| // 96 bytes -> 768 bits -> 128 base64url characters (6 bits per character) | ||
| // | ||
| var buf = new byte[96]; |
There was a problem hiding this comment.
Yes, this is also how we do it https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/94e6e2e3baf840b9e06c8afbe2197d8ec0c0871f/src/client/Microsoft.Identity.Client/Platforms/netcore/NetCoreCryptographyManager.cs#L21
But we use 32 bytes... and perform the padding logic - base64url encoding (which is classical base64 + padding) is very common in OAuth ...
There was a problem hiding this comment.
Is there a reason for the 32 bytes? (42.6667 base64 chars with a pad that'll always be trimmed)?
There was a problem hiding this comment.
I don't remember the reason, there was probably a reference implementation somewhere that was used. It did pass security reviews though.
There was a problem hiding this comment.
But obviously your argument of using 96 bytes for more entropy is valid, this is a better implementation.
There was a problem hiding this comment.
The nice thing about 96 bytes is this works out to a base64url string that doens't need padding!
We still trim any padding characters to be doubly sure however 😁
src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2SystemWebBrowser.cs
Show resolved
Hide resolved
Implement the Proof Key for Code Exchange (RFC 7636) specification in the OAuth2 client. This helps prevent hijacking of the authorisation code by malicious applications running on the user's machine.
Also fix a bug that was identified in the use of redirect URIs between the authorisation and token endpoints. If the redirect URI is "localhost" without an explicit port number, the port number we find and open must be included in the redirect URI sent to the token endpoint (i.e, the redirect URI containing the generated port number used in the token endpoint call).