-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Migrate parsing of private keys to Rust #12296
base: main
Are you sure you want to change the base?
Conversation
81ab411
to
66e7aec
Compare
paramiko failures are due to paramiko/paramiko#2490 (probably) |
test cases needed to coverage:
|
daeef67
to
3250d10
Compare
Test cases we need:
|
5fd4c86
to
b72b393
Compare
To be used in pyca#12296
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.
Copilot reviewed 6 out of 18 changed files in this pull request and generated no comments.
Files not reviewed (12)
- .github/downstream.d/paramiko.sh: Language not supported
- tests/hazmat/primitives/test_serialization.py: Evaluated as low risk
- tests/hazmat/primitives/test_dsa.py: Evaluated as low risk
- tests/hazmat/primitives/test_rsa.py: Evaluated as low risk
- src/rust/cryptography-x509/src/pkcs8.rs: Evaluated as low risk
- tests/hazmat/backends/test_openssl.py: Evaluated as low risk
- src/rust/cryptography-key-parsing/src/lib.rs: Evaluated as low risk
- tests/hazmat/primitives/test_ec.py: Evaluated as low risk
- src/rust/src/backend/ec.rs: Evaluated as low risk
- src/rust/src/error.rs: Evaluated as low risk
- src/rust/cryptography-key-parsing/Cargo.toml: Evaluated as low risk
- src/rust/cryptography-key-parsing/src/rsa.rs: Evaluated as low risk
Comments suppressed due to low confidence (6)
src/rust/cryptography-key-parsing/src/pkcs8.rs:111
- [nitpick] The error message for unsupported key types might not be clear enough for the user to understand what went wrong. It should provide more context about the unsupported key type.
Err(KeyParsingError::UnsupportedKeyType(k.algorithm.oid().clone()))
src/rust/cryptography-key-parsing/src/pkcs8.rs:21
- The function
parse_private_key
handles various algorithm parameters, but there is no indication that each case is covered by tests. Ensure that test cases exist for each algorithm parameter handled in this function.
pub fn parse_private_key(data: &[u8]) -> KeyParsingResult<openssl::pkey::PKey<openssl::pkey::Private>> {
src/rust/src/backend/keys.rs:100
- Ensure that similar errors are handled consistently throughout the code.
return Err(CryptographyError::from(pyo3::exceptions::PyTypeError::new_err("Password was not given but private key is encrypted")));
src/rust/src/backend/keys.rs:78
- [nitpick] Avoid redundant clone operations to improve performance.
let tag = p.tag().to_string();
src/rust/src/backend/keys.rs:112
- Ensure that all possible ciphers are covered or that the error message provides enough information for debugging.
return Err(CryptographyError::from(pyo3::exceptions::PyValueError::new_err("Key encrypted with unknown cipher.")));
src/rust/cryptography-key-parsing/src/dsa.rs:17
- [nitpick] The function name parse_pkcs1_private_key is misleading as PKCS#1 is typically associated with RSA keys, not DSA keys. Consider renaming it to parse_dsa_private_key.
pub fn parse_pkcs1_private_key(
This reverts commit 0eec1a3.
No description provided.