Closed LWEdslev closed 5 months ago
Since all of these have a straightforward serialization to bytes, it would be good to use that, rather than custom derive.
@dignifiedquire perhaps we should use the serdect
crate for this like in e.g. https://github.com/rustcrypto/elliptic-curves ?
Since all of these have a straightforward serialization to bytes, it would be good to use that, rather than custom derive.
@dignifiedquire perhaps we should use the
serdect
crate for this like in e.g. https://github.com/rustcrypto/elliptic-curves ?
Why has serdect
not been used in src/key.rs PrivateKey
?
It hasn't been used in the rsa
crate at all yet, but has opinions around how data is serialized, namely it uses strategies which are constant time across various Serde serializers, regardless of if they're for binary or human readable formats (where hex is used to encode the latter), which is important when serializing private keys.
perhaps we should use the
serdect
So kinda like this?
impl Serialize for Signature {
fn serialize<S>(&self, serializer: S) -> core::result::Result<S::Ok, S::Error>
where
S: serdect::serde::Serializer,
{
serdect::slice::serialize_hex_lower_or_bin(&self.inner.to_bytes_be(), serializer)
}
}
impl<'de> Deserialize<'de> for Signature {
fn deserialize<D>(deserializer: D) -> core::result::Result<Self, D::Error>
where
D: serdect::serde::Deserializer<'de>,
{
let bytes = serdect::slice::deserialize_hex_or_bin_vec(deserializer)?;
let inner = BigUint::from_bytes_be(&bytes);
Ok(Self {
inner,
len: bytes.len(),
})
}
}
I can also do the others
sure đź‘Ť
I guess only Signature
has a straight forward serdect
implementation, since the other structs rely on RsaPublicKey
, RsaPrivateKey
and more, so that's gonna have to do for now.
@LWEdslev the elliptic-curve
crates use the SPKI serialization for public keys. You could do something similar for RsaPublicKey
and VerifyingKey
at least: computing the SPKI serialization with EncodePublicKey::to_public_key_der
, and then encoding that using serdect
.
@tarcieri I see, I'll give it a whirl
RsaPublicKey
, RsaPrivateKey
and the structs in pkcs1v15
now have the serdect
implementations
Should I add some serde_test
tests like the key::tests::test_serde
test for the new implementations?
Sure
Due to the PhantomData<D>
fields that has D: Digest
, I can't use PartialEq
on all the structs and therefore I can't use serde_test::assert_tokens
. I could not find a better solution than using serde_json
for those structs.
Can you post the code that isn’t working?
Can you post the code that isn’t working?
For the oaep::EncryptingKey
which uses the D: Digest
generic I tried something like this:
#[test]
#[cfg(feature = "serde")]
fn test_serde() {
use super::*;
use rand_chacha::{rand_core::SeedableRng, ChaCha8Rng};
use serde_test::{assert_tokens, Configure, Token};
let mut rng = ChaCha8Rng::from_seed([42; 32]);
let priv_key = crate::RsaPrivateKey::new(&mut rng, 64).expect("failed to generate key");
let encrypting_key = EncryptingKey::<sha2::Sha256>::new(priv_key.to_public_key());
let tokens = [
Token::Struct { name: "EncryptingKey", len: 2 },
Token::Str("inner"),
Token::Str("3024300d06092a864886f70d01010105000313003010020900cc6c6130e35b46bf0203010001"),
Token::Str("label"),
Token::Str("None"),
Token::StructEnd,
];
assert_tokens(&encrypting_key.readable(), &tokens);
}
To use the assert_tokens
I need to add PartialEq
to the EcnryptingKey
struct. Then I get an error when running cargo test --features serde
:
error[E0277]: can't compare `CoreWrapper<CtVariableCoreWrapper<Sha256VarCore, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>, OidSha256>>` with `CoreWrapper<CtVariableCoreWrapper<Sha256VarCore, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>, OidSha256>>`
Now all of them have the serde_test
tests
So what's next?
As a bit of a meta comment on the above suggestions: all of these types have built-in serialization support that the serde
impls can be thin wrappers for, rather than duplicating. It shouldn't be necessary to reach into inner
, for example
Yeah, that's better
How do I make the pss::SigningKey
use the correct algorithm OID when decoding?
EncodePrivateKey
uses pkcs1::ALGORITHM_ID
but ID_RSASSA_PSS
is required.
Edit: I see that this is addressed in #422
@LWEdslev either OID should work now (pkcs1::ALGORITHM_ID
or ID_RSASSA_PSS
), now that #424 is merged, though you may need to rebase
@tarcieri I don't see how either OID should work for the pss::SigningKey
.
With the merge of #424 the TryFrom<pkcs8::PrivateKeyInfo<'_>> for SigningKey<D>
asserts
private_key_info
.algorithm
.assert_algorithm_oid(ID_RSASSA_PSS)?;
ID_RSASSA_PSS
is the OID 1.2.840.113549.1.1.10
but the encoding is done with
self.inner.to_pkcs8_der()
And the inner RsaPrivateKey
has EncodePrivateKey
implemented such that algorithms (pkcs1::ALGORITHM_ID
) OID is 1.2.840.113549.1.1.1
So encoding a pss::SigningKey
it can't be decoded back into itself.
@LWEdslev aah, so it does. It should probably use verify_algorithm_id
which accepts either
@tarcieri Will you review this when you have time?
Yes
Added serde support (Serialization, Deserialization) for structs in
src/pkcs1v15/
:419