diff --git a/algorithm/shacrypt/decoder.go b/algorithm/shacrypt/decoder.go index 22b8e91..1a947d3 100644 --- a/algorithm/shacrypt/decoder.go +++ b/algorithm/shacrypt/decoder.go @@ -1,8 +1,10 @@ package shacrypt import ( + "errors" "fmt" "strconv" + "strings" "github.com/go-crypt/crypt/algorithm" "github.com/go-crypt/crypt/internal/encoding" @@ -120,15 +122,18 @@ func decode(variant Variant, parts []string) (digest algorithm.Digest, err error case "rounds": var rounds uint64 - if rounds, err = strconv.ParseUint(param.Value, 10, 32); err != nil { + if rounds, err = strconv.ParseUint(param.Value, 10, 64); err != nil && (!errors.Is(err, strconv.ErrRange) || strings.Trim(param.Value, "0123456789") != "") { return nil, fmt.Errorf("%w: option '%s' has invalid value '%s': %v", algorithm.ErrEncodedHashInvalidOptionValue, param.Key, param.Value, err) } - if rounds < IterationsMin || rounds > IterationsMax { - return nil, fmt.Errorf(algorithm.ErrFmtInvalidIntParameter, algorithm.ErrEncodedHashInvalidOptionValue, param.Key, IterationsMin, "", IterationsMax, rounds) + switch { + case rounds < IterationsMin: + decoded.iterations = IterationsMin + case rounds > IterationsMax: + decoded.iterations = IterationsMax + default: + decoded.iterations = int(rounds) } - - decoded.iterations = int(rounds) default: return nil, fmt.Errorf("%w: option '%s' with value '%s' is unknown", algorithm.ErrEncodedHashInvalidOptionKey, param.Key, param.Value) } diff --git a/algorithm/shacrypt/regression_test.go b/algorithm/shacrypt/regression_test.go index 17f46a3..200d881 100644 --- a/algorithm/shacrypt/regression_test.go +++ b/algorithm/shacrypt/regression_test.go @@ -7,15 +7,40 @@ import ( "github.com/stretchr/testify/require" ) -func TestDecodeRejectsUnusableRounds(t *testing.T) { +func TestDecodeClampsOutOfRangeRounds(t *testing.T) { + testCases := []struct { + name string + digest string + expected string + }{ + {"Zero", "$6$rounds=0$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"}, + {"BelowMinimum", "$6$rounds=100$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"}, + {"JustBelowMinimum", "$6$rounds=999$saltsalt$keykeykey", "$6$rounds=1000$saltsalt$keykeykey"}, + {"JustAboveMaximum", "$6$rounds=1000000000$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"}, + {"AboveMaximum", "$6$rounds=4294967295$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"}, + {"AboveUint32", "$6$rounds=4294967296$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"}, + {"AboveUint64", "$6$rounds=99999999999999999999$saltsalt$keykeykey", "$6$rounds=999999999$saltsalt$keykeykey"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + digest, err := Decode(tc.digest) + + require.NoError(t, err) + assert.Equal(t, tc.expected, digest.Encode()) + }) + } +} + +func TestDecodeRejectsInvalidRounds(t *testing.T) { testCases := []struct { name string digest string }{ - {"Zero", "$6$rounds=0$saltsalt$keykeykey"}, - {"BelowMinimum", "$6$rounds=100$saltsalt$keykeykey"}, - {"JustBelowMinimum", "$6$rounds=999$saltsalt$keykeykey"}, - {"AboveMaximum", "$6$rounds=4294967295$saltsalt$keykeykey"}, + {"Empty", "$6$rounds=$saltsalt$keykeykey"}, + {"Negative", "$6$rounds=-1$saltsalt$keykeykey"}, + {"NotNumeric", "$6$rounds=abc$saltsalt$keykeykey"}, + {"AboveUint64NotNumericSuffix", "$6$rounds=99999999999999999999x$saltsalt$keykeykey"}, } for _, tc := range testCases { @@ -28,6 +53,38 @@ func TestDecodeRejectsUnusableRounds(t *testing.T) { } } +func TestDecodeRoundsTooLowSpecVectors(t *testing.T) { + testCases := []struct { + name string + digest string + expected string + }{ + { + "SHA256", + "$5$rounds=10$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC", + "$5$rounds=1000$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC", + }, + { + "SHA512", + "$6$rounds=10$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.", + "$6$rounds=1000$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + digest, err := Decode(tc.digest) + require.NoError(t, err) + + match, err := digest.MatchAdvanced("the minimum number is still observed") + + require.NoError(t, err) + assert.True(t, match) + assert.Equal(t, tc.expected, digest.Encode()) + }) + } +} + func TestDecodeAcceptsRoundsWithinRange(t *testing.T) { testCases := []struct { name string diff --git a/fuzz_test.go b/fuzz_test.go index 9885017..a8cbef6 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -1,7 +1,6 @@ package crypt import ( - "strings" "testing" ) @@ -44,52 +43,6 @@ func FuzzNormalize(f *testing.F) { }) } -// argon2MemoryHardParameter is the argon2 memory parameter carried by several corpus entries. Deriving a key with it -// allocates 2GiB which is more than a test run should require, so the entries which carry it and also decode are -// skipped when verifying passwords. FuzzDecode and FuzzNormalize still cover them as neither derives a key. -const argon2MemoryHardParameter = "m=2097152" - -func TestCheckPasswordNeverPanicsOverCorpus(t *testing.T) { - for _, encodedDigest := range corpusDecode { - t.Run(encodedDigest, func(t *testing.T) { - var ( - valid bool - err error - ) - - if strings.Contains(encodedDigest, argon2MemoryHardParameter) { - if _, decodeErr := Decode(encodedDigest); decodeErr == nil { - t.Skip("deriving a key from this digest would allocate 2GiB; it remains covered by FuzzDecode and FuzzNormalize") - } - } - - if !assertNotPanics(t, func() { valid, err = CheckPassword("password", encodedDigest) }) { - return - } - - if valid && err != nil { - t.Fatalf("CheckPassword reported a match alongside the error %v", err) - } - }) - } -} - -func assertNotPanics(t *testing.T, f func()) (ok bool) { - t.Helper() - - defer func() { - if r := recover(); r != nil { - t.Errorf("panic: %v", r) - - ok = false - } - }() - - f() - - return true -} - var corpusDecode = []string{ // Well formed digests for each supported identifier. "$argon2id$v=19$m=2097152,t=1,p=4$YmxhaGJsYWhibGFoYmxhaA$Vt4rHrEcdEJ+A9FBAOtqE21NX2NDaCyR3xr0PJmg+dU",