1
\$\begingroup\$

As part of a larger authentication/authorization system, I've developed a small .NET Core 3.0 service for hashing passwords using PBKDF2 (with a salt) and validating passwords against a stored hash/salt, for use as part of a larger authentication/authorization system.

Code organization:

  • IPasswordService is the main interface used by the larger system.
  • PasswordService is the implementation, the heart of this body of code.
  • KeyDerivationParameters and CryptoRng are used when configuring and loading the service (ideally through a DI container).
  • ICryptoRng is used as an interface to stub out when unit testing PasswordService.
  • ValueTypes contains domain-specific types wrapping primitives, using the ValueOf library.
  • HashedPassword is a simple wrapper encapsulating both a hash and a salt.
  • PasswordServiceTests contains my unit tests.

My main concerns (though all feedback is welcome):

  • Security - making sure there's no obvious attack I'm missing. I should note that I only chose PBKDF2 because it has an implementation in .NET; argon and bcrypt are only available in C# through third-party libraries in various states of maintenance. I also intend on using a larger iteration count in practice than the 10,000 iterations specified in the tests; I wanted to keep the unit tests fast.
  • Tests - I only have four unit tests for the PasswordService, and I'm not confident that they provide enough of a safety net.
  • The use of ValueOf to create meaningful domain-specific types instead of using primitives everywhere. In Haskell, using newtypes for this sort of thing is common, but I'm curious what C# developers think about it.

The code is broken up into eight files across two projects; it's available on GitHub here, as well as below. The projects are .NET Core 3.0, using

<PropertyGroup>
  <LangVersion>8.0</LangVersion>
  <Nullable>enable</Nullable>
</PropertyGroup>

in the .csproj files to enable C# 8's nullable reference types.

IPasswordService.cs

namespace AuthSystemPasswordService.Interfaces
{
    public interface IPasswordService
    {
        HashedPassword GeneratePasswordHashAndSalt(PlaintextPassword password);
        bool CheckIfPasswordMatchesHash(PlaintextPassword password, HashedPassword hash);
    }
}

ICryptoRng.cs

namespace AuthSystemPasswordService.Interfaces
{
    public interface ICryptoRng
    {
        byte[] GetRandomBytes(int numBytes);
    }
}

ValueTypes.cs

using ValueOf;

namespace AuthSystemPasswordService
{
    public class PlaintextPassword : ValueOf<string, PlaintextPassword>
    {
    }

    public class Base64Hash : ValueOf<string, Base64Hash>
    {
    }

    public class Base64Salt : ValueOf<string, Base64Salt>
    {
    }

    public class IterationCount : ValueOf<int, IterationCount>
    {
    }

    public class SaltLength : ValueOf<int, SaltLength>
    {
    }

    public class KeyLength : ValueOf<int, KeyLength>
    {
    }
}

HashedPassword.cs

namespace AuthSystemPasswordService
{
    public struct HashedPassword
    {
        public Base64Hash Base64PasswordHash { get; }
        public Base64Salt Base64Salt { get; }

        public HashedPassword(Base64Hash passwordHash, Base64Salt salt)
        {
            Base64PasswordHash = passwordHash;
            Base64Salt = salt;
        }
    }
}

KeyDerivationParameters.cs

using Microsoft.AspNetCore.Cryptography.KeyDerivation;

namespace AuthSystemPasswordService
{
    public struct KeyDerivationParameters
    {
        public KeyDerivationPrf DerivationFunction { get; }

        public IterationCount IterationCount { get; }

        public SaltLength SaltLength { get; }

        public KeyLength KeyLength { get; }

        public KeyDerivationParameters(KeyDerivationPrf derivationFunction, IterationCount iterationCount,
            SaltLength saltLength, KeyLength keyLength)
        {
            DerivationFunction = derivationFunction;
            IterationCount = iterationCount;
            SaltLength = saltLength;
            KeyLength = keyLength;
        }
    }
}

CryptoRng.cs

using AuthSystemPasswordService.Interfaces;
using System.Security.Cryptography;

namespace AuthSystemPasswordService.Services
{
    public class CryptoRng : ICryptoRng
    {
        private RandomNumberGenerator Generator { get; }

        public CryptoRng()
        {
            Generator = RandomNumberGenerator.Create();
        }

        public byte[] GetRandomBytes(int numBytes)
        {
            var bytes = new byte[numBytes];
            Generator.GetBytes(bytes);
            return bytes;
        }
    }
}

PasswordService

using AuthSystemPasswordService.Interfaces;
using Microsoft.AspNetCore.Cryptography.KeyDerivation;
using System;
using System.Linq;

namespace AuthSystemPasswordService.Services
{
    public class PasswordService : IPasswordService
    {
        private KeyDerivationParameters Parameters { get; }
        private ICryptoRng Rng { get; }

        public PasswordService(KeyDerivationParameters parameters, ICryptoRng rng)
        {
            Parameters = parameters;
            Rng = rng;
        }

        public HashedPassword GeneratePasswordHashAndSalt(PlaintextPassword password)
        {
            var saltBytes = Rng.GetRandomBytes(Parameters.SaltLength.Value);
            var salt = Convert.ToBase64String(saltBytes);

            var hashBytes = KeyDerivation.Pbkdf2(password.Value, saltBytes, Parameters.DerivationFunction,
                Parameters.IterationCount.Value, Parameters.KeyLength.Value);
            var hash = Convert.ToBase64String(hashBytes);

            return new HashedPassword(Base64Hash.From(hash), Base64Salt.From(salt));
        }

        public bool CheckIfPasswordMatchesHash(PlaintextPassword password, HashedPassword hash)
        {
            var passwordHash = KeyDerivation.Pbkdf2(password.Value, Convert.FromBase64String(hash.Base64Salt.Value), Parameters.DerivationFunction,
                Parameters.IterationCount.Value, Parameters.KeyLength.Value);
            return passwordHash.SequenceEqual(Convert.FromBase64String(hash.Base64PasswordHash.Value));
        }
    }
}

PasswordServiceTests

using AuthSystemPasswordService.Interfaces;
using AuthSystemPasswordService.Services;
using Microsoft.AspNetCore.Cryptography.KeyDerivation;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute;
using System;

namespace AuthSystemPasswordService.Test
{
    [TestClass]
    public class PasswordServiceTests
    {
        [TestMethod]
        [TestCategory("UnitTest")]
        public void GenerateHashAndSalt_ReturnsSalt_WithNumberOfBytesEqualToSaltLengthParameter()
        {
            // Arrange
            var iterationCount = 10_000;
            var saltLength = 16;
            var keyLength = 64;
            var parameters = new KeyDerivationParameters(KeyDerivationPrf.HMACSHA512,
                IterationCount.From(iterationCount), SaltLength.From(saltLength), KeyLength.From(keyLength));

            var rng = Substitute.For<ICryptoRng>();
            rng.GetRandomBytes(Arg.Any<int>()).Returns(args => new byte[args.Arg<int>()]);

            var service = new PasswordService(parameters, rng);

            // Act
            var hash = service.GeneratePasswordHashAndSalt(PlaintextPassword.From("somePassword"));

            // Assert
            Assert.AreEqual(saltLength, Convert.FromBase64String(hash.Base64Salt.Value).Length);
        }

        [TestMethod]
        [TestCategory("UnitTest")]
        public void GenerateHashAndSalt_ReturnsHash_WithNumberOfBytesEqualToKeyLengthParameter()
        {
            // Arrange
            var iterationCount = 10_000;
            var saltLength = 16;
            var keyLength = 64;
            var parameters = new KeyDerivationParameters(KeyDerivationPrf.HMACSHA512,
                IterationCount.From(iterationCount), SaltLength.From(saltLength), KeyLength.From(keyLength));

            var rng = Substitute.For<ICryptoRng>();
            rng.GetRandomBytes(Arg.Any<int>()).Returns(args => new byte[args.Arg<int>()]);

            var service = new PasswordService(parameters, rng);

            // Act
            var hash = service.GeneratePasswordHashAndSalt(PlaintextPassword.From("somePassword"));

            // Assert
            Assert.AreEqual(keyLength, Convert.FromBase64String(hash.Base64PasswordHash.Value).Length);
        }

        [TestMethod]
        [TestCategory("UnitTest")]
        public void GenerateHashAndSalt_ThenCheckingSamePassword_ReturnsTrue()
        {
            // Arrange
            var iterationCount = 10_000;
            var saltLength = 16;
            var keyLength = 64;
            var parameters = new KeyDerivationParameters(KeyDerivationPrf.HMACSHA512,
                IterationCount.From(iterationCount), SaltLength.From(saltLength), KeyLength.From(keyLength));

            var rng = Substitute.For<ICryptoRng>();
            rng.GetRandomBytes(Arg.Any<int>()).Returns(args => new byte[args.Arg<int>()]);

            var service = new PasswordService(parameters, rng);

            var password = PlaintextPassword.From("somePass");

            // Act
            var hash = service.GeneratePasswordHashAndSalt(password);
            var checkResult = service.CheckIfPasswordMatchesHash(password, hash);

            // Assert
            Assert.IsTrue(checkResult);
        }

        [TestMethod]
        [TestCategory("UnitTest")]
        public void GenerateHashAndSalt_ThenCheckingOtherPassword_ReturnsFalse()
        {
            // Arrange
            var iterationCount = 10_000;
            var saltLength = 16;
            var keyLength = 64;
            var parameters = new KeyDerivationParameters(KeyDerivationPrf.HMACSHA512,
                IterationCount.From(iterationCount), SaltLength.From(saltLength), KeyLength.From(keyLength));

            var rng = Substitute.For<ICryptoRng>();
            rng.GetRandomBytes(Arg.Any<int>()).Returns(args => new byte[args.Arg<int>()]);

            var service = new PasswordService(parameters, rng);

            var password = PlaintextPassword.From("somePass");
            var otherPass = PlaintextPassword.From("otherPass");

            // Act
            var hash = service.GeneratePasswordHashAndSalt(password);
            var checkResult = service.CheckIfPasswordMatchesHash(otherPass, hash);

            // Assert
            Assert.IsFalse(checkResult);
        }
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Possibly use github.com/BcryptNet/bcrypt.net instead of PBKDF2. \$\endgroup\$
    – DylanSp
    Oct 15, 2019 at 18:53
  • \$\begingroup\$ What's your goal? I see a lot of code, but in the end it just performs PBKDF2 and base 64 encoding? Wrapper classes should not be a goal in itself. \$\endgroup\$ Oct 22, 2019 at 4:32
  • \$\begingroup\$ @MaartenBodewes Ultimately, the goal is to provide a mockable, easy-to-use interface for the rest of my authentication/authorization system to use that has a bit more type safety than throwing byte[]'s around. \$\endgroup\$
    – DylanSp
    Oct 22, 2019 at 13:34

1 Answer 1

1
\$\begingroup\$

One potential security issue, as mentioned in this answer to a similar piece of code, is that the use of SequenceEqual() in CheckIfPasswordMatchesHash() may open this up to timing attacks.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.