Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create rule S5344: Passwords should not be stored in plain-text or with a fast hashing algorithm #8998

Closed
10 tasks
gaetan-ferry-sonarsource opened this issue Mar 29, 2024 · 0 comments · Fixed by #9278
Assignees
Labels
Area: C# C# rules related issues. Area: Security Related to Vulnerability and Security Hotspot rules Sprint: MMF-3716 crypto rules https://sonarsource.atlassian.net/browse/MMF-3716 Type: New Rule Implementation for a rule that HAS been specified.
Projects

Comments

@gaetan-ferry-sonarsource
Copy link

gaetan-ferry-sonarsource commented Mar 29, 2024

  • PasswordHasherOptions.IterationCount < 100K (Core)
  • PasswordHasherOptions.CompatibilityMode == IdentityV2 (Core)
  • KeyDerivation.Pbkdf2.iterationCount < 100K (Core)
  • Rfc2898DeriveBytes.Pbkdf2.iterations < 100K (Core)
  • PasswordHasher instantiated (FRM)
  • Rfc2898DeriveBytes.iterations < 100K (cross-platform)
  • Rfc2898DeriveBytes.hashAlgorithm does not exist (cross-platform)
  • (OpenBsdCrypt/BCrypt).Generate.cost < 12 (BouncyCastle)
  • PbeParametersGenerator.Init.iterationCount < 100K (BouncyCastle)
  • SCrypt.Generate N < 2^12, r < 8, or dklen < 32 (BouncyCastle)

Why

As part of MMF-3716, we want to close the gap between C# and other languages regarding cryptography-related rules support. S5344 is one of the rules that is not currently supported by this analyzer.

What

S5344 aims to detect when passwords are stored in clear text or with a fast hashing algorithm. Here, we will focus on detecting cost tweakable password hashing function configured with insufficiently strong parameters. The detection logic will be split between three components: .NET core, .NET framework and BouncyCastle.

Detection logic

.Net Core

For Microsoft.AspNetCore.Identity:

Raise on PasswordHasherOptions attributes matching :

  • IterationCount is < 100 000
  • CompatibilityMode is set to PasswordHasherCompatibilityMode.IdentityV2

Example code

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Options;

[ApiController]
[Route("passwordHasher")]
public class PasswordHasherController : ControllerBase
{
    [Route("Compliant")]
    public string Compliant()
    {
        string password = Request.Query["password"];
        PasswordHasher<User> hasher = new PasswordHasher<User>();
        string hash = hasher.HashPassword(new User("test", password), password);
        
        return hash;
    }

    [Route("Noncompliant/LowIterations")]
    public string NoncompliantLowIt()
    {
        string password = Request.Query["password"];
        IOptions<PasswordHasherOptions> options = Options.Create(new PasswordHasherOptions() {
            IterationCount = 1
        });
        PasswordHasher<User> hasher = new PasswordHasher<User>(options);
        string hash = hasher.HashPassword(new User("test", password), password);
        
        return hash;
    }

    [Route("Noncompliant/CompatMode")]
    public string NoncompliantCompat()
    {
        string password = Request.Query["password"];
        IOptions<PasswordHasherOptions> options = Options.Create(new PasswordHasherOptions() {
            CompatibilityMode = PasswordHasherCompatibilityMode.IdentityV2 // Noncompliant
        });
        PasswordHasher<User> hasher = new PasswordHasher<User>(options);
        string hash = hasher.HashPassword(new User("test", password), password);
        
        return hash;
    }
}

For Microsoft.AspNetCore.Cryptography.KeyDerivation:
Raise when KeyDerivation.Pbkdf2 is called with iterationCount < 100 000.

Example code

using Microsoft.AspNetCore.Cryptography.KeyDerivation;
using System.Security.Cryptography;

namespace demo.Controllers;


[ApiController]
[Route("crypto")]
public class CryptoController : ControllerBase
{
    [Route("aspnetcore/Compliant")]
    public string AspNetCoreCompliant()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);

        string hashed = Convert.ToBase64String(KeyDerivation.Pbkdf2(
            password: password!,
            salt: salt,
            prf: KeyDerivationPrf.HMACSHA256,
            iterationCount: 100000,
            numBytesRequested: 256 / 8));
        
        return hashed;
    }

    [Route("aspnetcore/Noncompliant/LowIterations")]
    public string AspNetCoreNoncompliantLowIt()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8); // divide by 8 to convert bits to bytes

        string hashed = Convert.ToBase64String(KeyDerivation.Pbkdf2(
            password: password!,
            salt: salt,
            prf: KeyDerivationPrf.HMACSHA256,
            iterationCount: 1, // Noncompliant
            numBytesRequested: 256 / 8));
        
        return hashed;
    }
}

For System.Security.Cryptography:

Raise when Rfc2898DeriveBytes is instantiated with an iterations parameter < 100 000.
Raise when Rfc2898DeriveBytes is instantiated without a hashAlgorithm parameter.
Raise when Rfc2898DeriveBytes.Pbkdf2 is called with an iterations parameter < 100 000.

Example code

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Cryptography.KeyDerivation;
using System.Security.Cryptography;

namespace demo.Controllers;


[ApiController]
[Route("crypto")]
public class CryptoController : ControllerBase
{
    [Route("security/Compliant/Constructor")]
    public string SecurityConstructorCompliant()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, salt, 100_000, HashAlgorithmName.SHA256);
        string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));
        
        return hashed;
    }

    [Route("security/Compliant/Method")]
    public string SecurityMethodCompliant()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        string hashed = Convert.ToBase64String(Rfc2898DeriveBytes.Pbkdf2(password, salt, 100_000, HashAlgorithmName.SHA256, 256 /8));
        
        return hashed;
    }

    [Route("security/NonCompliant/Constructor/LowIter")]
    public string SecurityConstructorNonCompliantLowIter()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, salt, 100, HashAlgorithmName.SHA256); // Noncompliant
        string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));
        
        return hashed;
    }

    [Route("security/NonCompliant/Constructor/obsolete")]
    public string SecurityConstructorNonCompliantObsolete()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, 128/8); // Noncompliant
        string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));
        
        return hashed;
    }
}

.Net Framework

For Microsoft.AspNet.Identity

Raise any time a PasswordHasher is instantiated.

Example code

using System;
using Microsoft.AspNet.Identity;

string password = "NotSoSecure";
PasswordHasher hasher = new PasswordHasher();
string hash = hasher.HashPassword(password); // Noncompliant, defaults to 1000 iterations

For System.Security.Cryptography:
Raise when Rfc2898DeriveBytes is instantiated with an iterations parameter < 10 000.
Raise when Rfc2898DeriveBytes is instantiated without a hashAlgorithm parameter.

Example code

using System;
using System.Security.Cryptography;

public static string SecurityConstructorCompliant(string password)
{

    byte[] salt = new byte[32];
    rngCsp.GetBytes(salt);
    Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, salt, 100_000, HashAlgorithmName.SHA256);
    string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));

    return hashed;
}

public static string SecurityConstructorNonCompliantLowIter(string password)
{
    byte[] salt = new byte[32];
    rngCsp.GetBytes(salt);
    Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, salt, 100, HashAlgorithmName.SHA256); // Noncompliant
    string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));

    return hashed;
}

public static string SecurityConstructorNonCompliantObsolete(string password)
{
    byte[] salt = new byte[32];
    rngCsp.GetBytes(salt);
    Rfc2898DeriveBytes kdf = new Rfc2898DeriveBytes(password, 128 / 8); // Noncompliant
    string hashed = Convert.ToBase64String(kdf.GetBytes(256 / 8));

    return hashed;
}

BouncyCastle

For Org.BouncyCastle.Crypto.Generators.OpenBsdBCrypt, or Org.BouncyCastle.Crypto.Generators.BCrypt:
Raise when Generate is called with cost < 12.

For Org.BouncyCastle.Crypto.PbeParametersGenerator:
Raise when Init is called with iterationCount < 100 000. Note that this method is usually indirectly called by a subclass such as Org.BouncyCastle.Crypto.Generators.Pkcs5S2ParametersGenerator.

For Org.BouncyCastle.Crypto.Generators.SCrypt:
Raise when Generate is called with N < 2^12, r < 8, or dklen < 32.

Example code

using Microsoft.AspNetCore.Mvc;
using System.Security.Cryptography;
using System.Text;
using Org.BouncyCastle.Crypto.Generators;
using Org.BouncyCastle.Crypto.Parameters;

namespace demo.Controllers;

[ApiController]
[Route("bc")]
public class BCController : ControllerBase
{
    [Route("Compliant/BCrypt")]
    public string CompliantBcrypt()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);

        string hashed = OpenBsdBCrypt.Generate(password.ToCharArray(), salt, 14);
        
        return hashed;
    }

    [Route("Noncompliant/Bcrypt")]
    public string NoncompliantBCrypt()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);

        string hashed = OpenBsdBCrypt.Generate(password.ToCharArray(), salt, 4); // Noncompliant
        
        return hashed;
    }

    [Route("Compliant/SCrypt")]
    public string CompliantScrypt()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        string hashed = Convert.ToBase64String(SCrypt.Generate(Encoding.Unicode.GetBytes(password), salt, 1<<12, 8, 1, 32));
        
        return hashed;
    }

    [Route("Noncompliant/Scrypt")]
    public string NoncompliantSCrypt()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        string hashed = Convert.ToBase64String(SCrypt.Generate(Encoding.Unicode.GetBytes(password), salt, 4, 8, 1, 32));  // Noncompliant
        
        return hashed;
    }

    [Route("Compliant/PBKDF2")]
    public string CompliantPBKDF2()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        Pkcs5S2ParametersGenerator gen = new Pkcs5S2ParametersGenerator();
        gen.Init(Encoding.Unicode.GetBytes(password), salt, 100_000); 
        KeyParameter keyParam = (KeyParameter) gen.GenerateDerivedMacParameters(256);
        string hashed = Convert.ToBase64String(keyParam.GetKey());
        
        return hashed;
    }

    [Route("Noncompliant/PBKDF2")]
    public string NoncompliantPBKDF2()
    {
        string password = Request.Query["password"];
        byte[] salt = RandomNumberGenerator.GetBytes(128 / 8);
        Pkcs5S2ParametersGenerator gen = new Pkcs5S2ParametersGenerator();
        gen.Init(Encoding.Unicode.GetBytes(password), salt, 100);  // Noncompliant
        KeyParameter keyParam = (KeyParameter) gen.GenerateDerivedMacParameters(256);
        string hashed = Convert.ToBase64String(keyParam.GetKey());
        
        return hashed;
    }
}

RSPEC

This rule's RSPEC contains information regarding messages and highlighting:

@pavel-mikula-sonarsource pavel-mikula-sonarsource added Area: C# C# rules related issues. Area: Security Related to Vulnerability and Security Hotspot rules Type: New Rule Implementation for a rule that HAS been specified. labels Apr 8, 2024
@costin-zaharia-sonarsource costin-zaharia-sonarsource added the Sprint: MMF-3716 crypto rules https://sonarsource.atlassian.net/browse/MMF-3716 label May 13, 2024
@gregory-paidis-sonarsource gregory-paidis-sonarsource moved this from To do to In progress in Best Kanban May 13, 2024
@gregory-paidis-sonarsource gregory-paidis-sonarsource moved this from In progress to Review in progress in Best Kanban May 15, 2024
Best Kanban automation moved this from Review in progress to Validate Peach May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: Security Related to Vulnerability and Security Hotspot rules Sprint: MMF-3716 crypto rules https://sonarsource.atlassian.net/browse/MMF-3716 Type: New Rule Implementation for a rule that HAS been specified.
Projects
Best Kanban
  
Validate Peach
4 participants