From 5f13ab2d333267c0851209db4837e196b902b0ed Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 21 Apr 2020 14:01:31 +0100 Subject: [PATCH] oauth: implement RFC 7636 PKCE in OAuth client Implement the Proof Key for Code Exchange (RFC 7636) specification in the OAuth2 client. --- Git-Credential-Manager.sln.DotSettings | 3 +- src/shared/GitHub/GitHubAuthentication.cs | 4 +- .../Authentication/OAuth2ClientTests.cs | 18 +-- .../OAuth2CryptographicCodeGeneratorTests.cs | 87 ++++++++++++++ .../Base64UrlConvertTests.cs | 37 ++++++ .../OAuth/OAuth2AuthorizationCodeResult.cs | 18 +++ .../Authentication/OAuth/OAuth2Client.cs | 44 ++++--- .../Authentication/OAuth/OAuth2Constants.cs | 6 +- .../OAuth/OAuth2CryptographicGenerator.cs | 107 +++++++++++++++++ .../OAuth/OAuth2NonceGenerator.cs | 19 --- .../Base64UrlConvert.cs | 27 +++++ .../Objects/TestOAuth2Server.cs | 109 ++++++++++++++++-- 12 files changed, 419 insertions(+), 60 deletions(-) create mode 100644 src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2CryptographicCodeGeneratorTests.cs create mode 100644 src/shared/Microsoft.Git.CredentialManager.Tests/Base64UrlConvertTests.cs create mode 100644 src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs create mode 100644 src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2CryptographicGenerator.cs delete mode 100644 src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2NonceGenerator.cs create mode 100644 src/shared/Microsoft.Git.CredentialManager/Base64UrlConvert.cs diff --git a/Git-Credential-Manager.sln.DotSettings b/Git-Credential-Manager.sln.DotSettings index b3ea5f20b..aa892666f 100644 --- a/Git-Credential-Manager.sln.DotSettings +++ b/Git-Credential-Manager.sln.DotSettings @@ -1,2 +1,3 @@  - No \ No newline at end of file + No + True \ No newline at end of file diff --git a/src/shared/GitHub/GitHubAuthentication.cs b/src/shared/GitHub/GitHubAuthentication.cs index 3ad9039c0..b6b45613d 100644 --- a/src/shared/GitHub/GitHubAuthentication.cs +++ b/src/shared/GitHub/GitHubAuthentication.cs @@ -203,9 +203,9 @@ public async Task GetOAuthTokenAsync(Uri targetUri, IEnumerab // Write message to the terminal (if any is attached) for some feedback that we're waiting for a web response Context.Terminal.WriteLine("info: please complete authentication in your browser..."); - string authCode = await oauthClient.GetAuthorizationCodeAsync(scopes, browser, CancellationToken.None); + OAuth2AuthorizationCodeResult authCodeResult = await oauthClient.GetAuthorizationCodeAsync(scopes, browser, CancellationToken.None); - return await oauthClient.GetTokenByAuthorizationCodeAsync(authCode, CancellationToken.None); + return await oauthClient.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None); } else { diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs index aaaa9084f..52938bbfd 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs @@ -39,9 +39,9 @@ public async Task OAuth2Client_GetAuthorizationCodeAsync() OAuth2Client client = CreateClient(httpHandler, endpoints); - string actualAuthCode = await client.GetAuthorizationCodeAsync(expectedScopes, browser, CancellationToken.None); + OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync(expectedScopes, browser, CancellationToken.None); - Assert.Equal(expectedAuthCode, actualAuthCode); + Assert.Equal(expectedAuthCode, result.Code); } [Fact] @@ -88,7 +88,7 @@ public async Task OAuth2Client_GetTokenByAuthorizationCodeAsync() string[] expectedScopes = {"read", "write", "delete"}; OAuth2Application app = CreateTestApplication(); - app.AuthCodes[authCode] = expectedScopes; + app.AuthGrants.Add(new OAuth2Application.AuthCodeGrant(authCode, expectedScopes)); var server = new TestOAuth2Server(endpoints); server.RegisterApplication(app); @@ -98,7 +98,8 @@ public async Task OAuth2Client_GetTokenByAuthorizationCodeAsync() OAuth2Client client = CreateClient(httpHandler, endpoints); - OAuth2TokenResult result = await client.GetTokenByAuthorizationCodeAsync(authCode, CancellationToken.None); + var authCodeResult = new OAuth2AuthorizationCodeResult(authCode); + OAuth2TokenResult result = await client.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None); Assert.NotNull(result); Assert.Equal(expectedScopes, result.Scopes); @@ -217,9 +218,10 @@ public async Task OAuth2Client_E2E_InteractiveWebFlowAndRefresh() OAuth2Client client = CreateClient(httpHandler, endpoints); - string authCode = await client.GetAuthorizationCodeAsync(expectedScopes, browser, CancellationToken.None); + OAuth2AuthorizationCodeResult authCodeResult = await client.GetAuthorizationCodeAsync( + expectedScopes, browser, CancellationToken.None); - OAuth2TokenResult result1 = await client.GetTokenByAuthorizationCodeAsync(authCode, CancellationToken.None); + OAuth2TokenResult result1 = await client.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None); Assert.NotNull(result1); Assert.Equal(expectedScopes, result1.Scopes); @@ -296,11 +298,11 @@ public async Task OAuth2Client_E2E_DeviceFlowAndRefresh() RedirectUris = new[] {TestRedirectUri} }; - private static OAuth2Client CreateClient(HttpMessageHandler httpHandler, OAuth2ServerEndpoints endpoints, IOAuth2NonceGenerator generator = null) + private static OAuth2Client CreateClient(HttpMessageHandler httpHandler, OAuth2ServerEndpoints endpoints, IOAuth2CodeGenerator generator = null) { return new OAuth2Client(new HttpClient(httpHandler), endpoints, TestClientId, TestRedirectUri, TestClientSecret) { - NonceGenerator = generator + CodeGenerator = generator }; } diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2CryptographicCodeGeneratorTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2CryptographicCodeGeneratorTests.cs new file mode 100644 index 000000000..8022d0f03 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2CryptographicCodeGeneratorTests.cs @@ -0,0 +1,87 @@ +using System.Linq; +using System.Security.Cryptography; +using System.Text; +using Microsoft.Git.CredentialManager.Authentication.OAuth; +using Xunit; + +namespace Microsoft.Git.CredentialManager.Tests.Authentication +{ + public class OAuth2CryptographicCodeGeneratorTests + { + private const string ValidBase64UrlCharsNoPad = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"; + + [Fact] + public void OAuth2CryptographicCodeGenerator_CreateNonce_IsUnique() + { + var generator = new OAuth2CryptographicCodeGenerator(); + + // Create a bunch of nonce values + var nonces = new string[32]; + for (int i = 0; i < nonces.Length; i++) + { + nonces[i] = generator.CreateNonce(); + } + + // There should be no duplicates + string[] uniqueNonces = nonces.Distinct().ToArray(); + Assert.Equal(uniqueNonces, nonces); + } + + [Fact] + public void OAuth2CryptographicCodeGenerator_CreatePkceCodeVerifier_IsUniqueBase64UrlStringWithoutPaddingAndLengthBetween43And128() + { + var generator = new OAuth2CryptographicCodeGenerator(); + + // Create a bunch of verifiers + var verifiers = new string[32]; + for (int i = 0; i < verifiers.Length; i++) + { + string v = generator.CreatePkceCodeVerifier(); + + // Assert the verifier is a base64url string without padding + char[] vs = v.ToCharArray(); + Assert.All(vs, x => Assert.Contains(x, ValidBase64UrlCharsNoPad)); + + // Assert the verifier is a string of length [43, 128] (inclusive) + Assert.InRange(v.Length, 43, 128); + + verifiers[i] = v; + } + + // There should be no duplicates + string[] uniqueVerifiers = verifiers.Distinct().ToArray(); + Assert.Equal(uniqueVerifiers, verifiers); + } + + [Fact] + public void OAuth2CryptographicCodeGenerator_CreatePkceCodeChallenge_Plain_ReturnsVerifierUnchanged() + { + var generator = new OAuth2CryptographicCodeGenerator(); + + var verifier = generator.CreatePkceCodeVerifier(); + var challenge = generator.CreatePkceCodeChallenge(OAuth2PkceChallengeMethod.Plain, verifier); + + Assert.Equal(verifier, challenge); + } + + [Fact] + public void OAuth2CryptographicCodeGenerator_CreatePkceCodeChallenge_Sha256_ReturnsBase64UrlEncodedSha256HashOfAsciiVerifier() + { + var generator = new OAuth2CryptographicCodeGenerator(); + + var verifier = generator.CreatePkceCodeVerifier(); + + byte[] verifierAsciiBytes = Encoding.ASCII.GetBytes(verifier); + byte[] hashedBytes; + using (var sha256 = SHA256.Create()) + { + hashedBytes = sha256.ComputeHash(verifierAsciiBytes); + } + + var expectedChallenge = Base64UrlConvert.Encode(hashedBytes, false); + var actualChallenge = generator.CreatePkceCodeChallenge(OAuth2PkceChallengeMethod.Sha256, verifier); + + Assert.Equal(expectedChallenge, actualChallenge); + } + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Base64UrlConvertTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Base64UrlConvertTests.cs new file mode 100644 index 000000000..8b5ba7167 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Base64UrlConvertTests.cs @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using Xunit; + +namespace Microsoft.Git.CredentialManager.Tests +{ + public class Base64UrlConvertTests + { + [Theory] + [InlineData(new byte[0], "")] + [InlineData(new byte[]{4}, "BA==")] + [InlineData(new byte[]{4,5}, "BAU=")] + [InlineData(new byte[]{4,5,6}, "BAUG")] + [InlineData(new byte[]{4,5,6,7}, "BAUGBw==")] + [InlineData(new byte[]{4,5,6,7,8}, "BAUGBwg=")] + [InlineData(new byte[]{4,5,6,7,8,9}, "BAUGBwgJ")] + public void Base64UrlConvert_Encode_WithPadding(byte[] data, string expected) + { + string actual = Base64UrlConvert.Encode(data, includePadding: true); + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData(new byte[0], "")] + [InlineData(new byte[]{4}, "BA")] + [InlineData(new byte[]{4,5}, "BAU")] + [InlineData(new byte[]{4,5,6}, "BAUG")] + [InlineData(new byte[]{4,5,6,7}, "BAUGBw")] + [InlineData(new byte[]{4,5,6,7,8}, "BAUGBwg")] + [InlineData(new byte[]{4,5,6,7,8,9}, "BAUGBwgJ")] + public void Base64UrlConvert_Encode_WithoutPadding(byte[] data, string expected) + { + string actual = Base64UrlConvert.Encode(data, includePadding: false); + Assert.Equal(expected, actual); + } + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs new file mode 100644 index 000000000..800577bf1 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +namespace Microsoft.Git.CredentialManager.Authentication.OAuth +{ + public class OAuth2AuthorizationCodeResult + { + public OAuth2AuthorizationCodeResult(string code, string codeVerifier = null) + { + Code = code; + CodeVerifier = codeVerifier; + } + + public string Code { get; } + + public string CodeVerifier { get; } + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs index b817fd03d..2ae835a5d 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs @@ -2,10 +2,8 @@ // Licensed under the MIT license. using System; using System.Collections.Generic; -using System.Net; using System.Net.Http; using System.Net.Http.Headers; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.Git.CredentialManager.Authentication.OAuth.Json; @@ -26,7 +24,7 @@ public interface IOAuth2Client /// User agent to use to start the authorization code grant flow. /// Token to cancel the operation. /// Authorization code. - Task GetAuthorizationCodeAsync(IEnumerable scopes, IOAuth2WebBrowser browser, CancellationToken ct); + Task GetAuthorizationCodeAsync(IEnumerable scopes, IOAuth2WebBrowser browser, CancellationToken ct); /// /// Retrieve a device code grant. @@ -40,10 +38,10 @@ public interface IOAuth2Client /// /// Exchange an authorization code acquired from for an access token. /// - /// Authorization code. + /// Authorization code grant result. /// Token to cancel the operation. /// Token result. - Task GetTokenByAuthorizationCodeAsync(string authorizationCode, CancellationToken ct); + Task GetTokenByAuthorizationCodeAsync(OAuth2AuthorizationCodeResult authorizationCodeResult, CancellationToken ct); /// /// Use a refresh token to get a new access token. @@ -70,7 +68,7 @@ public class OAuth2Client : IOAuth2Client private readonly string _clientId; private readonly string _clientSecret; - private IOAuth2NonceGenerator _nonceGenerator; + private IOAuth2CodeGenerator _codeGenerator; public OAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, string clientId, Uri redirectUri = null, string clientSecret = null) { @@ -81,24 +79,29 @@ public OAuth2Client(HttpClient httpClient, OAuth2ServerEndpoints endpoints, stri _clientSecret = clientSecret; } - public IOAuth2NonceGenerator NonceGenerator + public IOAuth2CodeGenerator CodeGenerator { - get => _nonceGenerator ?? (_nonceGenerator = new OAuth2NonceGenerator()); - set => _nonceGenerator = value; + get => _codeGenerator ?? (_codeGenerator = new OAuth2CryptographicCodeGenerator()); + set => _codeGenerator = value; } #region IOAuth2Client - public async Task GetAuthorizationCodeAsync(IEnumerable scopes, IOAuth2WebBrowser browser, CancellationToken ct) + public async Task GetAuthorizationCodeAsync(IEnumerable scopes, IOAuth2WebBrowser browser, CancellationToken ct) { - string state = NonceGenerator.CreateNonce(); - string scopesStr = string.Join(" ", scopes); + string state = CodeGenerator.CreateNonce(); + string codeVerifier = CodeGenerator.CreatePkceCodeVerifier(); + string codeChallenge = CodeGenerator.CreatePkceCodeChallenge(OAuth2PkceChallengeMethod.Sha256, codeVerifier); var queryParams = new Dictionary { - [OAuth2Constants.AuthorizationEndpoint.ResponseTypeParameter] = OAuth2Constants.AuthorizationEndpoint.AuthorizationCodeResponseType, + [OAuth2Constants.AuthorizationEndpoint.ResponseTypeParameter] = + OAuth2Constants.AuthorizationEndpoint.AuthorizationCodeResponseType, [OAuth2Constants.ClientIdParameter] = _clientId, [OAuth2Constants.AuthorizationEndpoint.StateParameter] = state, + [OAuth2Constants.AuthorizationEndpoint.PkceChallengeMethodParameter] = + OAuth2Constants.AuthorizationEndpoint.PkceChallengeMethodS256, + [OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter] = codeChallenge }; if (_redirectUri != null) @@ -106,6 +109,7 @@ public async Task GetAuthorizationCodeAsync(IEnumerable scopes, queryParams[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString(); } + string scopesStr = string.Join(" ", scopes); if (!string.IsNullOrWhiteSpace(scopesStr)) { queryParams[OAuth2Constants.ScopeParameter] = scopesStr; @@ -140,7 +144,7 @@ public async Task GetAuthorizationCodeAsync(IEnumerable scopes, throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response."); } - return authCode; + return new OAuth2AuthorizationCodeResult(authCode, codeVerifier); } public async Task GetDeviceCodeAsync(IEnumerable scopes, CancellationToken ct) @@ -177,15 +181,21 @@ public async Task GetDeviceCodeAsync(IEnumerable } } - public async Task GetTokenByAuthorizationCodeAsync(string authorizationCode, CancellationToken ct) + public async Task GetTokenByAuthorizationCodeAsync(OAuth2AuthorizationCodeResult authorizationCodeResult, CancellationToken ct) { var formData = new Dictionary { [OAuth2Constants.TokenEndpoint.GrantTypeParameter] = OAuth2Constants.TokenEndpoint.AuthorizationCodeGrantType, - [OAuth2Constants.TokenEndpoint.AuthorizationCodeParameter] = authorizationCode, - [OAuth2Constants.ClientIdParameter] = _clientId, + [OAuth2Constants.TokenEndpoint.AuthorizationCodeParameter] = authorizationCodeResult.Code, + [OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier, + [OAuth2Constants.ClientIdParameter] = _clientId }; + if (authorizationCodeResult.CodeVerifier != null) + { + formData[OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier; + } + if (_redirectUri != null) { formData[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString(); diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Constants.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Constants.cs index e6ec17198..88cc35553 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Constants.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Constants.cs @@ -14,6 +14,10 @@ public static class AuthorizationEndpoint public const string StateParameter = "state"; public const string AuthorizationCodeResponseType = "code"; public const string ResponseTypeParameter = "response_type"; + public const string PkceChallengeParameter = "code_challenge"; + public const string PkceChallengeMethodParameter = "code_challenge_method"; + public const string PkceChallengeMethodPlain = "plain"; + public const string PkceChallengeMethodS256 = "S256"; } public static class AuthorizationGrantResponse @@ -30,7 +34,7 @@ public static class TokenEndpoint public const string GrantTypeParameter = "grant_type"; public const string AuthorizationCodeGrantType = "authorization_code"; public const string RefreshTokenGrantType = "refresh_token"; - + public const string PkceVerifierParameter = "code_verifier"; public const string AuthorizationCodeParameter = "code"; public const string RefreshTokenParameter = "refresh_token"; } diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2CryptographicGenerator.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2CryptographicGenerator.cs new file mode 100644 index 000000000..cde623b60 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2CryptographicGenerator.cs @@ -0,0 +1,107 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Security.Cryptography; +using System.Text; + +namespace Microsoft.Git.CredentialManager.Authentication.OAuth +{ + public enum OAuth2PkceChallengeMethod + { + Plain = 0, + Sha256 = 1 + } + + public interface IOAuth2CodeGenerator + { + /// + /// Create a random string value suitable for use as a nonce. + /// + /// A random string. + string CreateNonce(); + + /// + /// Create a cryptographically random code verifier for use with Proof Key for Code Exchange (PKCE). + /// + /// PKCE code verifier. + string CreatePkceCodeVerifier(); + + /// + /// Create a code challenge for the given Proof Key for Code Exchange (PKCE) code verifier. + /// + /// Challenge method. + /// /// PKCE code verifier. + /// PKCE code challenge. + string CreatePkceCodeChallenge(OAuth2PkceChallengeMethod challengeMethod, string codeVerifier); + } + + public class OAuth2CryptographicCodeGenerator : IOAuth2CodeGenerator + { + // Do not include padding of the base64url string to avoid percent-encoding when passed in a URI + private const bool PkceIncludeBase64UrlPadding = false; + + public string CreateNonce() + { + return Guid.NewGuid().ToString("N"); + } + + public string CreatePkceCodeVerifier() + { + // + // RFC 7636 requires the code verifier to match the following ABNF: + // + // code-verifier = 43*128unreserved + // unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" + // ALPHA = %x41-5A / %x61-7A + // DIGIT = %x30-39 + // + // The base64url encoding (RFC 6749) is a subset of these characters + // so we opt to convert our random bytes into Base64URL format for + // the code verifier. + // + // RFC 7636 mandates the code verifier must be between 43 and 128 characters + // long (inclusive). We want to generate a string at the top end of this range + // for maximum entropy. At the same time we want to avoid using the padding + // character '=' because this character is percent-encoded when used in URLs. + // To avoid padding we need the number of input bytes to be divisible by 3. + // + // In order to achieve 128 base64url characters AND avoid padding we should + // generate exactly 96 random bytes. Why 96 bytes? 96 is divisible by 3 and: + // + // 96 bytes -> 768 bits -> 128 base64url characters (6 bits per character) + // + var buf = new byte[96]; + var rng = RandomNumberGenerator.Create(); + rng.GetBytes(buf); + + return Base64UrlConvert.Encode(buf, PkceIncludeBase64UrlPadding); + } + + public string CreatePkceCodeChallenge(OAuth2PkceChallengeMethod challengeMethod, string codeVerifier) + { + switch (challengeMethod) + { + case OAuth2PkceChallengeMethod.Plain: + return codeVerifier; + + case OAuth2PkceChallengeMethod.Sha256: + // The "S256" code challenge is computed as follows, per RFC 7636: + // + // code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) + // + using (var sha256 = SHA256.Create()) + { + return Base64UrlConvert.Encode( + sha256.ComputeHash( + Encoding.ASCII.GetBytes(codeVerifier) + ), + PkceIncludeBase64UrlPadding + ); + } + + default: + throw new ArgumentOutOfRangeException(nameof(challengeMethod), challengeMethod, "Unknown PKCE code challenge method."); + } + } + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2NonceGenerator.cs b/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2NonceGenerator.cs deleted file mode 100644 index 188478f33..000000000 --- a/src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2NonceGenerator.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT license. -using System; - -namespace Microsoft.Git.CredentialManager.Authentication.OAuth -{ - public interface IOAuth2NonceGenerator - { - string CreateNonce(); - } - - public class OAuth2NonceGenerator : IOAuth2NonceGenerator - { - public string CreateNonce() - { - return Guid.NewGuid().ToString("N"); - } - } -} diff --git a/src/shared/Microsoft.Git.CredentialManager/Base64UrlConvert.cs b/src/shared/Microsoft.Git.CredentialManager/Base64UrlConvert.cs new file mode 100644 index 000000000..7e65cd535 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager/Base64UrlConvert.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; + +namespace Microsoft.Git.CredentialManager +{ + public static class Base64UrlConvert + { + public static string Encode(byte[] data, bool includePadding = true) + { + const char base64PadCharacter = '='; + const char base64Character62 = '+'; + const char base64Character63 = '/'; + const char base64UrlCharacter62 = '-'; + const char base64UrlCharacter63 = '_'; + + // The base64url format is the same as regular base64 format except: + // 1. character 62 is "-" (minus) not "+" (plus) + // 2. character 63 is "_" (underscore) not "/" (slash) + string base64Url = Convert.ToBase64String(data) + .Replace(base64Character62, base64UrlCharacter62) + .Replace(base64Character63, base64UrlCharacter63); + + return includePadding ? base64Url : base64Url.TrimEnd(base64PadCharacter); + } + } +} diff --git a/src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs b/src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs index 6e693fad0..7f45d2ca0 100644 --- a/src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs +++ b/src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs @@ -5,6 +5,8 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Security.Cryptography; +using System.Text; using System.Threading.Tasks; using Microsoft.Git.CredentialManager.Authentication.OAuth; using Microsoft.Git.CredentialManager.Authentication.OAuth.Json; @@ -72,12 +74,36 @@ private Task OnAuthorizationEndpointAsync(HttpRequestMessag reqQuery.TryGetValue(OAuth2Constants.ScopeParameter, out string scopeStr); string[] scopes = scopeStr?.Split(' '); + // Code challenge is optional + reqQuery.TryGetValue(OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter, out string codeChallenge); + + // Code challenge method is optional and defaults to "plain" + var codeChallengeMethod = OAuth2PkceChallengeMethod.Plain; + if (reqQuery.TryGetValue(OAuth2Constants.AuthorizationEndpoint.PkceChallengeMethodParameter, out string challengeMethodStr)) + { + if (StringComparer.OrdinalIgnoreCase.Equals(challengeMethodStr, + OAuth2Constants.AuthorizationEndpoint.PkceChallengeMethodPlain)) + { + codeChallengeMethod = OAuth2PkceChallengeMethod.Plain; + } + else if (StringComparer.OrdinalIgnoreCase.Equals(challengeMethodStr, + OAuth2Constants.AuthorizationEndpoint.PkceChallengeMethodS256)) + { + codeChallengeMethod = OAuth2PkceChallengeMethod.Sha256; + } + else + { + throw new Exception($"Unsupported code challenge method '{challengeMethodStr}'"); + } + } + // Create the auth code grant - string code = app.CreateAuthorizationCode(TokenGenerator, scopes); + OAuth2Application.AuthCodeGrant grant = app.CreateAuthorizationCodeGrant( + TokenGenerator, scopes, codeChallenge, codeChallengeMethod); var respQuery = new Dictionary { - [OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter] = code + [OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter] = grant.Code }; // State is optional but must be included in the reply if specified @@ -162,9 +188,11 @@ private async Task OnTokenEndpointAsync(HttpRequestMessage throw new Exception("Missing authorization code parameter"); } + formData.TryGetValue(OAuth2Constants.TokenEndpoint.PkceVerifierParameter, out string codeVerifier); + app.ValidateRedirect(formData[OAuth2Constants.RedirectUriParameter]); - tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode); + tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode, codeVerifier); } else if (StringComparer.OrdinalIgnoreCase.Equals(grantType, OAuth2Constants.TokenEndpoint.RefreshTokenGrantType)) { @@ -257,6 +285,22 @@ private static string GetNextValueOrRandom(List values, ref int index) public class OAuth2Application { + public class AuthCodeGrant + { + public AuthCodeGrant(string code, string[] scopes, + string codeChallenge = null, OAuth2PkceChallengeMethod codeChallengeMethod = OAuth2PkceChallengeMethod.Plain) + { + Code = code; + Scopes = scopes; + CodeChallenge = codeChallenge; + CodeChallengeMethod = codeChallengeMethod; + } + public string Code { get; } + public string[] Scopes { get; } + public string CodeChallenge { get; } + public OAuth2PkceChallengeMethod CodeChallengeMethod { get; } + } + public class DeviceCodeGrant { public DeviceCodeGrant(string userCode, string deviceCode, string[] scopes) @@ -283,7 +327,7 @@ public OAuth2Application(string id) public Uri[] RedirectUris { get; set; } - public IDictionary AuthCodes { get; } = new Dictionary(); + public IList AuthGrants { get; } = new List(); public IList DeviceGrants { get; } = new List(); @@ -291,12 +335,15 @@ public OAuth2Application(string id) public IDictionary RefreshTokens { get; } = new Dictionary(); - public string CreateAuthorizationCode(TestOAuth2ServerTokenGenerator generator, IEnumerable scopes) + public AuthCodeGrant CreateAuthorizationCodeGrant(TestOAuth2ServerTokenGenerator generator, + string[] scopes, string codeChallenge, OAuth2PkceChallengeMethod codeChallengeMethod) { string code = generator.CreateAuthorizationCode(); - AuthCodes[code] = scopes.ToArray(); - return code; + var grant = new AuthCodeGrant(code, scopes, codeChallenge, codeChallengeMethod); + AuthGrants.Add(grant); + + return grant; } public DeviceCodeGrant CreateDeviceCodeGrant(TestOAuth2ServerTokenGenerator generator, string[] scopes) @@ -339,29 +386,67 @@ public bool IsDeviceCodeGrantApproved(string deviceCode) return grant.Approved; } - public TokenEndpointResponseJson CreateTokenByAuthorizationGrant(TestOAuth2ServerTokenGenerator generator, string authCode) + public TokenEndpointResponseJson CreateTokenByAuthorizationGrant( + TestOAuth2ServerTokenGenerator generator, string authCode, string codeVerifier) { - if (!AuthCodes.TryGetValue(authCode, out string[] scopes)) + var grant = AuthGrants.FirstOrDefault(x => x.Code == authCode); + if (grant is null) { throw new Exception($"Invalid authorization code '{authCode}'"); } + if (!string.IsNullOrWhiteSpace(grant.CodeChallenge)) + { + if (string.IsNullOrWhiteSpace(codeVerifier)) + { + throw new Exception("Missing code verifier"); + } + + switch (grant.CodeChallengeMethod) + { + case OAuth2PkceChallengeMethod.Sha256: + using (var sha256 = SHA256.Create()) + { + string challenge = Base64UrlConvert.Encode( + sha256.ComputeHash( + Encoding.ASCII.GetBytes(codeVerifier) + ), + false + ); + + if (challenge != grant.CodeChallenge) + { + throw new Exception($"Invalid code verifier '{codeVerifier}'"); + } + } + break; + + case OAuth2PkceChallengeMethod.Plain: + // The case matters! + if (!StringComparer.Ordinal.Equals(codeVerifier, grant.CodeChallenge)) + { + throw new Exception($"Invalid code verifier '{codeVerifier}'"); + } + break; + } + } + string accessToken = generator.CreateAccessToken(); string refreshToken = generator.CreateRefreshToken(); // Remove the auth code grant now we've generated an access token (do not allow auth code reuse) - AuthCodes.Remove(authCode); + AuthGrants.Remove(grant); // Store the tokens AccessTokens[accessToken] = refreshToken; - RefreshTokens[refreshToken] = scopes; + RefreshTokens[refreshToken] = grant.Scopes; return new TokenEndpointResponseJson { TokenType = Constants.Http.WwwAuthenticateBearerScheme, AccessToken = accessToken, RefreshToken = refreshToken, - Scope = string.Join(" ", scopes) // Keep the same scopes as before + Scope = string.Join(" ", grant.Scopes) // Keep the same scopes as before }; }