Skip to content

🛡️ Sentinel: [MEDIUM] Add VerifyHMACSha256 for secure HMAC verification #32

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.

Sign up for GitHub

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

Closed
Laisky wants to merge 1 commit into from
Closed

🛡️ Sentinel: [MEDIUM] Add VerifyHMACSha256 for secure HMAC verification #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions crypto/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,25 @@ func HMACSha256(key []byte, data io.Reader) ([]byte, error) {

return h.Sum(nil), nil
}

// VerifyHMACSha256 verify HMAC by sha256
//
// # Args:
// - key: secure key, no limit on length
// - data: raw data to verify HMAC
// - signature: HMAC signature to verify
//
// # Returns:
// - err: nil if signature match
func VerifyHMACSha256(key, signature []byte, data io.Reader) error {
Comment on lines +330 to +339
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment for VerifyHMACSha256 lists args in the order key, data, signature, but the function signature is func VerifyHMACSha256(key, signature []byte, data io.Reader). Please update the comment (or reorder parameters) so the documented argument order matches the actual API and avoids misuse by callers.

Copilot uses AI. Check for mistakes.
h, err := HMACSha256(key, data)
if err != nil {
return errors.Wrap(err, "calculate hmac")
}

if !hmac.Equal(h, signature) {
return errors.New("signature not match")
Comment on lines +345 to +346
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VerifyHMACSha256 accepts any signature length and relies on hmac.Equal. hmac.Equal returns immediately when lengths differ, which undermines the goal of constant-time verification and also differs from other codepaths that enforce a 32-byte HMAC (e.g., SM4 decrypt validates HMAC length). Consider explicitly validating that signature is exactly 32 bytes (sha256 size) and returning an error otherwise.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error string "signature not match" is both grammatically incorrect and inconsistent with other verification helpers in this package (e.g., VerifyByEd25519WithSHA512 returns "invalid signature", and SM4 HMAC verification returns "hmac not match"). Consider using a consistent message (and update tests accordingly) so callers can reliably match/handle verification failures.

Suggested change
return errors.New("signature not match")
return errors.New("hmac not match")

Copilot uses AI. Check for mistakes.
}

return nil
}
36 changes: 36 additions & 0 deletions crypto/sign_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,3 +700,39 @@ func TestHMAC(t *testing.T) {
})
}
}

func TestVerifyHMACSha256(t *testing.T) {
t.Parallel()

key := []byte("secret-key")
content := []byte("hello, world")

t.Run("valid signature", func(t *testing.T) {
sig, err := HMACSha256(key, bytes.NewReader(content))
require.NoError(t, err)

err = VerifyHMACSha256(key, sig, bytes.NewReader(content))
require.NoError(t, err)
})

t.Run("invalid signature", func(t *testing.T) {
err := VerifyHMACSha256(key, []byte("invalid-sig"), bytes.NewReader(content))
require.ErrorContains(t, err, "signature not match")
})

t.Run("invalid key", func(t *testing.T) {
sig, err := HMACSha256(key, bytes.NewReader(content))
require.NoError(t, err)

err = VerifyHMACSha256([]byte("wrong-key"), sig, bytes.NewReader(content))
require.ErrorContains(t, err, "signature not match")
})

t.Run("invalid content", func(t *testing.T) {
sig, err := HMACSha256(key, bytes.NewReader(content))
require.NoError(t, err)

err = VerifyHMACSha256(key, sig, bytes.NewReader([]byte("wrong-content")))
require.ErrorContains(t, err, "signature not match")
})
}
Loading