From 3d4dbbc7151a86551f7f1a235b4c4bc07e50d5cb Mon Sep 17 00:00:00 2001 From: James Elliott Date: Thu, 24 Sep 2026 21:04:37 +1000 Subject: [PATCH] fix(sha1crypt): apply the default salt length SaltLengthMin was 0, so the hasher's check for an unset salt length (bytesSalt < SaltLengthMin) was never true and the default of 8 was never applied. As a result every digest produced with the default options had an empty salt, meaning identical passwords produced identical digests. SaltLengthMin is now 1, matching the documented minimum for WithSaltLength. This also means WithSaltLength(0) and HashWithSalt with an empty salt now return an error. Existing digests with an empty salt can still be decoded and verified. --- algorithm/sha1crypt/const.go | 2 +- algorithm/sha1crypt/sha1crypt_test.go | 35 ++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/algorithm/sha1crypt/const.go b/algorithm/sha1crypt/const.go index 6bdd8fe..fbe94a6 100644 --- a/algorithm/sha1crypt/const.go +++ b/algorithm/sha1crypt/const.go @@ -15,7 +15,7 @@ const ( AlgIdentifier = "sha1" // SaltLengthMin is the minimum salt size accepted. - SaltLengthMin = 0 + SaltLengthMin = 1 // SaltLengthMax is the maximum salt size accepted. SaltLengthMax = 64 diff --git a/algorithm/sha1crypt/sha1crypt_test.go b/algorithm/sha1crypt/sha1crypt_test.go index 159bdf0..d7363c5 100644 --- a/algorithm/sha1crypt/sha1crypt_test.go +++ b/algorithm/sha1crypt/sha1crypt_test.go @@ -37,10 +37,11 @@ func TestWithSaltLength(t *testing.T) { have int err string }{ - {"ShouldNotErrMin", 0, ""}, + {"ShouldNotErrMin", 1, ""}, {"ShouldNotErrMax", 64, ""}, - {"ShouldErrBelowMin", -1, "sha1crypt validation error: parameter is invalid: parameter 'salt length' must be between 0 and 64 but is set to '-1'"}, - {"ShouldErrAboveMax", 65, "sha1crypt validation error: parameter is invalid: parameter 'salt length' must be between 0 and 64 but is set to '65'"}, + {"ShouldErrZero", 0, "sha1crypt validation error: parameter is invalid: parameter 'salt length' must be between 1 and 64 but is set to '0'"}, + {"ShouldErrBelowMin", -1, "sha1crypt validation error: parameter is invalid: parameter 'salt length' must be between 1 and 64 but is set to '-1'"}, + {"ShouldErrAboveMax", 65, "sha1crypt validation error: parameter is invalid: parameter 'salt length' must be between 1 and 64 but is set to '65'"}, } for _, tc := range testCases { @@ -125,7 +126,8 @@ func TestHashWithSalt(t *testing.T) { err string }{ {"ShouldNotErrValidSalt", []byte("abcdefgh"), ""}, - {"ShouldErrSaltTooLong", make([]byte, 65), "sha1crypt hashing error: salt is invalid: salt bytes must have a length of between 0 and 64 but has a length of 65"}, + {"ShouldErrSaltEmpty", nil, "sha1crypt hashing error: salt is invalid: salt bytes must have a length of between 1 and 64 but has a length of 0"}, + {"ShouldErrSaltTooLong", make([]byte, 65), "sha1crypt hashing error: salt is invalid: salt bytes must have a length of between 1 and 64 but has a length of 65"}, } for _, tc := range testCases { @@ -227,3 +229,28 @@ func TestDigestEncode(t *testing.T) { assert.NotEmpty(t, encoded) assert.Equal(t, encoded, digest.String()) } + +func TestHashUsesDefaultSaltLength(t *testing.T) { + hasher, err := New(WithIterations(1000)) + require.NoError(t, err) + + first, err := hasher.Hash("password") + require.NoError(t, err) + + second, err := hasher.Hash("password") + require.NoError(t, err) + + assert.Len(t, first.Salt(), SaltLengthDefault) + assert.Len(t, second.Salt(), SaltLengthDefault) + assert.NotEqual(t, first.Encode(), second.Encode()) +} + +func TestDecodeEmptySaltDigest(t *testing.T) { + // Digests with an empty salt were produced by default in earlier versions and must remain verifiable. + digest, err := Decode("$sha1$1000$$LpsYCXQDwcmGKSeE7gBjRolvHeh/") + require.NoError(t, err) + + assert.Empty(t, digest.Salt()) + assert.True(t, digest.Match("password")) + assert.False(t, digest.Match("wrong")) +}