diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs
index fda4c23dc0..9bcdedca70 100644
--- a/src/Umbraco.Core/Constants-Security.cs
+++ b/src/Umbraco.Core/Constants-Security.cs
@@ -62,6 +62,7 @@
public const string AspNetCoreV3PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V3";
public const string AspNetCoreV2PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V2";
public const string AspNetUmbraco8PasswordHashAlgorithmName = "HMACSHA256";
+ public const string AspNetUmbraco4PasswordHashAlgorithmName = "HMACSHA1";
}
}
}
diff --git a/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs b/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs
index 9b14c3ccba..dd28afbba0 100644
--- a/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs
+++ b/src/Umbraco.Core/Security/LegacyPasswordSecurity.cs
@@ -11,9 +11,6 @@ namespace Umbraco.Core.Security
///
public class LegacyPasswordSecurity
{
- // TODO: This class no longer has the logic available to verify the old old old password format, we should
- // include this ability so that upgrades for very old versions/data can work and then auto-migrate to the new password format.
-
private readonly IPasswordConfiguration _passwordConfiguration;
private readonly PasswordGenerator _generator;
@@ -34,13 +31,14 @@ namespace Umbraco.Core.Security
///
///
///
+ // TODO: Do we need this method? We shouldn't be using this class to create new password hashes for storage
public string HashPasswordForStorage(string password)
{
if (string.IsNullOrWhiteSpace(password))
throw new ArgumentException("password cannot be empty", nameof(password));
string salt;
- var hashed = HashNewPassword(password, out salt);
+ var hashed = HashNewPassword(_passwordConfiguration.HashAlgorithmType, password, out salt);
return FormatPasswordForStorage(hashed, salt);
}
@@ -51,6 +49,7 @@ namespace Umbraco.Core.Security
///
///
///
+ // TODO: Do we need this method? We shouldn't be using this class to create new password hashes for storage
public string FormatPasswordForStorage(string hashedPassword, string salt)
{
return salt + hashedPassword;
@@ -59,18 +58,24 @@ namespace Umbraco.Core.Security
///
/// Hashes a password with a given salt
///
+ /// The hashing algorithm for the password.
///
///
///
- public string HashPassword(string pass, string salt)
+ public string HashPassword(string algorithmType, string pass, string salt)
{
+ if (IsLegacySHA1Algorithm(algorithmType))
+ {
+ return HashLegacySHA1Password(pass);
+ }
+
//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);
byte[] inArray;
- var hashAlgorithm = GetCurrentHashAlgorithm();
+ var hashAlgorithm = GetHashAlgorithm(algorithmType);
var algorithm = hashAlgorithm as KeyedHashAlgorithm;
if (algorithm != null)
{
@@ -116,43 +121,55 @@ namespace Umbraco.Core.Security
///
/// Verifies if the password matches the expected hash+salt of the stored password string
///
+ /// The hashing algorithm for the stored password.
/// The password.
/// The value of the password stored in a data store.
///
- public bool VerifyPassword(string password, string dbPassword)
+ 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 (dbPassword.StartsWith(Constants.Security.EmptyPasswordPrefix))
return false;
- var storedHashedPass = ParseStoredHashPassword(dbPassword, out var salt);
- var hashed = HashPassword(password, salt);
+ var storedHashedPass = ParseStoredHashPassword(algorithm, dbPassword, out var salt);
+ var hashed = HashPassword(algorithm, password, salt);
return storedHashedPass == hashed;
}
///
/// Create a new password hash and a new salt
///
+ /// The hashing algorithm for the password.
///
///
///
- public string HashNewPassword(string newPassword, out string salt)
+ // TODO: Do we need this method? We shouldn't be using this class to create new password hashes for storage
+ public string HashNewPassword(string algorithm, string newPassword, out string salt)
{
salt = GenerateSalt();
- return HashPassword(newPassword, salt);
+ return HashPassword(algorithm, newPassword, salt);
}
///
/// Parses out the hashed password and the salt from the stored password string value
///
+ /// The hashing algorithm for the stored password.
///
/// returns the salt
///
- public string ParseStoredHashPassword(string storedString, out string salt)
+ 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))
+ {
+ salt = string.Empty;
+ return storedString;
+ }
+
+
var saltLen = GenerateSalt();
salt = storedString.Substring(0, saltLen.Length);
return storedString.Substring(saltLen.Length);
@@ -168,28 +185,61 @@ namespace Umbraco.Core.Security
///
/// Return the hash algorithm to use based on the
///
+ /// The hashing algorithm name.
///
///
- private HashAlgorithm GetCurrentHashAlgorithm()
+ private HashAlgorithm GetHashAlgorithm(string algorithm)
{
- if (_passwordConfiguration.HashAlgorithmType.IsNullOrWhiteSpace())
+ if (algorithm.IsNullOrWhiteSpace())
throw new InvalidOperationException("No hash algorithm type specified");
- var alg = HashAlgorithm.Create(_passwordConfiguration.HashAlgorithmType);
+ var alg = HashAlgorithm.Create(algorithm);
if (alg == null)
- throw new InvalidOperationException($"The hash algorithm specified {_passwordConfiguration.HashAlgorithmType} cannot be resolved");
+ throw new InvalidOperationException($"The hash algorithm specified {algorithm} cannot be resolved");
return alg;
}
public bool SupportHashAlgorithm(string algorithm)
{
- if (algorithm.InvariantEquals(typeof(HMACSHA256).Name))
+ // This is for the v6-v8 hashing algorithm
+ if (algorithm.InvariantEquals(Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName))
+ return true;
+
+ // This is for the <= v4 hashing algorithm
+ if (IsLegacySHA1Algorithm(algorithm))
return true;
- // TODO: Need to add the old old old format in here too which was just HMACSHA1 IIRC but had a custom key implementation as the password itself
return false;
}
+ private bool IsLegacySHA1Algorithm(string algorithm) => algorithm.InvariantEquals(Constants.Security.AspNetUmbraco4PasswordHashAlgorithmName);
+
+ ///
+ /// Hashes the password with the old v4 algorithm
+ ///
+ /// The password.
+ /// The encoded password.
+ private string HashLegacySHA1Password(string password)
+ {
+ var hashAlgorithm = GetLegacySHA1Algorithm(password);
+ var hash = Convert.ToBase64String(hashAlgorithm.ComputeHash(Encoding.Unicode.GetBytes(password)));
+ return hash;
+ }
+
+ ///
+ /// Returns the old v4 algorithm and settings
+ ///
+ ///
+ ///
+ private HashAlgorithm GetLegacySHA1Algorithm(string password)
+ {
+ return new HMACSHA1
+ {
+ //the legacy salt was actually the password :(
+ Key = Encoding.Unicode.GetBytes(password)
+ };
+ }
+
}
}
diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs
index 0ef6063910..4b5c084b01 100644
--- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs
+++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/PasswordSecurityTests.cs
@@ -14,14 +14,15 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security
[Test]
public void Check_Password_Hashed_Non_KeyedHashAlgorithm()
{
- var passwordSecurity = new LegacyPasswordSecurity(Mock.Of(x => x.HashAlgorithmType == "SHA256"));
+ var passwordConfiguration = Mock.Of(x => x.HashAlgorithmType == "SHA256");
+ var passwordSecurity = new LegacyPasswordSecurity(passwordConfiguration);
string salt;
var pass = "ThisIsAHashedPassword";
- var hashed = passwordSecurity.HashNewPassword(pass, out salt);
+ var hashed = passwordSecurity.HashNewPassword(passwordConfiguration.HashAlgorithmType, pass, out salt);
var storedPassword = passwordSecurity.FormatPasswordForStorage(hashed, salt);
- var result = passwordSecurity.VerifyPassword("ThisIsAHashedPassword", storedPassword);
+ var result = passwordSecurity.VerifyPassword(passwordConfiguration.HashAlgorithmType, "ThisIsAHashedPassword", storedPassword);
Assert.IsTrue(result);
}
@@ -29,14 +30,15 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security
[Test]
public void Check_Password_Hashed_KeyedHashAlgorithm()
{
- var passwordSecurity = new LegacyPasswordSecurity(Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName));
+ var passwordConfiguration = Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName);
+ var passwordSecurity = new LegacyPasswordSecurity(passwordConfiguration);
string salt;
var pass = "ThisIsAHashedPassword";
- var hashed = passwordSecurity.HashNewPassword(pass, out salt);
+ var hashed = passwordSecurity.HashNewPassword(passwordConfiguration.HashAlgorithmType, pass, out salt);
var storedPassword = passwordSecurity.FormatPasswordForStorage(hashed, salt);
- var result = passwordSecurity.VerifyPassword("ThisIsAHashedPassword", storedPassword);
+ var result = passwordSecurity.VerifyPassword(passwordConfiguration.HashAlgorithmType, "ThisIsAHashedPassword", storedPassword);
Assert.IsTrue(result);
}
@@ -57,13 +59,14 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security
[Test]
public void Get_Stored_Password_Hashed()
{
- var passwordSecurity = new LegacyPasswordSecurity(Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName));
+ var passwordConfiguration = Mock.Of(x => x.HashAlgorithmType == Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName);
+ var passwordSecurity = new LegacyPasswordSecurity(passwordConfiguration);
var salt = LegacyPasswordSecurity.GenerateSalt();
var stored = salt + "ThisIsAHashedPassword";
string initSalt;
- var result = passwordSecurity.ParseStoredHashPassword(stored, out initSalt);
+ var result = passwordSecurity.ParseStoredHashPassword(passwordConfiguration.HashAlgorithmType, stored, out initSalt);
Assert.AreEqual("ThisIsAHashedPassword", result);
}
diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs
index 4f2cb22b4a..e54eb97c67 100644
--- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs
+++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs
@@ -106,8 +106,8 @@ namespace Umbraco.Tests.Membership
Assert.AreNotEqual("test", createdMember.RawPasswordValue);
string salt;
- var storedPassword = provider.PasswordSecurity.ParseStoredHashPassword(createdMember.RawPasswordValue, out salt);
- var hashedPassword = provider.PasswordSecurity.HashPassword("testtest$1", salt);
+ var storedPassword = provider.PasswordSecurity.ParseStoredHashPassword(Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName, createdMember.RawPasswordValue, out salt);
+ var hashedPassword = provider.PasswordSecurity.HashPassword(Constants.Security.AspNetUmbraco8PasswordHashAlgorithmName, "testtest$1", salt);
Assert.AreEqual(hashedPassword, storedPassword);
}
diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs
index df3bc2935b..4ec3947263 100644
--- a/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs
+++ b/src/Umbraco.Web.BackOffice/Security/BackOfficePasswordHasher.cs
@@ -13,12 +13,12 @@ namespace Umbraco.Web.BackOffice.Security
///
public class BackOfficePasswordHasher : PasswordHasher
{
- private readonly LegacyPasswordSecurity _passwordSecurity;
+ private readonly LegacyPasswordSecurity _legacyPasswordSecurity;
private readonly IJsonSerializer _jsonSerializer;
public BackOfficePasswordHasher(LegacyPasswordSecurity passwordSecurity, IJsonSerializer jsonSerializer)
{
- _passwordSecurity = passwordSecurity;
+ _legacyPasswordSecurity = passwordSecurity;
_jsonSerializer = jsonSerializer;
}
@@ -27,9 +27,12 @@ namespace Umbraco.Web.BackOffice.Security
if (!user.PasswordConfig.IsNullOrWhiteSpace())
{
// check if the (legacy) password security supports this hash algorith and if so then use it
+ // TODO: I don't think we really want to do this because we want to migrate all old password formats
+ // to the new format, not keep using it. AFAIK this block should be removed along with the code within the
+ // LegacyPasswordSecurity except the part that just validates old ones.
var deserialized = _jsonSerializer.Deserialize(user.PasswordConfig);
- if (_passwordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
- return _passwordSecurity.HashPasswordForStorage(password);
+ if (_legacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
+ return _legacyPasswordSecurity.HashPasswordForStorage(password);
// We will explicitly detect names here since this allows us to future proof these checks.
// The default is PBKDF2.ASPNETCORE.V3:
@@ -58,9 +61,9 @@ namespace Umbraco.Web.BackOffice.Security
{
// check if the (legacy) password security supports this hash algorith and if so then use it
var deserialized = _jsonSerializer.Deserialize(user.PasswordConfig);
- if (_passwordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
+ if (_legacyPasswordSecurity.SupportHashAlgorithm(deserialized.HashAlgorithm))
{
- var result = _passwordSecurity.VerifyPassword(providedPassword, hashedPassword);
+ var result = _legacyPasswordSecurity.VerifyPassword(deserialized.HashAlgorithm, providedPassword, hashedPassword);
return result
? PasswordVerificationResult.Success
: PasswordVerificationResult.Failed;
diff --git a/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs b/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs
index 65d7836d42..2f5e858687 100644
--- a/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs
+++ b/src/Umbraco.Web/Security/BackOfficeOwinUserManager.cs
@@ -7,15 +7,14 @@ using Microsoft.Extensions.Options;
using Microsoft.Owin.Security.DataProtection;
using Umbraco.Core;
using Umbraco.Core.BackOffice;
-using Umbraco.Core.Configuration;
using Umbraco.Core.Configuration.Models;
using Umbraco.Core.Mapping;
-using Umbraco.Core.Security;
using Umbraco.Core.Services;
using Umbraco.Net;
namespace Umbraco.Web.Security
{
+ // TODO: Most of this is already migrated to netcore, there's probably not much more to go and then we can complete remove it
public class BackOfficeOwinUserManager : BackOfficeUserManager
{
public const string OwinMarkerKey = "Umbraco.Web.Security.Identity.BackOfficeUserManagerMarker";
@@ -118,11 +117,6 @@ namespace Umbraco.Web.Security
#endregion
- protected override IPasswordHasher GetDefaultPasswordHasher(IPasswordConfiguration passwordConfiguration)
- {
- return new UserAwarePasswordHasher(new LegacyPasswordSecurity(passwordConfiguration));
- }
-
protected void InitUserManager(BackOfficeOwinUserManager manager, IDataProtectionProvider dataProtectionProvider)
{
// use a custom hasher based on our membership provider
diff --git a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs
index d059113e0a..87f5864cdf 100644
--- a/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs
+++ b/src/Umbraco.Web/Security/Providers/UmbracoMembershipProvider.cs
@@ -83,7 +83,7 @@ namespace Umbraco.Web.Security.Providers
if (m == null) return false;
string salt;
- var encodedPassword = PasswordSecurity.HashNewPassword(newPassword, out salt);
+ var encodedPassword = PasswordSecurity.HashNewPassword(Membership.HashAlgorithmType, newPassword, out salt);
m.RawPasswordValue = PasswordSecurity.FormatPasswordForStorage(encodedPassword, salt);
m.LastPasswordChangeDate = DateTime.Now;
@@ -143,7 +143,7 @@ namespace Umbraco.Web.Security.Providers
}
string salt;
- var encodedPassword = PasswordSecurity.HashNewPassword(password, out salt);
+ var encodedPassword = PasswordSecurity.HashNewPassword(Membership.HashAlgorithmType, password, out salt);
var member = MemberService.CreateWithIdentity(
username,
@@ -406,7 +406,7 @@ namespace Umbraco.Web.Security.Providers
}
string salt;
- var encodedPassword = PasswordSecurity.HashNewPassword(generatedPassword, out salt);
+ var encodedPassword = PasswordSecurity.HashNewPassword(Membership.HashAlgorithmType, generatedPassword, out salt);
m.RawPasswordValue = PasswordSecurity.FormatPasswordForStorage(encodedPassword, salt);
m.LastPasswordChangeDate = DateTime.Now;
MemberService.Save(m);
@@ -519,7 +519,7 @@ namespace Umbraco.Web.Security.Providers
};
}
- var authenticated = PasswordSecurity.VerifyPassword(password, member.RawPasswordValue);
+ var authenticated = PasswordSecurity.VerifyPassword(Membership.HashAlgorithmType, password, member.RawPasswordValue);
var requiresFullSave = false;
diff --git a/src/Umbraco.Web/Security/UserAwarePasswordHasher.cs b/src/Umbraco.Web/Security/UserAwarePasswordHasher.cs
deleted file mode 100644
index 040d79be02..0000000000
--- a/src/Umbraco.Web/Security/UserAwarePasswordHasher.cs
+++ /dev/null
@@ -1,40 +0,0 @@
-using Microsoft.AspNetCore.Identity;
-using Umbraco.Core.BackOffice;
-using Umbraco.Core.Security;
-
-namespace Umbraco.Web.Security
-{
- public class UserAwarePasswordHasher : IPasswordHasher
- where T : BackOfficeIdentityUser
- {
- private readonly LegacyPasswordSecurity _passwordSecurity;
-
- public UserAwarePasswordHasher(LegacyPasswordSecurity passwordSecurity)
- {
- _passwordSecurity = passwordSecurity;
- }
-
- public string HashPassword(string password)
- {
- return _passwordSecurity.HashPasswordForStorage(password);
- }
-
- public string HashPassword(T user, string password)
- {
- // TODO: Implement the logic for this, we need to lookup the password format for the user and hash accordingly: http://issues.umbraco.org/issue/U4-10089
- //NOTE: For now this just falls back to the hashing we are currently using
-
- return HashPassword(password);
- }
-
- public PasswordVerificationResult VerifyHashedPassword(T user, string hashedPassword, string providedPassword)
- {
- // TODO: Implement the logic for this, we need to lookup the password format for the user and hash accordingly: http://issues.umbraco.org/issue/U4-10089
- //NOTE: For now this just falls back to the hashing we are currently using
-
- return _passwordSecurity.VerifyPassword(providedPassword, hashedPassword)
- ? PasswordVerificationResult.Success
- : PasswordVerificationResult.Failed;
- }
- }
-}
diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj
index af9f9f76f6..c015e0edee 100644
--- a/src/Umbraco.Web/Umbraco.Web.csproj
+++ b/src/Umbraco.Web/Umbraco.Web.csproj
@@ -194,7 +194,6 @@
-