Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The checks consistently enforce full signature consumption, with focused regression specs covering all affected algorithms and certificate parsing.
No open issues 🎉
What changed in this PR
Tightens SSH signature verification to reject trailing bytes that were previously ignored, improving decoding correctness.
Changes:
- Validate full signature consumption for RSA, DSA, ECDSA, and Ed25519.
- Add regression specs for trailing data in direct verification and certificate signatures.
| File | Description |
|---|---|
spec/public_key/rsa_spec.rb |
Tests rejection of trailing RSA signature data. |
spec/public_key/ed25519_spec.rb |
Tests rejection of trailing Ed25519 signature data. |
spec/public_key/ecdsa_spec.rb |
Tests rejection of trailing ECDSA signature data. |
spec/public_key/dsa_spec.rb |
Tests rejection of trailing DSA signature data. |
spec/certificate_spec.rb |
Tests malformed certificate signatures across all four key types. |
lib/ssh_data/public_key/rsa.rb |
Checks that RSA signature decoding consumes all bytes. |
lib/ssh_data/public_key/ed25519.rb |
Checks that Ed25519 signature decoding consumes all bytes. |
lib/ssh_data/public_key/ecdsa.rb |
Checks that ECDSA signature decoding consumes all bytes. |
lib/ssh_data/public_key/dsa.rb |
Checks that DSA signature decoding consumes all bytes. |
Review effort: Max
Our signature decoder does not validate that the bytes read is the same as the total number of bytes. This permitted trailing data after signatures.
vcsjones
force-pushed
the
reject-sig-trailing-data
branch
from
October 27, 2026 17:37
044f565 to
5681054
Compare
vcsjones
changed the base branch from
main
to
remove-unsupported-ruby-from-ci
October 27, 2026 17:37
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.
Our signature decoder does not validate that the bytes read is the same as the total number of bytes. This permitted trailing data after signatures.
This isn't really a security issue because you still need a correct signature to validate. This is more of a correctness issue because it permits additional data in the signature that is more or less ignored.