Stop storing passwords reversibly and fail fast on weak JWT secrets (audit #2 & #3)

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);

src/Business/Grand.Business.Core/Interfaces/Common/Security/IEncryptionService.cs

@@ -51,15 +51,25 @@ bool VerifyPassword(string enteredPassword, PasswordFormat passwordFormat, strin
     bool PasswordHashNeedsUpgrade(PasswordFormat passwordFormat, string storedPassword);
 
     /// <summary>
-    ///     Encrypt text
+    ///     Returns true when the stored value is the modern self-describing (PBKDF2) hash produced by
+    ///     <see cref="HashPassword" />. 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.
+    /// </summary>
+    bool IsHashedPassword(string storedPassword);
+
+    /// <summary>
+    ///     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 <see cref="HashPassword" /> and use ASP.NET Core Data
+    ///     Protection for reversible secrets.
     /// </summary>
     /// <param name="plainText">Text to encrypt</param>
     /// <param name="privateKey">Encryption private key</param>
     /// <returns>Encrypted text</returns>
     string EncryptText(string plainText, string privateKey);
 
     /// <summary>
-    ///     Decrypt text
+    ///     Decrypt text (legacy TripleDES). Legacy counterpart of <see cref="EncryptText" />; see its remarks.
     /// </summary>
     /// <param name="cipherText">Text to decrypt</param>
     /// <param name="encryptionPrivateKey">Encryption private key</param>

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;
+
+/// <summary>
+///     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.
+/// </summary>
+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<ILoggerFactory>()
+            .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;
+}

src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs

@@ -10,7 +10,7 @@
 
 public class LoginValidator : BaseGrandValidator<LoginModel>
 {
     public LoginValidator(
         IEnumerable<IValidatorConsumer<LoginModel>> validators,
         BackendAPIConfig apiConfig, ICustomerService customerService, IUserApiService userApiService,
         IEncryptionService encryptionService)
@@ -30,7 +30,14 @@
                 {
                     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;
                 }
             }

src/Modules/Grand.Module.Installer/Grand.Module.Installer.csproj

@@ -6,6 +6,9 @@
 		<AddRazorSupportForMvc>true</AddRazorSupportForMvc>
 	</PropertyGroup>
 	<ItemGroup>
+		<ProjectReference Include="..\..\Business\Grand.Business.Core\Grand.Business.Core.csproj">
+		  <Private>False</Private>
+		</ProjectReference>
 		<ProjectReference Include="..\..\Core\Grand.Data\Grand.Data.csproj">
 		  <Private>False</Private>
 		</ProjectReference>

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<IEncryptionService>();
+        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)

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()
     {

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()

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": "",