diff --git a/token/hmac/bytes.go b/token/hmac/bytes.go index 2a980f5e..b991e075 100644 --- a/token/hmac/bytes.go +++ b/token/hmac/bytes.go @@ -14,7 +14,7 @@ import ( func RandomBytes(n int) ([]byte, error) { bytes := make([]byte, n) if _, err := io.ReadFull(rand.Reader, bytes); err != nil { - return []byte{}, errorsx.WithStack(err) + return nil, errorsx.WithStack(err) } return bytes, nil } diff --git a/token/hmac/bytes_test.go b/token/hmac/bytes_test.go index 26e16d15..5e1beb4e 100644 --- a/token/hmac/bytes_test.go +++ b/token/hmac/bytes_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRandomBytes(t *testing.T) { @@ -17,13 +18,11 @@ func TestRandomBytes(t *testing.T) { func TestPseudoRandomness(t *testing.T) { runs := 65536 - results := map[string]bool{} + results := map[string]struct{}{} for i := 0; i < runs; i++ { bytes, err := RandomBytes(128) - assert.NoError(t, err) - - _, ok := results[string(bytes)] - assert.False(t, ok) - results[string(bytes)] = true + require.NoError(t, err) + results[string(bytes)] = struct{}{} } + assert.Len(t, results, runs) } diff --git a/token/hmac/hmacsha.go b/token/hmac/hmacsha.go index c3b0985d..ce64c6f9 100644 --- a/token/hmac/hmacsha.go +++ b/token/hmac/hmacsha.go @@ -36,10 +36,7 @@ type HMACStrategy struct { } const ( - // key should be at least 256 bit long, making it - minimumEntropy int = 32 - - // the secrets (client and global) should each have at least 16 characters making it harder to guess them + minimumEntropy = 32 minimumSecretLength = 32 ) @@ -51,27 +48,27 @@ func (c *HMACStrategy) Generate(ctx context.Context) (string, string, error) { c.Lock() defer c.Unlock() - secrets, err := c.Config.GetGlobalSecret(ctx) + globalSecret, err := c.Config.GetGlobalSecret(ctx) if err != nil { return "", "", err } - if len(secrets) < minimumSecretLength { - return "", "", errors.Errorf("secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got %d byte", len(secrets)) + if len(globalSecret) < minimumSecretLength { + return "", "", errors.Errorf("secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got %d byte", len(globalSecret)) } var signingKey [32]byte - copy(signingKey[:], secrets) + copy(signingKey[:], globalSecret) entropy := c.Config.GetTokenEntropy(ctx) if entropy < minimumEntropy { entropy = minimumEntropy } - // When creating secrets not intended for usage by human users (e.g., + // When creating tokens not intended for usage by human users (e.g., // client secrets or token handles), the authorization server should // include a reasonable level of entropy in order to mitigate the risk - // of guessing attacks. The token value should be >=128 bits long and + // of guessing attacks. The token value should be >=128 bits long and // constructed from a cryptographically strong random or pseudo-random // number sequence (see [RFC4086] for best current practice) generated // by the authorization server. @@ -91,34 +88,36 @@ func (c *HMACStrategy) Generate(ctx context.Context) (string, string, error) { func (c *HMACStrategy) Validate(ctx context.Context, token string) (err error) { var keys [][]byte - secrets, err := c.Config.GetGlobalSecret(ctx) + globalSecret, err := c.Config.GetGlobalSecret(ctx) if err != nil { return err } + if len(globalSecret) > 0 { + keys = append(keys, globalSecret) + } + rotatedSecrets, err := c.Config.GetRotatedGlobalSecrets(ctx) if err != nil { return err } - if len(secrets) > 0 { - keys = append(keys, secrets) + keys = append(keys, rotatedSecrets...) + + if len(keys) == 0 { + return errors.New("a secret for signing HMAC-SHA512/256 is expected to be defined, but none were") } - keys = append(keys, rotatedSecrets...) for _, key := range keys { if err = c.validate(ctx, key, token); err == nil { return nil } else if errors.Is(err, fosite.ErrTokenSignatureMismatch) { + // Continue to the next key. The error will be returned if it is the last key. } else { return err } } - if err == nil { - return errors.New("a secret for signing HMAC-SHA512/256 is expected to be defined, but none were") - } - return err } @@ -130,13 +129,11 @@ func (c *HMACStrategy) validate(ctx context.Context, secret []byte, token string var signingKey [32]byte copy(signingKey[:], secret) - split := strings.Split(token, ".") - if len(split) != 2 { + tokenKey, tokenSignature, ok := strings.Cut(token, ".") + if !ok { return errorsx.WithStack(fosite.ErrInvalidTokenFormat) } - tokenKey := split[0] - tokenSignature := split[1] if tokenKey == "" || tokenSignature == "" { return errorsx.WithStack(fosite.ErrInvalidTokenFormat) } @@ -161,13 +158,11 @@ func (c *HMACStrategy) validate(ctx context.Context, secret []byte, token string } func (c *HMACStrategy) Signature(token string) string { - split := strings.Split(token, ".") - - if len(split) != 2 { + _, sig, ok := strings.Cut(token, ".") + if !ok { return "" } - - return split[1] + return sig } func (c *HMACStrategy) generateHMAC(ctx context.Context, data []byte, key *[32]byte) []byte { diff --git a/token/hmac/hmacsha_test.go b/token/hmac/hmacsha_test.go index a5ae2218..546a1d3b 100644 --- a/token/hmac/hmacsha_test.go +++ b/token/hmac/hmacsha_test.go @@ -6,6 +6,7 @@ package hmac import ( "context" "crypto/sha512" + "fmt" "testing" "github.com/ory/fosite" @@ -23,113 +24,111 @@ func TestGenerateFailsWithShortCredentials(t *testing.T) { } func TestGenerate(t *testing.T) { - for _, c := range []struct { - globalSecret []byte - tokenEntropy int - }{ - { - globalSecret: []byte("1234567890123456789012345678901234567890"), - tokenEntropy: 32, - }, - { - globalSecret: []byte("1234567890123456789012345678901234567890"), - tokenEntropy: 64, - }, - } { - config := &fosite.Config{ - GlobalSecret: c.globalSecret, - TokenEntropy: c.tokenEntropy, - } - cg := HMACStrategy{Config: config} - - token, signature, err := cg.Generate(context.Background()) - require.NoError(t, err) - require.NotEmpty(t, token) - require.NotEmpty(t, signature) - t.Logf("Token: %s\n Signature: %s", token, signature) - - err = cg.Validate(context.Background(), token) - require.NoError(t, err) - - validateSignature := cg.Signature(token) - assert.Equal(t, signature, validateSignature) - - config.GlobalSecret = []byte("baz") - err = cg.Validate(context.Background(), token) - require.Error(t, err) + ctx := context.Background() + config := &fosite.Config{ + GlobalSecret: []byte("1234567890123456789012345678901234567890"), + } + cg := HMACStrategy{Config: config} + + for _, entropy := range []int{32, 64} { + t.Run(fmt.Sprintf("entropy=%d", entropy), func(t *testing.T) { + config.TokenEntropy = entropy + + token, signature, err := cg.Generate(ctx) + require.NoError(t, err) + require.NotEmpty(t, token) + require.NotEmpty(t, signature) + + err = cg.Validate(ctx, token) + require.NoError(t, err) + + actualSignature := cg.Signature(token) + assert.Equal(t, signature, actualSignature) + + config.GlobalSecret = append([]byte("not"), config.GlobalSecret...) + err = cg.Validate(ctx, token) + assert.ErrorIs(t, err, fosite.ErrTokenSignatureMismatch) + }) } } func TestValidateSignatureRejects(t *testing.T) { - var err error cg := HMACStrategy{ Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")}, } for k, c := range []string{ "", " ", - "foo.bar", + ".", "foo.", ".foo", } { - err = cg.Validate(context.Background(), c) - assert.Error(t, err) - t.Logf("Passed test case %d", k) + t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { + err := cg.Validate(context.Background(), c) + assert.ErrorIs(t, err, fosite.ErrInvalidTokenFormat) + }) } + + err := cg.Validate(context.Background(), "foo.bar") + assert.ErrorIs(t, err, fosite.ErrTokenSignatureMismatch) } func TestValidateWithRotatedKey(t *testing.T) { - old := HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")}} + ctx := context.Background() + oldGlobalSecret := []byte("1234567890123456789012345678901234567890") + old := HMACStrategy{Config: &fosite.Config{GlobalSecret: oldGlobalSecret}} now := HMACStrategy{Config: &fosite.Config{ GlobalSecret: []byte("0000000090123456789012345678901234567890"), RotatedGlobalSecrets: [][]byte{ []byte("abcdefgh90123456789012345678901234567890"), - []byte("1234567890123456789012345678901234567890"), + oldGlobalSecret, }, - }, - } + }} - token, _, err := old.Generate(context.Background()) + token, _, err := old.Generate(ctx) require.NoError(t, err) - require.EqualError(t, now.Validate(context.Background(), "thisisatoken.withaninvalidsignature"), fosite.ErrTokenSignatureMismatch.Error()) - require.NoError(t, now.Validate(context.Background(), token)) + assert.ErrorIs(t, now.Validate(ctx, "thisisatoken.withaninvalidsignature"), fosite.ErrTokenSignatureMismatch) + assert.NoError(t, now.Validate(ctx, token)) } func TestValidateWithRotatedKeyInvalid(t *testing.T) { - old := HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("1234567890123456789012345678901234567890")}} + ctx := context.Background() + oldGlobalSecret := []byte("1234567890123456789012345678901234567890") + old := HMACStrategy{Config: &fosite.Config{GlobalSecret: oldGlobalSecret}} now := HMACStrategy{Config: &fosite.Config{ GlobalSecret: []byte("0000000090123456789012345678901234567890"), RotatedGlobalSecrets: [][]byte{ []byte("abcdefgh90123456789012345678901"), - []byte("1234567890123456789012345678901234567890"), + oldGlobalSecret, }}, } - token, _, err := old.Generate(context.Background()) + token, _, err := old.Generate(ctx) require.NoError(t, err) - require.EqualError(t, now.Validate(context.Background(), token), "secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got 31 byte") + require.EqualError(t, now.Validate(ctx, token), "secret for signing HMAC-SHA512/256 is expected to be 32 byte long, got 31 byte") - require.EqualError(t, (&HMACStrategy{Config: &fosite.Config{}}).Validate(context.Background(), token), "a secret for signing HMAC-SHA512/256 is expected to be defined, but none were") + require.EqualError(t, (&HMACStrategy{Config: &fosite.Config{}}).Validate(ctx, token), "a secret for signing HMAC-SHA512/256 is expected to be defined, but none were") } func TestCustomHMAC(t *testing.T) { - def := HMACStrategy{Config: &fosite.Config{ - GlobalSecret: []byte("1234567890123456789012345678901234567890")}, - } - sha512 := HMACStrategy{Config: &fosite.Config{ - GlobalSecret: []byte("1234567890123456789012345678901234567890"), + ctx := context.Background() + globalSecret := []byte("1234567890123456789012345678901234567890") + defaultHasher := HMACStrategy{Config: &fosite.Config{ + GlobalSecret: globalSecret, + }} + sha512Hasher := HMACStrategy{Config: &fosite.Config{ + GlobalSecret: globalSecret, HMACHasher: sha512.New, - }, - } + }} - token, _, err := def.Generate(context.Background()) + token, _, err := defaultHasher.Generate(ctx) require.NoError(t, err) - require.EqualError(t, sha512.Validate(context.Background(), token), fosite.ErrTokenSignatureMismatch.Error()) + require.ErrorIs(t, sha512Hasher.Validate(ctx, token), fosite.ErrTokenSignatureMismatch) - token512, _, err := sha512.Generate(context.Background()) + token512, _, err := sha512Hasher.Generate(ctx) require.NoError(t, err) - require.NoError(t, sha512.Validate(context.Background(), token512)) - require.EqualError(t, def.Validate(context.Background(), token512), fosite.ErrTokenSignatureMismatch.Error()) + require.NoError(t, sha512Hasher.Validate(ctx, token512)) + require.ErrorIs(t, defaultHasher.Validate(ctx, token512), fosite.ErrTokenSignatureMismatch) }