From 5a8bf4c8901b8ed6db6df24c1b34bdf8d9151081 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 20 Mar 2025 15:13:05 +0100 Subject: [PATCH] Innovation/opaque grant validator (#5533) * Add grant validator * Fix 2fa * Add featureflag * Add comments * Cleanup * Set active endpoint * Fix test --- .../OpaqueKeyExchangeController.cs | 10 +- .../OpaqueSetRegistrationActiveRequest.cs | 9 ++ .../Services/IOpaqueKeyExchangeService.cs | 18 ++- .../OpaqueKeyExchangeService.cs | 50 +++++- .../Services/Implementations/UserService.cs | 2 +- .../Controllers/AccountsController.cs | 4 +- src/Identity/IdentityServer/ApiClient.cs | 2 +- .../OpaqueKeyExchangeGrantValidator.cs | 144 ++++++++++++++++++ .../Utilities/ServiceCollectionExtensions.cs | 3 +- .../Controllers/AccountsControllerTests.cs | 3 + 10 files changed, 231 insertions(+), 14 deletions(-) create mode 100644 src/Core/Auth/Models/Api/Request/Opaque/OpaqueSetRegistrationActiveRequest.cs create mode 100644 src/Identity/IdentityServer/RequestValidators/OpaqueKeyExchangeGrantValidator.cs diff --git a/src/Api/Auth/Controllers/OpaqueKeyExchangeController.cs b/src/Api/Auth/Controllers/OpaqueKeyExchangeController.cs index f307d0d2c9..9136afe67d 100644 --- a/src/Api/Auth/Controllers/OpaqueKeyExchangeController.cs +++ b/src/Api/Auth/Controllers/OpaqueKeyExchangeController.cs @@ -10,7 +10,6 @@ using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; [Route("opaque")] -[Authorize("Web")] public class OpaqueKeyExchangeController : Controller { private readonly IOpaqueKeyExchangeService _opaqueKeyExchangeService; @@ -25,6 +24,7 @@ public class OpaqueKeyExchangeController : Controller _userService = userService; } + [Authorize("Web")] [HttpPost("~/opaque/start-registration")] public async Task StartRegistrationAsync([FromBody] OpaqueRegistrationStartRequest request) { @@ -34,6 +34,7 @@ public class OpaqueKeyExchangeController : Controller } + [Authorize("Web")] [HttpPost("~/opaque/finish-registration")] public async void FinishRegistrationAsync([FromBody] OpaqueRegistrationFinishRequest request) { @@ -41,6 +42,13 @@ public class OpaqueKeyExchangeController : Controller await _opaqueKeyExchangeService.FinishRegistration(request.SessionId, Convert.FromBase64String(request.RegistrationUpload), user, request.KeySet); } + [Authorize("Web")] + [HttpPost("~/opaque/set-registration-active")] + public async void SetRegistrationActive([FromBody] OpaqueSetRegistrationActiveRequest request) + { + var user = await _userService.GetUserByPrincipalAsync(User); + await _opaqueKeyExchangeService.SetRegistrationActiveForAccount(request.SessionId, user); + } // TODO: Remove and move to token endpoint [HttpPost("~/opaque/start-login")] diff --git a/src/Core/Auth/Models/Api/Request/Opaque/OpaqueSetRegistrationActiveRequest.cs b/src/Core/Auth/Models/Api/Request/Opaque/OpaqueSetRegistrationActiveRequest.cs new file mode 100644 index 0000000000..66bb682f86 --- /dev/null +++ b/src/Core/Auth/Models/Api/Request/Opaque/OpaqueSetRegistrationActiveRequest.cs @@ -0,0 +1,9 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Core.Auth.Models.Api.Request.Opaque; + +public class OpaqueSetRegistrationActiveRequest +{ + [Required] + public Guid SessionId { get; set; } +} diff --git a/src/Core/Auth/Services/IOpaqueKeyExchangeService.cs b/src/Core/Auth/Services/IOpaqueKeyExchangeService.cs index bc67fce2a1..1d1d4365e9 100644 --- a/src/Core/Auth/Services/IOpaqueKeyExchangeService.cs +++ b/src/Core/Auth/Services/IOpaqueKeyExchangeService.cs @@ -35,22 +35,34 @@ public interface IOpaqueKeyExchangeService /// tuple(login SessionId for cache lookup, Server crypto material) public Task<(Guid, byte[])> StartLogin(byte[] request, string email); /// - /// Accepts the client's login request and validates it against the server's crypto material. If successful then the user is logged in. + /// Accepts the client's login request and validates it against the server's crypto material. If successful then the session is marked as authenticated. /// If using a fake account we will return a standard failed login. If the account does have a legitimate credential but is still invalid - /// we will return a failed login. + /// the session is not marked as authenticated. /// /// /// /// public Task FinishLogin(Guid sessionId, byte[] finishCredential); /// + /// Returns the user for the authentication session, or null if the session is invalid or has not yet finished authentication. + /// + /// + /// + public Task GetUserForAuthenticatedSession(Guid sessionId); + /// + /// Clears the authentication session from the cache. + /// + /// + /// + public Task ClearAuthenticationSession(Guid sessionId); + /// /// This is where registration really finishes. This method writes the Credential to the database. If a credential already exists then it will be removed before the new one is added. /// A user can only have one credential. /// /// cache value /// user being acted on /// void - public Task SetActive(Guid sessionId, User user); + public Task SetRegistrationActiveForAccount(Guid sessionId, User user); /// /// Removes the credential for the user. /// diff --git a/src/Core/Auth/Services/Implementations/OpaqueKeyExchangeService.cs b/src/Core/Auth/Services/Implementations/OpaqueKeyExchangeService.cs index 4b9b4bce96..1684e478c0 100644 --- a/src/Core/Auth/Services/Implementations/OpaqueKeyExchangeService.cs +++ b/src/Core/Auth/Services/Implementations/OpaqueKeyExchangeService.cs @@ -1,4 +1,5 @@ -using System.Text; +using System.Security.Cryptography; +using System.Text; using System.Text.Json; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Opaque; @@ -92,11 +93,13 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService var serverRegistration = credentialBlob.PasswordFile; var loginResponse = _bitwardenOpaque.StartLogin(cipherConfiguration.ToNativeConfiguration(), serverSetup, serverRegistration, request, user.Id.ToString()); - var sessionId = Guid.NewGuid(); - var loginSession = new OpaqueKeyExchangeLoginSession() { + var sessionId = MakeCryptoGuid(); + var loginSession = new OpaqueKeyExchangeLoginSession() + { UserId = user.Id, LoginState = loginResponse.state, - CipherConfiguration = cipherConfiguration + CipherConfiguration = cipherConfiguration, + IsAuthenticated = false }; await _distributedCache.SetAsync(string.Format(LOGIN_SESSION_KEY, sessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(loginSession))); return (sessionId, loginResponse.credentialResponse); @@ -124,6 +127,8 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService try { var success = _bitwardenOpaque.FinishLogin(cipherConfiguration.ToNativeConfiguration(), loginState, credentialFinalization); + loginSession.IsAuthenticated = true; + await _distributedCache.SetAsync(string.Format(LOGIN_SESSION_KEY, sessionId), Encoding.ASCII.GetBytes(JsonSerializer.Serialize(loginSession))); return true; } catch (Exception e) @@ -134,7 +139,24 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService } } - public async Task SetActive(Guid sessionId, User user) + public async Task GetUserForAuthenticatedSession(Guid sessionId) + { + var serializedLoginSession = await _distributedCache.GetAsync(string.Format(LOGIN_SESSION_KEY, sessionId)); + if (serializedLoginSession == null) + { + throw new InvalidOperationException("Session not found"); + } + var loginSession = JsonSerializer.Deserialize(Encoding.ASCII.GetString(serializedLoginSession))!; + + if (!loginSession.IsAuthenticated) + { + throw new InvalidOperationException("Session not authenticated"); + } + + return await _userRepository.GetByIdAsync(loginSession.UserId!)!; + } + + public async Task SetRegistrationActiveForAccount(Guid sessionId, User user) { var serializedRegisterSession = await _distributedCache.GetAsync(string.Format(REGISTER_SESSION_KEY, sessionId)); if (serializedRegisterSession == null) @@ -186,6 +208,23 @@ public class OpaqueKeyExchangeService : IOpaqueKeyExchangeService await _opaqueKeyExchangeCredentialRepository.DeleteAsync(credential); } } + + private static Guid MakeCryptoGuid() + { + // Get 16 cryptographically random bytes + byte[] data = RandomNumberGenerator.GetBytes(16); + + // Mark it as a version 4 GUID + data[7] = (byte)((data[7] | (byte)0x40) & (byte)0x4f); + data[8] = (byte)((data[8] | (byte)0x80) & (byte)0xbf); + + return new Guid(data); + } + + public async Task ClearAuthenticationSession(Guid sessionId) + { + await _distributedCache.RemoveAsync(string.Format(LOGIN_SESSION_KEY, sessionId)); + } } public class OpaqueKeyExchangeRegisterSession @@ -203,4 +242,5 @@ public class OpaqueKeyExchangeLoginSession public required Guid UserId { get; set; } public required byte[] LoginState { get; set; } public required OpaqueKeyExchangeCipherConfiguration CipherConfiguration { get; set; } + public required bool IsAuthenticated { get; set; } } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 6793b982eb..10a5d21429 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -670,7 +670,7 @@ public class UserService : UserManager, IUserService, IDisposable if (opaqueSessionId != null) { - await _opaqueKeyExchangeService.SetActive((Guid)opaqueSessionId, user); + await _opaqueKeyExchangeService.SetRegistrationActiveForAccount((Guid)opaqueSessionId, user); } else { diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 6e66ecef6a..0bf8e35d49 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,8 +1,8 @@ using System.Diagnostics; using System.Text; +using System.Text.Json; using Bit.Api.Auth.Models.Request.Opaque; using Bit.Api.Auth.Models.Response.Opaque; -using System.Text.Json; using Bit.Core; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; @@ -267,7 +267,7 @@ public class AccountsController : Controller } var credential = await _opaqueKeyExchangeCredentialRepository.GetByUserIdAsync(user.Id); - if (credential != null) + if (credential != null && _featureService.IsEnabled(FeatureFlagKeys.OpaqueKeyExchange)) { return new PreloginResponseModel(kdfInformation, JsonSerializer.Deserialize(credential.CipherConfiguration)!); } diff --git a/src/Identity/IdentityServer/ApiClient.cs b/src/Identity/IdentityServer/ApiClient.cs index 5d768ae806..5b9686ea02 100644 --- a/src/Identity/IdentityServer/ApiClient.cs +++ b/src/Identity/IdentityServer/ApiClient.cs @@ -14,7 +14,7 @@ public class ApiClient : Client string[] scopes = null) { ClientId = id; - AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode, WebAuthnGrantValidator.GrantType }; + AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode, WebAuthnGrantValidator.GrantType, OpaqueKeyExchangeGrantValidator.GrantType }; RefreshTokenExpiration = TokenExpiration.Sliding; RefreshTokenUsage = TokenUsage.ReUse; SlidingRefreshTokenLifetime = 86400 * refreshTokenSlidingDays; diff --git a/src/Identity/IdentityServer/RequestValidators/OpaqueKeyExchangeGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/OpaqueKeyExchangeGrantValidator.cs new file mode 100644 index 0000000000..6a3cd62f22 --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/OpaqueKeyExchangeGrantValidator.cs @@ -0,0 +1,144 @@ +using System.Security.Claims; +using Bit.Core; +using Bit.Core.AdminConsole.Services; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; +using Bit.Core.Auth.Services; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Core.Tokens; +using Duende.IdentityServer.Models; +using Duende.IdentityServer.Validation; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Identity.IdentityServer.RequestValidators; + +public class OpaqueKeyExchangeGrantValidator : BaseRequestValidator, IExtensionGrantValidator +{ + public const string GrantType = "opaque-ke"; + private IUserRepository userRepository; + private IOpaqueKeyExchangeService opaqueKeyExchangeService; + + public OpaqueKeyExchangeGrantValidator( + UserManager userManager, + IUserService userService, + IEventService eventService, + IDeviceValidator deviceValidator, + ITwoFactorAuthenticationValidator twoFactorAuthenticationValidator, + IOrganizationUserRepository organizationUserRepository, + IMailService mailService, + ILogger logger, + ICurrentContext currentContext, + GlobalSettings globalSettings, + ISsoConfigRepository ssoConfigRepository, + IUserRepository userRepository, + IPolicyService policyService, + IDataProtectorTokenFactory assertionOptionsDataProtector, + IFeatureService featureService, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder, + IOpaqueKeyExchangeService opaqueKeyExchangeService) + : base( + userManager, + userService, + eventService, + deviceValidator, + twoFactorAuthenticationValidator, + organizationUserRepository, + mailService, + logger, + currentContext, + globalSettings, + userRepository, + policyService, + featureService, + ssoConfigRepository, + userDecryptionOptionsBuilder) + { + this.userRepository = userRepository; + this.opaqueKeyExchangeService = opaqueKeyExchangeService; + } + + string IExtensionGrantValidator.GrantType => "opaque-ke"; + + public async Task ValidateAsync(ExtensionGrantValidationContext context) + { + var sessionId = context.Request.Raw.Get("sessionId"); + if (string.IsNullOrWhiteSpace(sessionId)) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); + return; + } + + var user = await opaqueKeyExchangeService.GetUserForAuthenticatedSession(Guid.Parse(sessionId)); + if (user == null) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); + return; + } + + await ValidateAsync(context, context.Request, new CustomValidatorRequestContext { User = user }); + } + + protected override Task ValidateContextAsync(ExtensionGrantValidationContext context, + CustomValidatorRequestContext validatorContext) + { + if (validatorContext.User == null) + { + return Task.FromResult(false); + } + + return Task.FromResult(true); + } + + protected override async Task SetSuccessResult(ExtensionGrantValidationContext context, User user, + List claims, Dictionary customResponse) + { + context.Result = new GrantValidationResult(user.Id.ToString(), "Application", + identityProvider: Constants.IdentityProvider, + claims: claims.Count > 0 ? claims : null, + customResponse: customResponse); + await opaqueKeyExchangeService.ClearAuthenticationSession(Guid.Parse(context.Request.Raw.Get("sessionId"))); + } + + protected override ClaimsPrincipal GetSubject(ExtensionGrantValidationContext context) + { + return context.Result.Subject; + } + + [Obsolete("Consider using SetValidationErrorResult instead.")] + protected override void SetTwoFactorResult(ExtensionGrantValidationContext context, + Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Two factor required.", + customResponse); + } + + [Obsolete("Consider using SetValidationErrorResult instead.")] + protected override void SetSsoResult(ExtensionGrantValidationContext context, + Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", + customResponse); + } + + [Obsolete("Consider using SetValidationErrorResult instead.")] + protected override void SetErrorResult(ExtensionGrantValidationContext context, Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, customResponse: customResponse); + } + + protected override void SetValidationErrorResult( + ExtensionGrantValidationContext context, CustomValidatorRequestContext requestContext) + { + context.Result = new GrantValidationResult + { + Error = requestContext.ValidationErrorResult.Error, + ErrorDescription = requestContext.ValidationErrorResult.ErrorDescription, + IsError = true, + CustomResponse = requestContext.CustomResponse + }; + } +} diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index 36c38615a2..d39d6b07f5 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -53,7 +53,8 @@ public static class ServiceCollectionExtensions .AddResourceOwnerValidator() .AddClientStore() .AddIdentityServerCertificate(env, globalSettings) - .AddExtensionGrantValidator(); + .AddExtensionGrantValidator() + .AddExtensionGrantValidator(); if (CoreHelpers.SettingHasValue(globalSettings.IdentityServer.CosmosConnectionString)) { diff --git a/test/Identity.Test/Controllers/AccountsControllerTests.cs b/test/Identity.Test/Controllers/AccountsControllerTests.cs index 4928d08316..ade07ce049 100644 --- a/test/Identity.Test/Controllers/AccountsControllerTests.cs +++ b/test/Identity.Test/Controllers/AccountsControllerTests.cs @@ -47,6 +47,7 @@ public class AccountsControllerTests : IDisposable private readonly IFeatureService _featureService; private readonly IDataProtectorTokenFactory _registrationEmailVerificationTokenDataFactory; private readonly IOpaqueKeyExchangeCredentialRepository _opaqueKeyExchangeCredentialRepository; + private readonly IOpaqueKeyExchangeService _opaqueKeyExchangeService; private readonly GlobalSettings _globalSettings; @@ -64,6 +65,7 @@ public class AccountsControllerTests : IDisposable _featureService = Substitute.For(); _registrationEmailVerificationTokenDataFactory = Substitute.For>(); _opaqueKeyExchangeCredentialRepository = Substitute.For(); + _opaqueKeyExchangeService = Substitute.For(); _globalSettings = Substitute.For(); _sut = new AccountsController( @@ -79,6 +81,7 @@ public class AccountsControllerTests : IDisposable _featureService, _registrationEmailVerificationTokenDataFactory, _opaqueKeyExchangeCredentialRepository, + _opaqueKeyExchangeService, _globalSettings ); }