PasswordRecipientInfoBuilder for CMS #1273
Conversation
|
@tarcieri Regarding the security alert: I assume, I have to wait for the |
cms/src/builder.rs
Outdated
| /// Provided password encryptor | ||
| pub key_encryptor: P, | ||
| /// Random number generator | ||
| pub rng: &'r mut R, |
There was a problem hiding this comment.
It could be worth changing the signature of RecipientInfoBuilder::build trait function to build_with_rng, just so we don't have to bind the rng like that.
There was a problem hiding this comment.
Or maybe not, it's not object-safe anymore.
There was a problem hiding this comment.
So we leave it as it is? Or what kind of binding would you prefer?
There was a problem hiding this comment.
I'm not too fan of the &mut R in an object. I think that makes it harder to use. You need two instances of the RNG: for example in the context of HSM, you might not have two references to the RNG easily (kough pkcs11 kough).
If we could bring in the rng "late" (it's already in the EnvelopedDataBuilder::build_with_rng context), that be great. But the Vec<Box<dyn RecipientInfoBuilder>> requires it to be object-safe, which makes it impossible.
I don't have a good solution to offer, but that's my concern here (not a blocker obviously).
There was a problem hiding this comment.
the only solution I could come up with is this one: bkstein#1
But this introduces some API breakage in the builder (but we can just push that in a cms-0.3)
Co-authored-by: Arthur Gautier <superbaloo+registrations.github@superbaloo.net>
Co-authored-by: Arthur Gautier <superbaloo+registrations.github@superbaloo.net>
|
(@bkstein if you rebase, the security audit error should go away. This is not actionable for now and we'll ignore it until RSA 0.10 is released) |
Obviously, this should not be forgotten. Co-authored-by: Arthur Gautier <superbaloo+registrations.github@superbaloo.net>
This requires two rng to be made available, which might not necessarily be available in some cases.
|
@baloo First, I apologize for the long time beeing absent. We were busy working on our project and we had this PR as a local patch. But I'm interested in making this official and would like to resume. I started by updating to the master branch and experienced problems with the new crate versions. The clippy error is due to incompatible versioning of some Dependencies (digest is referenced by ecdsa with version v0.11.0-pre.8, but I introduced pkdf2, which references digest v0.10.7): At the moment, I don't see how to resolve this. |
|
Ha, this is an oversight. I've bumped a bunch of dependencies in the ecosystem to pre-release versions but forgot pkcs5. See; #1391 (also: no need to apologize, we all have various priorities :)) |
|
@baloo I updated this PR to the latest master and dependencies. As for me, this can be merged. |
bkstein
changed the title
|
@baloo Reverted to draft, because I think, it's better to modify the rng handling. You mentioned above, that 2 rng's are now necessary for the pwri-builder. But it should be possible to get by with one rng, if I pass it from the builder to the encryption method. |
|
@baloo I tried to write a test for the |
You can grab it back in anycase:
Yes! Please grab it! |
|
@baloo I think, this PR is ready. However, there is a problem in CI with cargo-audit which is not related to my changes, I think. |
This provides a builder for CMS'
PasswordRecipientInfoaccording to RFC 3211.In contrast to the
KeyTransRecipientInfoBuilderthe key-encryption algorithm must be provided by the user (who has to provide an implementation ofPwriEncryptor). This allows for more flexibility in choosing allgorithms for key derivation and encryption.An example for using the implementation is in the new test.