yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Further enhancements for legacy password support. (#12124)

Browse Source 
* Further enhancements for legacy password support.

For users - try new style passwords first and fallback on failure seeing
as a valid modern password is the norm, rehash is only one time.

For both users and members also deals with the fact that for
useLegacyEncoding we could store any old thing in passwordConfig
e.g. it's possible to get Umbraco8 to store "HMACSHA384" alongside
the hash even though it's really HMACSHA1 with password used as key
(try it out by tweaking machine key settings and setting
useLegacyEncoding=true).

Has behavioral breaking changes in LegacyPasswordSecurity as the
code now expects consumers to to respect IsSupportedHashAlgorithm
rather than ignoring it.

* Less rushed removals
This commit is contained in:
Paul Johnson
2022-03-11 10:41:04 +00:00
committed by GitHub
parent cc05428640
commit ff2865c1dc
5 changed files with 146 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
src/Umbraco.Core/Security/LegacyPasswordSecurity.cs
View File

@@ -8,12 +8,17 @@ namespace Umbraco.Cms.Core.Security
{
/// <summary>
/// Handles password hashing and formatting for legacy hashing algorithms
/// Handles password hashing and formatting for legacy hashing algorithms.
/// </summary>
/// <remarks>
/// Should probably be internal.
/// </remarks>
public class LegacyPasswordSecurity
{
// TODO: Remove v11
// Used for tests
[EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("We shouldn't be altering our public API to make test code easier, removing v11")]
public string HashPasswordForStorage(string algorithmType, string password)
{
if (string.IsNullOrWhiteSpace(password))
@@ -24,13 +29,15 @@ namespace Umbraco.Cms.Core.Security
return FormatPasswordForStorage(algorithmType, hashed, salt);
}
// TODO: Remove v11
// Used for tests
[EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("We shouldn't be altering our public API to make test code easier, removing v11")]
public string FormatPasswordForStorage(string algorithmType, string hashedPassword, string salt)
{
if (IsLegacySHA1Algorithm(algorithmType))
if (!SupportHashAlgorithm(algorithmType))
{
return hashedPassword;
throw new InvalidOperationException($"{algorithmType} is not supported");
}
return salt + hashedPassword;
@@ -45,10 +52,15 @@ namespace Umbraco.Cms.Core.Security
/// <returns></returns>
public bool VerifyPassword(string algorithm, string password, string dbPassword)
{
if (string.IsNullOrWhiteSpace(dbPassword)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(dbPassword));
if (string.IsNullOrWhiteSpace(dbPassword))
{
throw new ArgumentException("Value cannot be null or whitespace.", nameof(dbPassword));
}
if (dbPassword.StartsWith(Constants.Security.EmptyPasswordPrefix))
{
return false;
}
try
{
@@ -61,7 +73,6 @@ namespace Umbraco.Cms.Core.Security
//This can happen if the length of the password is wrong and a salt cannot be extracted.
return false;
}
}
/// <summary>
@@ -69,12 +80,13 @@ namespace Umbraco.Cms.Core.Security
/// </summary>
public bool VerifyLegacyHashedPassword(string password, string dbPassword)
{
var hashAlgorith = new HMACSHA1
var hashAlgorithm = new HMACSHA1
{
//the legacy salt was actually the password :(
Key = Encoding.Unicode.GetBytes(password)
};
var hashed = Convert.ToBase64String(hashAlgorith.ComputeHash(Encoding.Unicode.GetBytes(password)));
var hashed = Convert.ToBase64String(hashAlgorithm.ComputeHash(Encoding.Unicode.GetBytes(password)));
return dbPassword == hashed;
}
@@ -87,6 +99,8 @@ namespace Umbraco.Cms.Core.Security
/// <param name="salt"></param>
/// <returns></returns>
// TODO: Do we need this method? We shouldn't be using this class to create new password hashes for storage
// TODO: Remove v11
[Obsolete("We shouldn't be altering our public API to make test code easier, removing v11")]
public string HashNewPassword(string algorithm, string newPassword, out string salt)
{
salt = GenerateSalt();
@@ -102,15 +116,15 @@ namespace Umbraco.Cms.Core.Security
/// <returns></returns>
public string ParseStoredHashPassword(string algorithm, string storedString, out string salt)
{
if (string.IsNullOrWhiteSpace(storedString)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(storedString));
// This is for the <= v4 hashing algorithm for which there was no salt
if (IsLegacySHA1Algorithm(algorithm))
if (string.IsNullOrWhiteSpace(storedString))
{
salt = string.Empty;
return storedString;
throw new ArgumentException("Value cannot be null or whitespace.", nameof(storedString));
}
if (!SupportHashAlgorithm(algorithm))
{
throw new InvalidOperationException($"{algorithm} is not supported");
}
var saltLen = GenerateSalt();
salt = storedString.Substring(0, saltLen.Length);
@@ -133,12 +147,12 @@ namespace Umbraco.Cms.Core.Security
/// <returns></returns>
private string HashPassword(string algorithmType, string pass, string salt)
{
if (IsLegacySHA1Algorithm(algorithmType))
if (!SupportHashAlgorithm(algorithmType))
{
return HashLegacySHA1Password(pass);
throw new InvalidOperationException($"{algorithmType} is not supported");
}
//This is the correct way to implement this (as per the sql membership provider)
// This is the correct way to implement this (as per the sql membership provider)
var bytes = Encoding.Unicode.GetBytes(pass);
var saltBytes = Convert.FromBase64String(salt);
@@ -219,42 +233,7 @@ namespace Umbraco.Cms.Core.Security
return true;
}
// This is for the <= v4 hashing algorithm
if (IsLegacySHA1Algorithm(algorithm))
{
return true;
}
return false;
}
private bool IsLegacySHA1Algorithm(string algorithm) => algorithm.InvariantEquals(Constants.Security.AspNetUmbraco4PasswordHashAlgorithmName);
/// <summary>
/// Hashes the password with the old v4 algorithm
/// </summary>
/// <param name="password">The password.</param>
/// <returns>The encoded password.</returns>
private string HashLegacySHA1Password(string password)
{
using var hashAlgorithm = GetLegacySHA1Algorithm(password);
var hash = Convert.ToBase64String(hashAlgorithm.ComputeHash(Encoding.Unicode.GetBytes(password)));
return hash;
}
/// <summary>
/// Returns the old v4 algorithm and settings
/// </summary>
/// <param name="password"></param>
/// <returns></returns>
private HashAlgorithm GetLegacySHA1Algorithm(string password)
{
return new HMACSHA1
{
//the legacy salt was actually the password :(
Key = Encoding.Unicode.GetBytes(password)
};
}
}
}

3
src/Umbraco.Infrastructure/Security/MemberPasswordHasher.cs
View File

@@ -71,6 +71,7 @@ namespace Umbraco.Cms.Core.Security
return result;
}
}
// We need to check for clear text passwords from members as the first thing. This was possible in v8 :(
else if (IsSuccessfulLegacyPassword(hashedPassword, providedPassword))
{
@@ -138,7 +139,7 @@ namespace Umbraco.Cms.Core.Security
}
var result = LegacyPasswordSecurity.VerifyPassword(Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName, providedPassword, hashedPassword);
return result || LegacyPasswordSecurity.VerifyPassword(Constants.Security.AspNetUmbraco4PasswordHashAlgorithmName, providedPassword, hashedPassword);
return result || LegacyPasswordSecurity.VerifyLegacyHashedPassword(providedPassword, hashedPassword);
}
private static string DecryptLegacyPassword(string encryptedPassword, string algorithmName, string decryptionKey)

97
src/Umbraco.Infrastructure/Security/UmbracoPasswordHasher.cs
View File

@@ -1,3 +1,4 @@
using System;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Models.Membership;
@@ -10,7 +11,6 @@ namespace Umbraco.Cms.Core.Security
where TUser: UmbracoIdentityUser
{
private readonly IJsonSerializer _jsonSerializer;
private readonly PasswordHasher<TUser> _aspnetV2PasswordHasher = new PasswordHasher<TUser>(new V2PasswordHasherOptions());
public UmbracoPasswordHasher(LegacyPasswordSecurity legacyPasswordSecurity, IJsonSerializer jsonSerializer)
{
@@ -43,57 +43,64 @@ namespace Umbraco.Cms.Core.Security
{
if (user is null)
{
throw new System.ArgumentNullException(nameof(user));
throw new ArgumentNullException(nameof(user));
}
if (!user.PasswordConfig.IsNullOrWhiteSpace())
try
{
// check if the (legacy) password security supports this hash algorith and if so then use it
var deserialized = _jsonSerializer.Deserialize<PersistedPasswordSettings>(user.PasswordConfig);
if (LegacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
// Best case and most likely scenario, a modern hash supported by ASP.Net identity.
PasswordVerificationResult upstreamResult = base.VerifyHashedPassword(user, hashedPassword, providedPassword);
if (upstreamResult != PasswordVerificationResult.Failed)
{
var result = LegacyPasswordSecurity.VerifyPassword(deserialized.HashAlgorithm, providedPassword, hashedPassword);
//We need to special handle this case, apparently v8 still saves the user algorithm as {"hashAlgorithm":"HMACSHA256"}, when using legacy encoding and hasinging.
if (result == false)
{
result = LegacyPasswordSecurity.VerifyLegacyHashedPassword(providedPassword, hashedPassword);
}
return result
? PasswordVerificationResult.SuccessRehashNeeded
: PasswordVerificationResult.Failed;
}
// We will explicitly detect names here
// The default is PBKDF2.ASPNETCORE.V3:
// PBKDF2 with HMAC-SHA256, 128-bit salt, 256-bit subkey, 10000 iterations.
// The underlying class only lets us change 2 things which is the version: options.CompatibilityMode and the iteration count
// The PBKDF2.ASPNETCORE.V2 settings are:
// PBKDF2 with HMAC-SHA1, 128-bit salt, 256-bit subkey, 1000 iterations.
switch (deserialized.HashAlgorithm)
{
case Constants.Security.AspNetCoreV3PasswordHashAlgorithmName:
return base.VerifyHashedPassword(user, hashedPassword, providedPassword);
case Constants.Security.AspNetCoreV2PasswordHashAlgorithmName:
var legacyResult = _aspnetV2PasswordHasher.VerifyHashedPassword(user, hashedPassword, providedPassword);
if (legacyResult == PasswordVerificationResult.Success)
return PasswordVerificationResult.SuccessRehashNeeded;
return legacyResult;
return upstreamResult;
}
}
// else go the default (v3)
return base.VerifyHashedPassword(user, hashedPassword, providedPassword);
}
private class V2PasswordHasherOptions : IOptions<PasswordHasherOptions>
{
public PasswordHasherOptions Value => new PasswordHasherOptions
catch (FormatException)
{
CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2
};
// hash wasn't a valid base64 encoded string, MS concat the salt bytes and hash bytes and base 64 encode both together.
// We however historically base 64 encoded the salt bytes and hash bytes separately then concat the strings so we got 2 sets of padding.
// both salt bytes and hash bytes lengths were not evenly divisible by 3 hence 2 sets of padding.
// We could check upfront with TryFromBase64String, but not whilst we target netstandard 2.0
// so might as well just deal with the exception.
}
// At this point we either have a legacy password or a bad attempt.
// Check the supported worst case scenario, a "useLegacyEncoding" password - HMACSHA1 but with password used as key so not unique for users sharing same password
// This was the standard for v4.
// Do this first because with useLegacyEncoding the algorithm stored in the database is irrelevant.
if (LegacyPasswordSecurity.VerifyLegacyHashedPassword(providedPassword, hashedPassword))
{
return PasswordVerificationResult.SuccessRehashNeeded;
}
// For users we expect to know the historic algorithm.
// NOTE: MemberPasswordHasher subclasses this class to deal with the fact that PasswordConfig wasn't stored.
if (user.PasswordConfig.IsNullOrWhiteSpace())
{
return PasswordVerificationResult.Failed;
}
PersistedPasswordSettings deserialized;
try
{
deserialized = _jsonSerializer.Deserialize<PersistedPasswordSettings>(user.PasswordConfig);
}
catch
{
return PasswordVerificationResult.Failed;
}
if (!LegacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
{
return PasswordVerificationResult.Failed;
}
// Last chance must be HMACSHA256 or SHA1
return LegacyPasswordSecurity.VerifyPassword(deserialized.HashAlgorithm, providedPassword, hashedPassword)
? PasswordVerificationResult.SuccessRehashNeeded
: PasswordVerificationResult.Failed;
}
}
}

18
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Security/LegacyPasswordSecurityTests.cs
View File

@@ -1,10 +1,14 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System;
using System.Security.Cryptography;
using System.Text;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Security;
using Umbraco.Extensions;
using Constants = Umbraco.Cms.Core.Constants;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Security
@@ -15,7 +19,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Security
[Test]
public void Check_Password_Hashed_Non_KeyedHashAlgorithm()
{
IPasswordConfiguration passwordConfiguration = Mock.Of<IPasswordConfiguration>(x => x.HashAlgorithmType == "SHA256");
IPasswordConfiguration passwordConfiguration = Mock.Of<IPasswordConfiguration>(x => x.HashAlgorithmType == "SHA1");
var passwordSecurity = new LegacyPasswordSecurity();
var pass = "ThisIsAHashedPassword";
@@ -45,14 +49,12 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Security
[Test]
public void Check_Password_Legacy_v4_SHA1()
{
IPasswordConfiguration passwordConfiguration = Mock.Of<IPasswordConfiguration>(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco4PasswordHashAlgorithmName);
var passwordSecurity = new LegacyPasswordSecurity();
const string clearText = "ThisIsAHashedPassword";
var clearTextUnicodeBytes = Encoding.Unicode.GetBytes(clearText);
using var algorithm = new HMACSHA1(clearTextUnicodeBytes);
var dbPassword = Convert.ToBase64String(algorithm.ComputeHash(clearTextUnicodeBytes));
var pass = "ThisIsAHashedPassword";
var hashed = passwordSecurity.HashNewPassword(passwordConfiguration.HashAlgorithmType, pass, out string salt);
var storedPassword = passwordSecurity.FormatPasswordForStorage(passwordConfiguration.HashAlgorithmType, hashed, salt);
var result = passwordSecurity.VerifyPassword(passwordConfiguration.HashAlgorithmType, "ThisIsAHashedPassword", storedPassword);
var result = new LegacyPasswordSecurity().VerifyLegacyHashedPassword(clearText, dbPassword);
Assert.IsTrue(result);
}

58
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoPasswordHasherTests.cs
View File

@@ -1,5 +1,9 @@
using System;
using System.Security.Cryptography;
using System.Text;
using AutoFixture.NUnit3;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Models.Membership;
@@ -15,9 +19,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
// Technically MD5, HMACSHA384 & HMACSHA512 were also possible but opt in as opposed to historic defaults.
[Test]
[InlineAutoMoqData("HMACSHA256", "Umbraco9Rocks!", "uB/pLEhhe1W7EtWMv/pSgg==1y8+aso9+h3AKRtJXlVYeg2TZKJUr64hccj82ZZ7Ksk=")] // Actually HMACSHA256
[InlineAutoMoqData("HMACSHA256", "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")] // v4 site legacy password, with incorrect algorithm specified in database actually HMACSHA1 with password used as key.
[InlineAutoMoqData("SHA1", "Umbraco9Rocks!", "6tZGfG9NTxJJYp19Fac9og==zzRggqANxhb+CbD/VabEt8cIde8=")] // When SHA1 is set on machine key.
public void VerifyHashedPassword_WithValidLegacyPasswordHash_ReturnsSuccessRehashNeeded(
public void VerifyHashedPassword_ValidHashWithoutLegacyEncoding_ReturnsSuccessRehashNeeded(
string algorithm,
string providedPassword,
string hashedPassword,
@@ -34,10 +37,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
Assert.AreEqual(PasswordVerificationResult.SuccessRehashNeeded, result);
}
[Test]
[InlineAutoMoqData("PBKDF2.ASPNETCORE.V3", "Umbraco9Rocks!", "AQAAAAEAACcQAAAAEDCrYcnIhHKr38yuchsDu6AFqqmLNvRooKObV25GC1LC1tLY+gWGU4xNug0lc17PHA==")]
public void VerifyHashedPassword_WithValidModernPasswordHash_ReturnsSuccess(
[InlineAutoMoqData("HMACSHA1", "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")]
[InlineAutoMoqData("HMACSHA256", "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")]
[InlineAutoMoqData("FOOBARBAZQUX", "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")]
[InlineAutoMoqData("", "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")]
[InlineAutoMoqData(null, "Umbraco9Rocks!", "t0U8atXTX/efNCtTafukwZeIpr8=")]
public void VerifyHashedPassword_ValidHashWithLegacyEncoding_ReturnsSuccessRehashNeeded(
string algorithm,
string providedPassword,
string hashedPassword,
@@ -51,7 +57,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
var result = sut.VerifyHashedPassword(aUser, hashedPassword, providedPassword);
Assert.AreEqual(PasswordVerificationResult.Success, result);
Assert.AreEqual(PasswordVerificationResult.SuccessRehashNeeded, result);
}
[Test]
@@ -73,6 +79,46 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
Assert.AreEqual(PasswordVerificationResult.Failed, result);
}
[Test]
[AutoMoqData]
public void VerifyHashedPassword_WithIdentityV1OrV2StyleHash_ReturnsSuccessRehashNeeded(
TestUserStub aUser,
UmbracoPasswordHasher<TestUserStub> sut)
{
var options = Options.Create(new PasswordHasherOptions
{
CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2
});
var upstreamHasher = new PasswordHasher<TestUserStub>(options);
const string password = "Umbraco9Rocks!";
var identityV1Or2StyleHash = upstreamHasher.HashPassword(aUser, password);
var result = sut.VerifyHashedPassword(aUser, identityV1Or2StyleHash, password);
Assert.AreEqual(PasswordVerificationResult.SuccessRehashNeeded, result);
}
[Test]
[AutoMoqData]
public void VerifyHashedPassword_WithIdentityV3StyleHash_ReturnsSuccess(
TestUserStub aUser,
UmbracoPasswordHasher<TestUserStub> sut)
{
var options = Options.Create(new PasswordHasherOptions
{
CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV3
});
var upstreamHasher = new PasswordHasher<TestUserStub>(options);
const string password = "Umbraco9Rocks!";
var identityV1Or2StyleHash = upstreamHasher.HashPassword(aUser, password);
var result = sut.VerifyHashedPassword(aUser, identityV1Or2StyleHash, password);
Assert.AreEqual(PasswordVerificationResult.Success, result);
}
public class TestUserStub : UmbracoIdentityUser
{
public TestUserStub() => PasswordConfig = "not null or empty";