Skip to content

crypto: support pre-hashed signing/verify#40

Merged
MichaelMure merged 1 commit into
masterfrom
prehashed
Apr 8, 2026
Merged

crypto: support pre-hashed signing/verify#40
MichaelMure merged 1 commit into
masterfrom
prehashed

Conversation

@MichaelMure

@MichaelMure MichaelMure commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Touches signature option parsing and hash selection, which can subtly affect signing/verification behavior across algorithms. RSA explicitly rejects PREHASHED, but other key types now exercise new code paths that must be validated for correctness and interoperability.

Overview
Introduces a new crypto.PREHASHED hash mode plus WithSigningPreHashed() to allow signing/verification when the caller provides an already-hashed digest (no additional hashing applied).

Updates the shared crypto test harness to run a prehashed sign/verify scenario and adds per-algorithm support flags (ECDSA curves and secp256k1 enabled; ed25519 and rsa disabled). RSA SignToASN1/VerifyASN1 now explicitly error/return false when PREHASHED is requested.

Reviewed by Cursor Bugbot for commit 1700a13. Bugbot is set up for automated code reviews on this repo. Configure here.

@MichaelMure MichaelMure requested a review from Copilot April 8, 2026 11:15
dovydas55
dovydas55 previously approved these changes Apr 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a “pre-hashed” signing/verification mode to the crypto abstraction by adding a new crypto.PREHASHED hash value, a convenience signing option to select it, and expanding the shared testsuite to validate behavior across key types.

Changes:

  • Add crypto.PREHASHED and wire it into crypto.Hash as an “identity” hash implementation.
  • Add crypto.WithSigningPreHashed() and extend the testsuite/harness to run pre-hashed sign/verify cases.
  • Explicitly reject PREHASHED for RSA sign/verify paths, while enabling it for ECDSA-based keys in tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crypto/hash.go Adds PREHASHED to the Hash enum and hooks it into String(), New(), and varsig mapping.
crypto/internal/hash.go Introduces an identity hash.Hash implementation used for PREHASHED.
crypto/options.go Adds WithSigningPreHashed() signing option.
crypto/_testsuite/testsuite.go Extends harness + suite to test pre-hashed signing/verification behavior.
crypto/rsa/private.go Rejects PREHASHED in RSA signing with an error.
crypto/rsa/public.go Rejects PREHASHED in RSA verification by returning false.
crypto/*/key_test.go Sets SupportsPrehashed for each algorithm’s harness (true for ECDSA curves/secp256k1; false for RSA/ed25519).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crypto/hash.go
Comment thread crypto/internal/hash.go
Comment thread crypto/options.go
Comment thread crypto/rsa/private.go

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1700a13. Configure here.

Comment thread crypto/internal/hash.go
@MichaelMure MichaelMure merged commit eb99032 into master Apr 8, 2026
7 checks passed
@MichaelMure MichaelMure deleted the prehashed branch April 8, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants