diff --git a/src/Business/Grand.Business.Common/Services/Security/EncryptionService.cs b/src/Business/Grand.Business.Common/Services/Security/EncryptionService.cs index 908816040..704ff8b79 100644 --- a/src/Business/Grand.Business.Common/Services/Security/EncryptionService.cs +++ b/src/Business/Grand.Business.Common/Services/Security/EncryptionService.cs @@ -159,6 +159,11 @@ public virtual bool PasswordHashNeedsUpgrade(PasswordFormat passwordFormat, stri return parts.Length != 5 || !int.TryParse(parts[2], out var iterations) || iterations < Iterations; } + public virtual bool IsHashedPassword(string storedPassword) + { + return IsPbkdf2Hash(storedPassword); + } + private static bool IsPbkdf2Hash(string storedPassword) { return storedPassword != null && storedPassword.StartsWith(Pbkdf2Prefix + "$", StringComparison.Ordinal); diff --git a/src/Business/Grand.Business.Core/Interfaces/Common/Security/IEncryptionService.cs b/src/Business/Grand.Business.Core/Interfaces/Common/Security/IEncryptionService.cs index b437ba3cb..a5888d3d0 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Common/Security/IEncryptionService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Common/Security/IEncryptionService.cs @@ -51,7 +51,17 @@ bool VerifyPassword(string enteredPassword, PasswordFormat passwordFormat, strin bool PasswordHashNeedsUpgrade(PasswordFormat passwordFormat, string storedPassword); /// - /// Encrypt text + /// Returns true when the stored value is the modern self-describing (PBKDF2) hash produced by + /// . Use it to pick the verification path for records whose password format is + /// not tracked in a separate field (e.g. API users), instead of inferring it from other columns. + /// + bool IsHashedPassword(string storedPassword); + + /// + /// Encrypt text (legacy TripleDES). Retained only to verify pre-existing reversibly-encrypted values + /// (legacy customer "Encrypted" password format and legacy API users). Do NOT use for new data or for + /// protecting secrets at rest - hash passwords with and use ASP.NET Core Data + /// Protection for reversible secrets. /// /// Text to encrypt /// Encryption private key @@ -59,7 +69,7 @@ bool VerifyPassword(string enteredPassword, PasswordFormat passwordFormat, strin string EncryptText(string plainText, string privateKey); /// - /// Decrypt text + /// Decrypt text (legacy TripleDES). Legacy counterpart of ; see its remarks. /// /// Text to decrypt /// Encryption private key diff --git a/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs b/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs new file mode 100644 index 000000000..6436707b9 --- /dev/null +++ b/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs @@ -0,0 +1,72 @@ +using Grand.Infrastructure; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace Grand.Module.Api.Infrastructure; + +/// +/// Fails fast at startup when an enabled API is configured with a missing, placeholder or too-short JWT signing +/// key. A weak/known key lets anyone forge valid tokens, so outside Development this aborts startup instead of +/// silently running with a forgeable secret. +/// +public class ApiSecurityStartup : IStartupApplication +{ + //shipped placeholder in appsettings.json - must never be used to sign real tokens + private const string PlaceholderSecret = "your private secret key to use api"; + + //HS256 needs a >= 256-bit key; the signing key is the raw ASCII bytes of the secret, so require >= 32 chars + private const int MinSecretLength = 32; + + public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment) + { + var logger = application.Services.GetRequiredService() + .CreateLogger(nameof(ApiSecurityStartup)); + + var backend = new BackendAPIConfig(); + application.Configuration.GetSection("BackendAPI").Bind(backend); + Validate("BackendAPI", backend.Enabled, backend.SecretKey, webHostEnvironment, logger); + + var frontend = new FrontendAPIConfig(); + application.Configuration.GetSection("FrontendAPI").Bind(frontend); + Validate("FrontendAPI", frontend.Enabled, frontend.SecretKey, webHostEnvironment, logger); + } + + private static void Validate(string section, bool enabled, string secret, IWebHostEnvironment env, ILogger logger) + { + if (!enabled) + return; + + if (IsStrong(secret)) + return; + + var message = + $"{section}.SecretKey is missing, still set to the placeholder, or shorter than {MinSecretLength} " + + "characters. Configure a strong, unique secret (e.g. via the environment variable " + + $"{section}__SecretKey or a secret store) - a weak key allows forging valid API tokens."; + + //in Development we only warn so local setups keep working; in every other environment we abort startup + if (env.IsDevelopment()) + logger.LogWarning("{Message}", message); + else + throw new InvalidOperationException(message); + } + + private static bool IsStrong(string secret) + { + return !string.IsNullOrWhiteSpace(secret) + && secret.Length >= MinSecretLength + && !string.Equals(secret, PlaceholderSecret, StringComparison.Ordinal); + } + + public void ConfigureServices(IServiceCollection services, IConfiguration configuration) + { + } + + public int Priority => 0; + public bool BeforeConfigure => true; +} diff --git a/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs b/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs index 68d4b2440..08ea7710c 100644 --- a/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs +++ b/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs @@ -30,7 +30,14 @@ public LoginValidator( { var base64EncodedBytes = Convert.FromBase64String(x.Password); var password = Encoding.UTF8.GetString(base64EncodedBytes); - if (userapi.Password == encryptionService.EncryptText(password, userapi.PrivateKey)) + //detect the format from the stored password itself: modern records are PBKDF2 hashes, legacy + //records are reversibly encrypted with the per-user PrivateKey (verified in constant time) + var isValid = encryptionService.IsHashedPassword(userapi.Password) + ? encryptionService.VerifyPassword(password, PasswordFormat.Hashed, userapi.Password, + string.Empty, HashedPasswordFormat.SHA1) + : !string.IsNullOrEmpty(userapi.PrivateKey) && encryptionService.VerifyPassword(password, + PasswordFormat.Encrypted, userapi.Password, userapi.PrivateKey, HashedPasswordFormat.SHA1); + if (isValid) return true; } } diff --git a/src/Modules/Grand.Module.Installer/Grand.Module.Installer.csproj b/src/Modules/Grand.Module.Installer/Grand.Module.Installer.csproj index 0c2aa8bfd..c23d05e9e 100644 --- a/src/Modules/Grand.Module.Installer/Grand.Module.Installer.csproj +++ b/src/Modules/Grand.Module.Installer/Grand.Module.Installer.csproj @@ -6,6 +6,9 @@ true + + False + False diff --git a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs index e7e0efa51..d82cc4dfd 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs @@ -1,3 +1,4 @@ +using Grand.Business.Core.Interfaces.Common.Security; using Grand.Data; using Grand.Domain; using Grand.Domain.Admin; @@ -436,33 +437,13 @@ protected virtual Task InstallMenuAdminSiteMap() protected virtual async Task HashDefaultCustomerPassword(string defaultUserEmail, string defaultUserPassword) { - var passwordSalt = CommonHelper.GenerateRandomDigitCode(24); - - var tDes = TripleDES.Create(); - - tDes.Key = new ASCIIEncoding().GetBytes(passwordSalt); - tDes.IV = new ASCIIEncoding().GetBytes(passwordSalt[^8..]); - - var encryptedBinary = EncryptTextToMemory(defaultUserPassword, tDes.Key, tDes.IV); - var password = Convert.ToBase64String(encryptedBinary); - var customer = _customerRepository.Table.FirstOrDefault(x=>x.Email == defaultUserEmail); - customer!.Password = password; - customer!.PasswordSalt = passwordSalt; - customer!.PasswordFormatId = PasswordFormat.Encrypted; + //create the default administrator with a modern PBKDF2 hash (salt/parameters embedded in the value) + var encryptionService = _serviceProvider.GetRequiredService(); + var customer = _customerRepository.Table.FirstOrDefault(x => x.Email == defaultUserEmail); + customer!.Password = encryptionService.HashPassword(defaultUserPassword); + customer!.PasswordSalt = string.Empty; + customer!.PasswordFormatId = PasswordFormat.Hashed; await _customerRepository.UpdateAsync(customer); - - } - private static byte[] EncryptTextToMemory(string data, byte[] key, byte[] iv) - { - using var ms = new MemoryStream(); - using (var cs = new CryptoStream(ms, TripleDES.Create().CreateEncryptor(key, iv), CryptoStreamMode.Write)) - { - var toEncrypt = new UnicodeEncoding().GetBytes(data); - cs.Write(toEncrypt, 0, toEncrypt.Length); - cs.FlushFinalBlock(); - } - - return ms.ToArray(); } private async Task CreateIndexes(IDatabaseContext dbContext, DataSettings dataSettings) diff --git a/src/Tests/Grand.Business.Common.Tests/Services/Security/EncryptionServiceTests.cs b/src/Tests/Grand.Business.Common.Tests/Services/Security/EncryptionServiceTests.cs index 0ce7b6cf5..ac1ddd91e 100644 --- a/src/Tests/Grand.Business.Common.Tests/Services/Security/EncryptionServiceTests.cs +++ b/src/Tests/Grand.Business.Common.Tests/Services/Security/EncryptionServiceTests.cs @@ -145,6 +145,16 @@ public void PasswordHashNeedsUpgrade_TrueWhenEmbeddedIterationsBelowConfigured() Assert.IsFalse(stronger.PasswordHashNeedsUpgrade(PasswordFormat.Hashed, stronger.HashPassword("password"))); } + [TestMethod] + public void IsHashedPassword_TrueOnlyForPbkdf2Format() + { + Assert.IsTrue(_encryptionService.IsHashedPassword(_encryptionService.HashPassword("password"))); + Assert.IsFalse(_encryptionService.IsHashedPassword( + _encryptionService.CreatePasswordHash("password", _encryptionService.CreateSaltKey(16)))); + Assert.IsFalse(_encryptionService.IsHashedPassword("some-reversibly-encrypted-value")); + Assert.IsFalse(_encryptionService.IsHashedPassword(null)); + } + [TestMethod] public void VerifyPassword_Pbkdf2_FailsClosedOnMalformedHash() { diff --git a/src/Web/Grand.Web.Admin/Controllers/ApiUserController.cs b/src/Web/Grand.Web.Admin/Controllers/ApiUserController.cs index e6c235c95..af031cc0e 100644 --- a/src/Web/Grand.Web.Admin/Controllers/ApiUserController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/ApiUserController.cs @@ -24,8 +24,8 @@ public ApiUserController(IUserApiService userApiService, IEncryptionService encr protected (string hashpassword, string privatekey) HashPassword(string password) { - var pk = CommonHelper.GenerateRandomDigitCode(24); - return (_encryptionService.EncryptText(password, pk), pk); + //store a one-way PBKDF2 hash instead of reversible encryption; PrivateKey is no longer used for new records + return (_encryptionService.HashPassword(password), string.Empty); } public IActionResult Index() diff --git a/src/Web/Grand.Web/App_Data/appsettings.json b/src/Web/Grand.Web/App_Data/appsettings.json index af0e5803f..43b956c35 100644 --- a/src/Web/Grand.Web/App_Data/appsettings.json +++ b/src/Web/Grand.Web/App_Data/appsettings.json @@ -198,7 +198,9 @@ "FrontendAPI": { "Enabled": true, "JsonContentType": false, - //when is enabled, use ContentType = application/json to read json from body, default is form-data + //when is enabled, use ContentType = application/json to read json from body, default is form-data + //REQUIRED: replace with a strong, unique secret (>= 32 chars) kept out of source control (env var FrontendAPI__SecretKey or a secret store). + //Outside Development the app refuses to start while this is empty/placeholder/too short - a weak key lets anyone forge API tokens. "SecretKey": "your private secret key to use api", "ValidateIssuer": false, "ValidIssuer": "", @@ -214,6 +216,8 @@ //locally: https://localhost:44350/scalar/v1 "BackendAPI": { "Enabled": true, + //REQUIRED: replace with a strong, unique secret (>= 32 chars) kept out of source control (env var BackendAPI__SecretKey or a secret store). + //Outside Development the app refuses to start while this is empty/placeholder/too short - a weak key lets anyone forge admin API tokens. "SecretKey": "your private secret key to use api", "ValidateIssuer": false, "ValidIssuer": "",