-
Notifications
You must be signed in to change notification settings - Fork 156
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
Implement Signer/Verifier/Signature interfaces for the RSA signatures #174
Conversation
e211bef
to
76200c9
Compare
As far as the structure goes, I'd suggest doing something similar to the https://github.com/rustcrypto/elliptic-curves crates, which define the following types inside of modules named after the signature algorithm:
See the following modules as examples:
...then For PSS, parameters like the MGF digest can be generic parameters, e.g. pub struct SigningKey<Mgf: Digest> {
inner: RsaPrivateKey,
mgf: PhantomData<Mgf>,
[...]
} |
What about the |
@lumag yep! |
Move DummyRng to the separate module to allow it to be used from PaddingScheme module. Signed-off-by: Dmitry Baryshkov <[email protected]>
@tarcieri I have mostly finished the implementation of Signer/Verifier implementations. However I now have an issue with the PSS Signer. The trait passes self as non-mutable object, while the signing function uses salt rng as mutable. |
|
f1e596b
to
0e507d5
Compare
Signed-off-by: Dmitry Baryshkov <[email protected]>
Add tests for pkcs1v15 and pss signature verification functions to check that verifying invalid signatures returns an error. Signed-off-by: Dmitry Baryshkov <[email protected]>
@tarcieri pushed next iteration following your comments. |
Looking better! |
52ad0a5
to
cea23c7
Compare
@tarcieri done |
Looks mostly good to me now. @lumag can you update the PR description? |
Implement Signature, Signer and Verifier traits from signature crate. Signed-off-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
@tarcieri updated the description |
Is it necessary to couple pub struct Signature<T: AsRef<[u8]>> {
bytes: T
}
pub mod std_alloc {
pub type Signature = super::Signature<alloc::vec::Vec<u8>>;
}
// proper re-exports depending on `std` feature? |
I realized that |
@vccggorski the There's been some discussion of heapless support using stack-allocated bigints, e.g. via the
The constructor is fallible, so that is not a problem at all. The |
Initially I had a separate alloc-less |
Also by reading the code I got a little bit confused because my understanding of a difference between |
The The |
I did not see a particular value of implementing the |
@lumag the nice thing would be using a hardware accelerator for hashing, but given this crate isn't particularly embedded-friendly to begin with I think leaving that out would be fine for an MVP |
@tarcieri ok, I will add them to my queue to take a look after sorting out the |
I'm going to go ahead and merge this. @lumag feel free to follow up with |
Regarding the |
@sandhose @tarcieri Well, I followed the previous design of signing the pre-hashed messages, since this looks to me the way how the RSA signatures usually work. However I'm fine with changing that to use raw messages and hashing them during the sign procedure. The only question is about the raw PKCS1 v1.5 signatures (which do not have the ASN.1 wrapping). |
@sandhose good catch! The @lumag it would probably be good to retain inherent methods which operate on the raw bytes of a message digest, then implement traits like Generally working directly with raw digests is an antipattern. It's much less of a problem with RSA than it is with e.g. ECDSA or Schnorr though, where it can lead to existential forgeries. |
Refactor the
rsa
crate to use the API defined by thesignature
crate.This adds
pss
andpkcs1v15
modules, each of them providingSignature
,Verifier
andSigner
/RandomizedSigner
implementations.