Repository navigation
fix: sign with ECDSA keys that have a short scalar - #47
Merged
Merged
Conversation
timweri
added
the
conformance
obelisk
approved these changes
Sep 24, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Security-sensitive invalid-scalar rejection paths need regression coverage.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Left-pads short ECDSA private scalars to the curve width before ring validation and signing.
Changes:
- Normalizes P-256 and P-384 scalar lengths securely.
- Adds signing round-trip tests with short scalars.
- Adds corresponding test keys.
| File | Description |
|---|---|
src/ssh/privkey.rs |
Pads and zeroizes ECDSA scalars. |
tests/signature-creation.rs |
Tests short-scalar signing. |
tests/keys/unencrypted/ecdsa_256_short_scalar |
Adds P-256 fixture. |
tests/keys/unencrypted/ecdsa_384_short_scalar |
Adds P-384 fixture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Flagged by Opus 5.5: When the first byte in the scalar is 0, sshcerts passes only 31 bytes down to ring, which require 32 bytes.
This now allows small and insecure scalars. This PR left-pad the small scalar and then does the safety check. This helps reject insecure keys.