Fix: Cleanup Crypto Issues#30
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens X.509 ECDSA signature verification to avoid cryptographically-weak “try multiple curves” behavior by requiring verification to use the curve explicitly declared in the issuer’s SubjectPublicKeyInfo, and by only accepting DER-encoded ECDSA signatures as required by X.509/RFC 5480.
Changes:
- Remove multi-curve ECDSA verification fallback and dispatch verification based on the declared EC named-curve OID in SPKI parameters.
- Restrict ECDSA signature verification helpers to DER-encoded signatures only, updating/adding tests to enforce the behavior.
- Add
AlgorithmIdentifier::parameters_oid()helpers (DER + RASN backends) and make small refactors in AES/account code paths.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| keetanetwork-x509/tests/builders.rs | Adds a regression test ensuring certificate verification requires a declared curve and fails for mismatched/absent curve parameters. |
| keetanetwork-x509/src/utils.rs | Updates ECDSA verification helpers to accept DER-only signatures and removes the multi-curve fallback verifier; adjusts tests accordingly. |
| keetanetwork-x509/src/certificates.rs | Adds verify_ecdsa_declared_curve() and routes ECDSA certificate signature verification through the SPKI-declared curve. |
| keetanetwork-crypto/src/algorithms/aes_ctr.rs | Minor refactor to avoid repeated as_ref() calls and make intermediate values explicit. |
| keetanetwork-crypto/src/algorithms/aes_common.rs | Minor refactor around IV/payload split return value. |
| keetanetwork-crypto/src/algorithms/aes_cbc.rs | Minor refactor to avoid repeated as_ref() calls and make intermediate values explicit. |
| keetanetwork-asn1/src/rasn.rs | Adds AlgorithmIdentifier::parameters_oid() for the RASN backend plus tests. |
| keetanetwork-asn1/src/der.rs | Adds AlgorithmIdentifier::parameters_oid() for the DER backend plus tests. |
| keetanetwork-account/src/account.rs | Minor refactor to avoid repeated as_ref() and make decrypted plaintext explicit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Merged
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.
Summary
Fixes various issues with cryptographic contexts.
Related Issues