Repository navigation
Add PrivateKey/PublicKey inherent methods for algorithms - #638
Conversation
|
Note: I only did I will say having two methods for when you do or don't have an RNG parameter is a lot easier than dealing with |
Should it be a cfg() ? This will also duplicate API surface and with #[inline] it will cause bigger code sizes - is the plan to remove the old API ? Ideally it would be good to gate PSS/PKCS in/out too ? |
It's the standard
If we landed this I would suggest deprecating |
Yea but it's always explicit API point where as this changes behaviour under the hood implicitly for anyone who hasn't asked for it (? especially in case any other crate includes the crate with the non-bifurcated feature.
This will make prevent adopting trait based key data injection (if that were to be supported) |
f8fb2d2 to
5dc15f7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #638 +/- ##
==========================================
- Coverage 92.89% 90.29% -2.60%
==========================================
Files 29 29
Lines 3981 4091 +110
==========================================
- Hits 3698 3694 -4
- Misses 283 397 +114 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5dc15f7 to
d3141b8
Compare
tarcieri
changed the title
RsaPrivateKey/RsaPublicKey inherent methodsRsaPrivateKey/RsaPublicKey inherent methods
tarcieri
changed the title
RsaPrivateKey/RsaPublicKey inherent methodsPrivateKey/PublicKey inherent methods
f092d5f to
5b2a1f5
Compare
tarcieri
changed the title
PrivateKey/PublicKey inherent methodsPrivateKey/PublicKey inherent methods for algorithms
5b2a1f5 to
849a12b
Compare
Adds algorithm-specific inherent methods for encryption and signing prefixed with the algorithm name, e.g. `pkcs1v15_decrypt`, `pkcs1v15_sign`, as an alternative to the `PaddingScheme` and `SignatureScheme` traits. We already have an entirely separate trait-based API from `PaddingScheme` and `SignatureScheme`, namely traits like `RandomizedEncryptor`, `RandomizedDecryptor`, and `Decryptor`, along with the traits from the `signature` crate. These traits are well-typed and enforce usage of a particular algorithm's family of types, but also seem to have overlapping responsibilities / use cases to the `PaddingScheme` and `SignatureScheme` traits. The current `PaddingScheme` and `SignatureScheme` traits do let you mix encryption and signing operations with a single key type (along with different algorithms), but they're a little awkward to use and I don't think we're getting a whole lot out of the trait-based abstraction they provide which isn't provided better by the aforementioned well-typed traits. The goal of being able to execute any operation of any algorithm you want on a given key type can also be met by just having a bunch of methods which do exactly what you want, which is what this PR proposes.
849a12b to
50bab3a
Compare
|
Gonna land this. I think the docs speak for themselves that this is a cleaner API. It still needs tests but I'd like to merge this and get it out in a release candidate for feedback then followup with tests. |
|
People should be migrating to 0.10 for security reasons and it should be friction less as possible? There is always 0.10.1 and additive surface should come later especially when it does not add anything re: mis-use resistance? There has been now like 20 release candidates and this can wait? |
|
I agree it should be as frictionless as possible an upgrade, however adding this does not add upgrade friction. The previous API remains, and isn't even deprecated. Edit: it's already merged, going forward with it |
|
Yeah but is it going to block a release for security issues? EDIT: Could always gate it into release as unstable surface. |
|
It's not blocking anything. It's merged and ready to ship. I'm not sure what you're suggesting: that I revert it, ship v0.10.0 without it, then unrevert it and ship v0.10.1? That doesn't make any sense. |
|
The indication was that there would be 21st release candidate before 0.10 because of this PR for collating feeback on this new API ? The additive surface can be added to 0.10.1 without this landing in 0.10 ? |
|
I'm doing a rc.22 now: #785 I am not going to revert this just because you'd prefer it shipped in v0.10.1. That just seems completely pointless to me. This is a breaking release that includes a number of different changes already, some of them security improvements, some of them just general API changes. It has been designed to be largely API compatible with previous releases, however, because it is a security-critical upgrade. Additive changes don't get in the way of that. |
|
I thought the idea was just to release and not add more release candidates but ok having more time to review existing changes and then also new API surface ok. EDIT: I can understand having more rc's given other changes but it all just adds up and more frition to delay 0.10 as sum ... |
|
I am also trying to close out as many remaining issues as possible that require breaking changes, before cutting a breaking release |
|
yea fair |
Adds tests for the new encryption/signing inherent methods added in #638
Adds tests for the new encryption/signing inherent methods added in #638
#82) * Add `PrivateKey`/`PublicKey` inherent methods for algorithms (RustCrypto#638) Adds algorithm-specific inherent methods for encryption and signing prefixed with the algorithm name, e.g. `pkcs1v15_decrypt`, `pkcs1v15_sign`, as an alternative to the `PaddingScheme` and `SignatureScheme` traits. We already have an entirely separate trait-based API from `PaddingScheme` and `SignatureScheme`, namely traits like `RandomizedEncryptor`, `RandomizedDecryptor`, and `Decryptor`, along with the traits from the `signature` crate. These traits are well-typed and enforce usage of a particular algorithm's family of types, but also seem to have overlapping responsibilities / use cases to the `PaddingScheme` and `SignatureScheme` traits. The current `PaddingScheme` and `SignatureScheme` traits do let you mix encryption and signing operations with a single key type (along with different algorithms), but they're a little awkward to use and I don't think we're getting a whole lot out of the trait-based abstraction they provide which isn't provided better by the aforementioned well-typed traits. The goal of being able to execute any operation of any algorithm you want on a given key type can also be met by just having a bunch of methods which do exactly what you want, which is what this PR proposes. Fork adaptation: the inherent methods are on the alloc key types only and are gated on `alloc`. The doc examples that call `Generate::generate` need `keygen` as well as `getrandom`, since the fork gates `Generate` on `keygen`. `pss_verify_prehash` and `pkcs1v15_verify_prehash` delegate to the fork's `Pss` and `Pkcs1v15Sign` verify paths, which perform the same checks; the fork has no byte-slice `verify` helpers. Upstream's README hunk goes to UPSTREAM_README.md. (cherry picked from commit 6ab482b) * Make `#![no_std]` unconditional (RustCrypto#784) Previously it was gated on `not(test)`. Only a tiny number of changes, mostly just imports, are needed to eliminate the conditional gating. Fork adaptation: the one conflict is a test the fork had already moved to `core::mem::swap`; it takes upstream's `mem::swap` with the new import. (cherry picked from commit 58c89a6) * Test the per-algorithm inherent key methods Upstream covers the new PublicKey/PrivateKey methods only from doc examples, which the coverage job does not run, so project coverage fell below the codecov target. Round-trip each method, with and without an explicit RNG, plus the hash-length and wrong-message rejections. --------- Co-authored-by: Tony Arcieri <bascule@gmail.com>
## Security - Replace `num-bigint-dig` with `crypto-bigint` for constant-time modular exponentiation (#394) - Redact private-key components from `Debug` output (#480) - Harden private-key operations following `draft-irtf-cfrg-rsa-guidance` (#710, #715, #739, #751, #752, #755, #756, #758, #770, #771, #773) - Zeroize stored secret-prime Montgomery parameters in `PrecomputedValues` (#716) - Implement PKCS#1 v1.5 implicit rejection: return deterministic synthetic plaintext on invalid padding instead of exposing a padding error and with it a sidechannel (#726, #729, #749) ## Added - PKCS#8/SPKI decoding for PSS keys, including `id-RSASSA-PSS` algorithm identifiers (#424) - `pss::VerifyingKey::salt_len` and opt-in automatic salt-length detection via `new_with_auto_salt_len` (#448, #546) - Wycheproof test coverage for PKCS#1 v1.5, PSS, and OAEP (#477, #740) - `PublicKeyParts::n_bytes` and `e_bytes` serialization helpers (#519) - Multipart signing trait implementations for PKCS#1 v1.5 and PSS (#525) - `Clone` and `core::error::Error` implementations for `Error` (#530, #694) - `PrivateKey::as_public_key` accessor (#592) - Algorithm-specific inherent methods on `PrivateKey` and `PublicKey` for OAEP encryption/decryption and PKCS#1 v1.5/PSS signing/verification, including prehash and explicit-RNG variants (#638) - `PrivateKey::from_components_with_large_exponent`, gated on `hazmat` (#648) - Constant-time validation harness using `cargo checkct`/BINSEC (#732, #733, #749, #756, #771, #773) - `PrivateKey::from_components_with_wide_primes` for unbalanced two-prime keys, which ordinary constructors now reject (#755) - `TryFrom<BitStringRef<'_>>` for PKCS#1 v1.5 and PSS signatures (#764) - `crypto_common::Generate` implementation and `Generate` re-export for generating 3072-bit keys with public exponent 65537 (#780) ## Changed - Replace `BigUint` in the public API with `BoxedUint`; return `NonZero<BoxedUint>` for the modulus and `Odd<BoxedUint>` for prime factors (#394, #735) - Use `crypto-primes` for prime generation (#394) - Use DER-based key encodings and hex-or-binary serde representations via `serdect`; extend serde support to OAEP, PKCS#1 v1.5, and PSS wrapper types (#420) - Accept arbitrary byte strings as OAEP labels (#467) - Gate PKCS#1/PKCS#8/SPKI support on the default-enabled `encoding` feature; `std` no longer implicitly enables encoding dependencies (#537, #543) - Enforce a 1024-bit minimum for key generation and default public-key construction/decoding; add `PublicKey::new_with_size_limits` and `hazmat`-gated unchecked key generation (#576, #760) - Bump `pkcs1` to v0.8; `pkcs8` to v0.11; `spki` to v0.8; `serdect` to v0.4 (#584, #684, #688, #766) - Make `Oaep` and `Pss` generic over their digest types instead of using `DynDigest` (#593) - Bump `rand_core` to v0.10, `crypto-common` to v0.2, `digest` to v0.11, and `signature` to v3 (#650, #671, #693) - Remove a redundant allocation during OAEP decryption (#659) - Improve encryption and verification performance by bounding exponentiation to the public exponent's bit length (#662) - Bump `crypto-bigint` to v0.7.10 and `crypto-primes` to v0.7 (#673, #766) - Raise the default maximum public-key modulus size to 8192 bits (#681) - Bump `sha1` and `sha2` to v0.11 (#683) - Store PKCS#1 v1.5 and PSS signatures as byte strings, preserving their encoded width (#722, #723) - MSRV 1.87; Rust 2024 edition (#731, #757) - Enable `getrandom` by default and use the system RNG for blinding in default decryption and signing paths (#739, #758) - Limit signature decoding to 1024 bytes by default; add `Signature::from_bytes_with_max_len` for explicit limits (#767) - Require the opt-in `pkcs1v15` feature for PKCS#1 v1.5 encryption and signatures (#778) - Accept fallible RNGs through `TryCryptoRng` and propagate RNG failures as errors (#779) - Rename `RsaPrivateKey` and `RsaPublicKey` to `PrivateKey` and `PublicKey`, retaining the old names as deprecated aliases (#781) ## Deprecated - `pss::BlindedSigningKey` in favor of `pss::SigningKey`, which now applies blinding by default (#758) - `PublicKey::new_with_max_size` in favor of `PublicKey::new_with_size_limits` (#760) ## Removed - `nightly` and `u64_digit` features associated with `num-bigint-dig` (#394) - Deprecated `Pkcs1v15Sign::new_raw` and `pkcs1v15` `*_with_prefix` constructors (#524) - Separate `pem` feature; PEM support is included with `encoding` (#537) - Optional `sha2` feature; SHA-2 support is now always enabled (#749) ## Fixed - Honor explicit larger-key construction limits during cryptographic operations, including OAEP decryption (#441, #595) - PKCS#1 v1.5 `SigningKey::from` and `VerifyingKey::from` now use the digest prefix consistent with `new` (#472, #498) - Check the modulus's actual bit length instead of its storage precision when enforcing size limits (#529) - Retry zero blinding factors instead of replacing them with one (#653) - Reject private-key factors less than or equal to one, even or repeated factors, and private exponents greater than or equal to the modulus (#655, #738, #772, #774) - Reject PSS salt lengths that cannot be represented by the `AlgorithmIdentifier` encoder instead of truncating them (#704) - Apply RFC 8017 encoded-message width rules for PSS, including moduli whose bit length is congruent to 1 modulo 8 (#725) - Return verification errors instead of panicking for undersized PSS encoded messages and overflowing salt lengths (#769) - Propagate CRT precomputation errors from private-key constructors (#782)
Adds algorithm-specific inherent methods for encryption and signing prefixed with the algorithm name, e.g.
pkcs1v15_decrypt,pkcs1v15_sign, as an alternative to thePaddingSchemeandSignatureSchemetraits.We already have an entirely separate trait-based API from
PaddingSchemeandSignatureScheme, namely traits likeRandomizedEncryptor,RandomizedDecryptor, andDecryptor, along with the traits from thesignaturecrate. These traits are well-typed and enforce usage of a particular algorithm's family of types, but also seem to have overlapping responsibilities / use cases to thePaddingSchemeandSignatureSchemetraits.The current
PaddingSchemeandSignatureSchemetraits do let you mix encryption and signing operations with a single key type (along with different algorithms), but they're a little awkward to use and I don't think we're getting a whole lot out of the trait-based abstraction they provide which isn't provided better by the aforementioned well-typed traits.The goal of being able to execute any operation of any algorithm you want on a given key type can also be met by just having a bunch of methods which do exactly what you want, which is what this PR proposes.