-
Notifications
You must be signed in to change notification settings - Fork 246
Revisit CCF crypto interfaces (mostly RSA/EC keys) #7425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revisit CCF crypto interfaces (mostly RSA/EC keys) #7425
Conversation
2f07a97 to
751f3e5
Compare
751f3e5 to
a7a4c47
Compare
There was a problem hiding this comment.
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 refactors the crypto API to explicitly separate EC (Elliptic Curve) and RSA key types, improving type safety and API clarity. The main changes include renaming PublicKey to ECPublicKey and KeyPair to ECKeyPair, removing problematic inheritance between RSA and EC key types, and introducing a KeyVariant type for handling different key types explicitly.
- Renamed core EC crypto types:
PublicKey→ECPublicKey,KeyPair→ECKeyPair - Separated RSA and EC key type hierarchies to eliminate error-prone inheritance
- Introduced
KeyVarianttemplate for explicit handling of different key types - Updated RSA API to use explicit padding parameters and unified method names
Reviewed Changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| include/ccf/crypto/key_variant.h | New file introducing KeyVariant template for handling different key types |
| include/ccf/crypto/ec_key_pair.h | Renamed from key_pair.h, now specifically for EC keys |
| include/ccf/crypto/ec_public_key.h | Renamed PublicKey class to ECPublicKey |
| include/ccf/crypto/rsa_key_pair.h | Updated RSA API to inherit from RSAPublicKey, unified method names |
| include/ccf/crypto/rsa_public_key.h | Added RSAPadding enum, removed inheritance, unified verify API |
| include/ccf/crypto/verifier.h | Updated to use KeyVariant for storing different key types |
| src/crypto/verifier.cpp | Implemented verify methods with KeyVariant handling |
| src/crypto/openssl/ec_key_pair.* | Renamed from key_pair.*, implementation for EC keys |
| src/crypto/openssl/ec_public_key.* | Renamed from public_key.*, implementation for EC public keys |
| src/crypto/openssl/rsa_public_key.* | Refactored to remove inheritance, unified verify API with padding |
| src/crypto/openssl/rsa_key_pair.* | Updated to inherit from RSAPublicKey_OpenSSL |
| src/crypto/key_pair.cpp | Deleted, moved to ec_key_pair.cpp |
| src/crypto/rsa_key_pair.cpp | Deleted, moved into openssl/rsa_key_pair.cpp |
| src/endpoints/authentication/jwt_auth.cpp | Updated to use ECPublicKey and new RSA verify API |
| src/js/extensions/ccf/crypto.cpp | Updated to use make_ec_public_key and unified JWK methods |
| doc/build_apps/crypto.rst | Updated documentation to reflect new type names |
| CHANGELOG.md | Added entry documenting the breaking API changes |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
4b97eb3 to
b66413f
Compare
Co-authored-by: Amaury Chamayou <amaury@xargs.fr>
Resolves #6673
PublicKey -> ECPublicKey,KeyPair -> ECKeyPair, etcccf::crypto::KeyVariantprovided for explicit handing of different key types✅ Long Test passed
☑️ AB perf: no regression
Mainly involved changes in RSA, other survived with no functional changes, the interface surface remains the same (except naming).
This's how it looks for RSA (and others, with probably extra non-exported files for historical reasons)