Skip to content

Commit

Permalink
New Salt&Hash algorithm for passwords (#3208)
Browse files Browse the repository at this point in the history 
New Salt&Hash algorithm for passwords
  • Loading branch information
@skofman1
skofman1 committed Sep 12, 2016
1 parent 1fbf37c commit a173a57
Show file tree
Hide file tree
Showing 30 changed files with 940 additions and 501 deletions.
11 changes: 6 additions & 5 deletions src/NuGetGallery.Core/CredentialTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ public static class CredentialTypes
{
public static class Password
{
public static readonly string Prefix = "password.";
public static readonly string Pbkdf2 = Prefix + "pbkdf2";
public static readonly string Sha1 = Prefix + "sha1";
public const string Prefix = "password.";
public const string Pbkdf2 = Prefix + "pbkdf2";
public const string Sha1 = Prefix + "sha1";
public const string V3 = Prefix + "v3";
}

public static readonly string ApiKeyV1 = "apikey.v1";
public static readonly string ExternalPrefix = "external.";
public const string ApiKeyV1 = "apikey.v1";
public const string ExternalPrefix = "external.";

public static bool IsPassword(string type)
{
Expand Down
4 changes: 3 additions & 1 deletion src/NuGetGallery.Core/NuGetGallery.Core.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@
<ItemGroup>
<None Include="app.config" />
<None Include="NuGetGallery.Core.nuspec" />
<None Include="packages.config" />
<None Include="packages.config">
<SubType>Designer</SubType>
</None>
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="Strings.resx">
Expand Down
4 changes: 3 additions & 1 deletion src/NuGetGallery.Operations/NuGetGallery.Operations.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,9 @@
<None Include="app.config">
<SubType>Designer</SubType>
</None>
<None Include="packages.config" />
<None Include="packages.config">
<SubType>Designer</SubType>
</None>
<None Include="Service References\SqlDac\Microsoft.SqlServer.Management.Dac.Services.wsdl" />
<None Include="Service References\SqlDac\Microsoft.SqlServer.Management.Dac.ServiceTypes.xsd">
<SubType>Designer</SubType>
Expand Down
4 changes: 4 additions & 0 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using NuGetGallery.Infrastructure.Lucene;
using NuGetGallery.Areas.Admin.Models;
using NuGetGallery.Configuration.SecretReader;
using NuGetGallery.Infrastructure.Authentication;

namespace NuGetGallery
{
Expand Down Expand Up @@ -55,6 +56,9 @@ protected override void Load(ContainerBuilder builder)
.AsSelf()
.As<FeatureConfiguration>();

builder.RegisterType<CredentialBuilder>().As<ICredentialBuilder>().SingleInstance();
builder.RegisterType<CredentialValidator>().As<ICredentialValidator>().SingleInstance();

builder.RegisterInstance(LuceneCommon.GetDirectory(configuration.Current.LuceneIndexLocation))
.As<Lucene.Net.Store.Directory>()
.SingleInstance();
Expand Down
132 changes: 81 additions & 51 deletions src/NuGetGallery/Authentication/AuthenticationService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,92 @@
using NuGetGallery.Authentication.Providers;
using NuGetGallery.Configuration;
using NuGetGallery.Diagnostics;
using NuGetGallery.Infrastructure.Authentication;

namespace NuGetGallery.Authentication
{
public class AuthenticationService
{
private readonly Dictionary<string, Func<string, string>> _credentialFormatters;
private Dictionary<string, Func<string, string>> _credentialFormatters;
private readonly IDiagnosticsSource _trace;
private readonly IAppConfiguration _config;
private readonly ICredentialBuilder _credentialBuilder;
private readonly ICredentialValidator _credentialValidator;

/// <summary>
/// This ctor is used for test only.
/// </summary>
protected AuthenticationService()
: this(null, null, null, AuditingService.None, Enumerable.Empty<Authenticator>())
{
Auditing = AuditingService.None;
Authenticators = new Dictionary<string, Authenticator>();
InitCredentialFormatters();
}

public AuthenticationService(IEntitiesContext entities, IAppConfiguration config, IDiagnosticsService diagnostics, AuditingService auditing, IEnumerable<Authenticator> providers)
public AuthenticationService(
IEntitiesContext entities, IAppConfiguration config, IDiagnosticsService diagnostics,
AuditingService auditing, IEnumerable<Authenticator> providers, ICredentialBuilder credentialBuilder,
ICredentialValidator credentialValidator)
{
_credentialFormatters = new Dictionary<string, Func<string, string>>(StringComparer.OrdinalIgnoreCase) {
{ "password", _ => Strings.CredentialType_Password },
{ "apikey", _ => Strings.CredentialType_ApiKey },
{ "external", FormatExternalCredentialType }
};
if (entities == null)
{
throw new ArgumentNullException(nameof(entities));
}

if (config == null)
{
throw new ArgumentNullException(nameof(config));
}

if (diagnostics == null)
{
throw new ArgumentNullException(nameof(diagnostics));
}

if (auditing == null)
{
throw new ArgumentNullException(nameof(auditing));
}

if (providers == null)
{
throw new ArgumentNullException(nameof(providers));
}

if (credentialBuilder == null)
{
throw new ArgumentNullException(nameof(credentialBuilder));
}

if (credentialValidator == null)
{
throw new ArgumentNullException(nameof(credentialValidator));
}

InitCredentialFormatters();

Entities = entities;
_config = config;
Auditing = auditing;
_trace = diagnostics.SafeGetSource("AuthenticationService");
Authenticators = providers.ToDictionary(p => p.Name, StringComparer.OrdinalIgnoreCase);
_credentialBuilder = credentialBuilder;
_credentialValidator = credentialValidator;
}

public IEntitiesContext Entities { get; private set; }
public IDictionary<string, Authenticator> Authenticators { get; private set; }
public AuditingService Auditing { get; private set; }

private void InitCredentialFormatters()
{
_credentialFormatters = new Dictionary<string, Func<string, string>>(StringComparer.OrdinalIgnoreCase) {
{ "password", _ => Strings.CredentialType_Password },
{ "apikey", _ => Strings.CredentialType_ApiKey },
{ "external", FormatExternalCredentialType }
};
}

public virtual async Task<AuthenticatedUser> Authenticate(string userNameOrEmail, string password)
{
using (_trace.Activity("Authenticate:" + userNameOrEmail))
Expand Down Expand Up @@ -80,9 +133,11 @@ await Auditing.SaveAuditRecord(

var passwordCredentials = user
.Credentials
.Where(c => c.Type.StartsWith(CredentialTypes.Password.Prefix, StringComparison.OrdinalIgnoreCase))
.Where(c => CredentialTypes.IsPassword(c.Type))
.ToList();
if (passwordCredentials.Count > 1 || !passwordCredentials.Any(c => String.Equals(c.Type, CredentialTypes.Password.Pbkdf2, StringComparison.OrdinalIgnoreCase)))

if (passwordCredentials.Count > 1 ||
!passwordCredentials.Any(c => string.Equals(c.Type, CredentialBuilder.LatestPasswordType, StringComparison.OrdinalIgnoreCase)))
{
await MigrateCredentials(user, passwordCredentials, password);
}
Expand Down Expand Up @@ -176,7 +231,7 @@ public virtual async Task<AuthenticatedUser> Register(string username, string em
.FirstOrDefault(u => u.Username == username || u.EmailAddress == emailAddress);
if (existingUser != null)
{
if (String.Equals(existingUser.Username, username, StringComparison.OrdinalIgnoreCase))
if (string.Equals(existingUser.Username, username, StringComparison.OrdinalIgnoreCase))
{
throw new EntityException(Strings.UsernameNotAvailable, username);
}
Expand All @@ -186,7 +241,6 @@ public virtual async Task<AuthenticatedUser> Register(string username, string em
}
}

var apiKey = Guid.NewGuid();
var newUser = new User(username)
{
EmailAllowed = true,
Expand All @@ -197,7 +251,7 @@ public virtual async Task<AuthenticatedUser> Register(string username, string em
};

// Add a credential for the password and the API Key
newUser.Credentials.Add(CredentialBuilder.CreateV1ApiKey(apiKey, TimeSpan.FromDays(_config.ExpirationInDaysForApiKeyV1)));
newUser.Credentials.Add(_credentialBuilder.CreateApiKey(TimeSpan.FromDays(_config.ExpirationInDaysForApiKeyV1)));
newUser.Credentials.Add(credential);

if (!_config.ConfirmEmailAddresses)
Expand All @@ -214,14 +268,6 @@ public virtual async Task<AuthenticatedUser> Register(string username, string em
return new AuthenticatedUser(newUser, credential);
}

[Obsolete("Use Register(string, string, Credential) now")]
public virtual Task<AuthenticatedUser> Register(string username, string password, string emailAddress)
{
var hashedPassword = CryptographyService.GenerateSaltedHash(password, Constants.PBKDF2HashAlgorithmId);
var passCred = new Credential(CredentialTypes.Password.Pbkdf2, hashedPassword);
return Register(username, emailAddress, passCred);
}

public virtual Task ReplaceCredential(string username, Credential credential)
{
var user = Entities
Expand Down Expand Up @@ -260,7 +306,7 @@ public virtual async Task<Credential> ResetPasswordWithToken(string username, st
throw new InvalidOperationException(Strings.UserIsNotYetConfirmed);
}

var cred = CredentialBuilder.CreatePbkdf2Password(newPassword);
var cred = _credentialBuilder.CreatePasswordCredential(newPassword);
await ReplaceCredentialInternal(user, cred);
user.PasswordResetToken = null;
user.PasswordResetTokenExpirationDate = null;
Expand Down Expand Up @@ -308,7 +354,7 @@ public virtual async Task GeneratePasswordResetToken(User user, int expirationIn
throw new InvalidOperationException(Strings.UserIsNotYetConfirmed);
}

if (!String.IsNullOrEmpty(user.PasswordResetToken) && !user.PasswordResetTokenExpirationDate.IsInThePast())
if (!string.IsNullOrEmpty(user.PasswordResetToken) && !user.PasswordResetTokenExpirationDate.IsInThePast())
{
return;
}
Expand All @@ -333,14 +379,13 @@ public virtual async Task<bool> ChangePassword(User user, string oldPassword, st
}

// Replace/Set password credential
var passwordCredential = CredentialBuilder.CreatePbkdf2Password(newPassword);
var passwordCredential = _credentialBuilder.CreatePasswordCredential(newPassword);
await ReplaceCredentialInternal(user, passwordCredential);

// Reset existing API keys
if (resetApiKey)
{
var apiKeyCredential = CredentialBuilder.CreateV1ApiKey(
Guid.NewGuid(), TimeSpan.FromDays(_config.ExpirationInDaysForApiKeyV1));
var apiKeyCredential = _credentialBuilder.CreateApiKey(TimeSpan.FromDays(_config.ExpirationInDaysForApiKeyV1));

await ReplaceCredentialInternal(user, apiKeyCredential);
}
Expand Down Expand Up @@ -415,7 +460,7 @@ public virtual async Task RemoveCredential(User user, Credential cred)
await Entities.SaveChangesAsync();
}

public async virtual Task<AuthenticateExternalLoginResult> ReadExternalLoginCredential(IOwinContext context)
public virtual async Task<AuthenticateExternalLoginResult> ReadExternalLoginCredential(IOwinContext context)
{
var result = await context.Authentication.AuthenticateAsync(AuthenticationTypes.External);
if (result == null)
Expand Down Expand Up @@ -459,11 +504,11 @@ public async virtual Task<AuthenticateExternalLoginResult> ReadExternalLoginCred
Authentication = null,
ExternalIdentity = result.Identity,
Authenticator = auther,
Credential = CredentialBuilder.CreateExternalCredential(authenticationType, idClaim.Value, nameClaim.Value + emailSuffix)
Credential = _credentialBuilder.CreateExternalCredential(authenticationType, idClaim.Value, nameClaim.Value + emailSuffix)
};
}

public async virtual Task<AuthenticateExternalLoginResult> AuthenticateExternalLogin(IOwinContext context)
public virtual async Task<AuthenticateExternalLoginResult> AuthenticateExternalLogin(IOwinContext context)
{
var result = await ReadExternalLoginCredential(context);

Expand Down Expand Up @@ -589,7 +634,7 @@ private Credential FindMatchingCredential(Credential credential)
else
{
// Don't put the credential itself in trace, but do put the Key for lookup later.
string message = String.Format(
string message = string.Format(
CultureInfo.CurrentCulture,
Strings.MultipleMatchingCredentials,
credential.Type,
Expand Down Expand Up @@ -627,37 +672,22 @@ private User FindByUserNameOrEmail(string userNameOrEmail)
return user;
}

public static bool ValidatePasswordCredential(IEnumerable<Credential> creds, string password, out Credential matched)
public bool ValidatePasswordCredential(IEnumerable<Credential> creds, string password, out Credential matched)
{
matched = creds.FirstOrDefault(c => ValidatePasswordCredential(c, password));
matched = creds.FirstOrDefault(c => _credentialValidator.ValidatePasswordCredential(c, password));
return matched != null;
}

private static readonly Dictionary<string, Func<string, Credential, bool>> _validators = new Dictionary<string, Func<string, Credential, bool>>(StringComparer.OrdinalIgnoreCase) {
{ CredentialTypes.Password.Pbkdf2, (password, cred) => CryptographyService.ValidateSaltedHash(cred.Value, password, Constants.PBKDF2HashAlgorithmId) },
{ CredentialTypes.Password.Sha1, (password, cred) => CryptographyService.ValidateSaltedHash(cred.Value, password, Constants.Sha1HashAlgorithmId) }
};

public static bool ValidatePasswordCredential(Credential cred, string password)
{
Func<string, Credential, bool> validator;
if (!_validators.TryGetValue(cred.Type, out validator))
{
return false;
}
return validator(password, cred);
}

private async Task MigrateCredentials(User user, List<Credential> creds, string password)
{
var toRemove = creds.Where(c =>
!String.Equals(
!string.Equals(
c.Type,
CredentialTypes.Password.Pbkdf2,
CredentialBuilder.LatestPasswordType,
StringComparison.OrdinalIgnoreCase))
.ToList();

// Remove any non PBKDF2 credentials
// Remove any non latest credentials
foreach (var cred in toRemove)
{
creds.Remove(cred);
Expand All @@ -669,7 +699,7 @@ private async Task MigrateCredentials(User user, List<Credential> creds, string
// Now add one if there are no credentials left
if (creds.Count == 0)
{
var newCred = CredentialBuilder.CreatePbkdf2Password(password);
var newCred = _credentialBuilder.CreatePasswordCredential(password);
await Auditing.SaveAuditRecord(new UserAuditRecord(user, AuditedUserAction.AddCredential, newCred));
user.Credentials.Add(newCred);
}
Expand Down
25 changes: 22 additions & 3 deletions src/NuGetGallery/Authentication/Providers/ApiKey/ApiKeyAuthenticationHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Owin.Logging;
using Microsoft.Owin.Security;
using Microsoft.Owin.Security.Infrastructure;
using NuGetGallery.Infrastructure.Authentication;

namespace NuGetGallery.Authentication.Providers.ApiKey
{
Expand All @@ -18,14 +19,32 @@ public class ApiKeyAuthenticationHandler : AuthenticationHandler<ApiKeyAuthentic

protected ILogger Logger { get; set; }
protected AuthenticationService Auth { get; set; }
protected ICredentialBuilder CredentialBuilder { get; set; }

private ApiKeyAuthenticationOptions TheOptions { get { return _options ?? Options; } }

internal ApiKeyAuthenticationHandler() { }
public ApiKeyAuthenticationHandler(ILogger logger, AuthenticationService auth)

public ApiKeyAuthenticationHandler(ILogger logger, AuthenticationService auth, ICredentialBuilder credentialBuilder)
{
if (logger == null)
{
throw new ArgumentNullException(nameof(logger));
}

if (auth == null)
{
throw new ArgumentNullException(nameof(auth));
}

if (credentialBuilder == null)
{
throw new ArgumentNullException(nameof(credentialBuilder));
}

Logger = logger;
Auth = auth;
CredentialBuilder = credentialBuilder;
}

internal Task InitializeAsync(ApiKeyAuthenticationOptions options, IOwinContext context)
Expand Down Expand Up @@ -70,10 +89,10 @@ internal void WriteStatus(string message, int statusCode)
protected override async Task<AuthenticationTicket> AuthenticateCoreAsync()
{
var apiKey = Request.Headers[TheOptions.ApiKeyHeaderName];
if (!String.IsNullOrEmpty(apiKey))
if (!string.IsNullOrEmpty(apiKey))
{
// Get the user
var authUser = await Auth.Authenticate(CredentialBuilder.CreateV1ApiKey(apiKey, TimeSpan.Zero));
var authUser = await Auth.Authenticate(CredentialBuilder.ParseApiKeyCredential(apiKey));
if (authUser != null)
{
// Set the current user
Expand Down
Loading

0 comments on commit a173a57

Please sign in to comment.