Skip to content

fix(wbn-sign): request SHA-256 explicitly for ECDSA P-256 signatures - #920

Merged
zgroza merged 1 commit into
push-vzouqtlyzurofrom
push-mmvsovltmvvt
Oct 14, 2026
Merged

zgroza merged 1 commit into
push-vzouqtlyzurofrom
push-mmvsovltmvvt

Conversation

@zgroza

@zgroza zgroza commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Signing and verification passed no digest to crypto.sign()/crypto.verify() and relied on OpenSSL choosing SHA-256 for EC keys. Node.js builds based on BoringSSL have no default digest for EC keys and throw instead, so pass SHA-256 explicitly for ECDSA P-256 keys (and still no digest for Ed25519). The produced signatures are unchanged.

Add a test that emulates the missing default digest for both the ESM and the CommonJS build.

@zgroza
zgroza requested a review from rferens October 12, 2026 17:05
@zgroza
zgroza added this pull request to stack #921 October 12, 2026 17:05

@rferens rferens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch. LGTM

Comment thread js/sign/README.md Outdated
Signing and verification passed no digest to crypto.sign()/crypto.verify()
and relied on OpenSSL choosing SHA-256 for EC keys. Node.js builds based on
BoringSSL have no default digest for EC keys and throw instead, so pass
SHA-256 explicitly for ECDSA P-256 keys (and still no digest for Ed25519).
The produced signatures are unchanged.

Add a test that emulates the missing default digest for both the ESM and
the CommonJS build.
@zgroza
zgroza force-pushed the push-mmvsovltmvvt branch from a29c047 to 8bdde90 Compare October 14, 2026 12:47
@zgroza
zgroza merged commit ccc046d into main Oct 14, 2026
36 of 37 checks passed
@zgroza
zgroza deleted the push-mmvsovltmvvt branch October 14, 2026 12:54
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.

2 participants