Skip to content
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.

Sign up for GitHub

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

core: Update to asn1_der v0.7 #2000

Merged
merged 8 commits into from
Mar 22, 2021
Merged

core: Update to asn1_der v0.7 #2000

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f42bdd6
core: Update to asn1_der v0.7
mxinden Mar 15, 2021
f73d672
core/identity/rsa: Use Sequence::get_as
mxinden Mar 15, 2021
2633d72
core/src/identity: Apply suggestions from review
mxinden Mar 19, 2021
32625a9
Merge branch 'libp2p/master' into asn1_der_v0.7
mxinden Mar 19, 2021
879c3ac
core/: Revert testing with libp2p-core v0.26
mxinden Mar 22, 2021
77a6c7f
*: Update changelogs and cargo tomls
mxinden Mar 22, 2021
62443d8
Merge branch 'libp2p/master' into asn1_der_v0.7
mxinden Mar 22, 2021
c19170b
core/src/identity/rsa: Wrap import line
mxinden Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ keywords = ["peer-to-peer", "libp2p", "networking"]
categories = ["network-programming", "asynchronous"]

[dependencies]
asn1_der = "0.6.1"
asn1_der = "0.7.3"
bs58 = "0.4.0"
ed25519-dalek = "1.0.1"
either = "1.5"
Expand Down Expand Up @@ -47,6 +47,8 @@ libp2p-tcp = { path = "../transports/tcp" }
multihash = { version = "0.13", default-features = false, features = ["arb"] }
quickcheck = "0.9.0"
wasm-timer = "0.2"
# TODO: For testing only. Remove.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will remove this before merging. Leaving it for now to ensure I don't introduce a regression.

libp2p-core-v026 = { version = "0.26.0", package = "libp2p-core" }

[build-dependencies]
prost-build = "0.7"
Expand Down
172 changes: 119 additions & 53 deletions core/src/identity/rsa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@

//! RSA keys.

use asn1_der::{Asn1Der, FromDerObject, IntoDerObject, DerObject, DerTag, DerValue, Asn1DerError};
use lazy_static::lazy_static;
use asn1_der::{typed::{DerEncodable, DerDecodable, Sequence}, DerObject, Asn1DerError, Asn1DerErrorVariant};
use super::error::*;
use ring::rand::SystemRandom;
use ring::signature::{self, RsaKeyPair, RSA_PKCS1_SHA256, RSA_PKCS1_2048_8192_SHA256};
Expand Down Expand Up @@ -93,15 +92,16 @@ impl PublicKey {
},
subjectPublicKey: Asn1SubjectPublicKey(self.clone())
};
let mut buf = vec![0u8; spki.serialized_len()];
spki.serialize(buf.iter_mut()).map(|_| buf)
.expect("RSA X.509 public key encoding failed.")
let mut buf = Vec::new();
let buf = spki.encode(&mut buf).map(|_| buf)
.expect("RSA X.509 public key encoding failed.");
buf
}

/// Decode an RSA public key from a DER-encoded X.509 SubjectPublicKeyInfo
/// structure. See also `encode_x509`.
pub fn decode_x509(pk: &[u8]) -> Result<PublicKey, DecodingError> {
Asn1SubjectPublicKeyInfo::deserialize(pk.iter())
Asn1SubjectPublicKeyInfo::decode(pk)
.map_err(|e| DecodingError::new("RSA X.509").source(e))
.map(|spki| spki.subjectPublicKey.0)
}
Expand All @@ -123,93 +123,148 @@ impl fmt::Debug for PublicKey {
// Primer: http://luca.ntop.org/Teaching/Appunti/asn1.html
// Playground: https://lapo.it/asn1js/

lazy_static! {
/// The DER encoding of the object identifier (OID) 'rsaEncryption' for
/// RSA public keys defined for X.509 in [RFC-3279] and used in
/// SubjectPublicKeyInfo structures defined in [RFC-5280].
///
/// [RFC-3279]: https://tools.ietf.org/html/rfc3279#section-2.3.1
/// [RFC-5280]: https://tools.ietf.org/html/rfc5280#section-4.1
static ref OID_RSA_ENCRYPTION_DER: DerObject =
DerObject {
tag: DerTag::x06,
value: DerValue {
data: vec![ 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01 ]
}
};
}
const ASN1_OBJECT_IDENTIFIER_TAG: u8 = 6;
/// The DER encoding of the object identifier (OID) 'rsaEncryption' for
/// RSA public keys defined for X.509 in [RFC-3279] and used in
/// SubjectPublicKeyInfo structures defined in [RFC-5280].
///
/// [RFC-3279]: https://tools.ietf.org/html/rfc3279#section-2.3.1
/// [RFC-5280]: https://tools.ietf.org/html/rfc5280#section-4.1
const ASN1_RSA_ENCRYPTION_OID: [u8;9] = [ 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01 ];

/// The ASN.1 OID for "rsaEncryption".
#[derive(Clone)]
struct Asn1OidRsaEncryption();

impl IntoDerObject for Asn1OidRsaEncryption {
fn into_der_object(self) -> DerObject {
OID_RSA_ENCRYPTION_DER.clone()
}
fn serialized_len(&self) -> usize {
OID_RSA_ENCRYPTION_DER.serialized_len()
impl asn1_der::typed::DerEncodable for Asn1OidRsaEncryption {
fn encode<S: asn1_der::Sink>(&self, sink: &mut S) -> Result<(), asn1_der::Asn1DerError> {
// TODO: `DerObject::write` is a hidden method. Should one use `new` or `new_from_source`?
// If so, I don't see a way to do that given that `S: Into<&'a [u8]` would be required.
asn1_der::DerObject::write(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the way to go?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the use of write is correct when serializing a new type; I use it for the same purpose and it should not be hidden 🙈

However when defining a new type you should put the implementation into a DerTypeView; e.g.:

/// A raw OID
#[derive(Copy, Clone)]
struct Asn1RawOid<'a> {
    object: DerObject<'a>
}
impl<'a> Asn1RawOid<'a> {
    /// Wraps an OID byte literal
    pub fn new<S: Sink + Into<&'a[u8]>>(oid: &[u8], sink: S) -> Result<Self, asn1_der::Asn1DerError> {
        let object = DerObject::new(Self::TAG, oid, sink)?;
        Ok(Self { object })
    }

    /// The underlying OID as byte literal
    pub fn oid(&self) -> &[u8] {
        self.object.value()
    }

    /// Writes an OID raw `value` as DER-object to `sink`
    pub fn write<S: Sink>(value: &[u8], sink: &mut S) -> Result<(), Asn1DerError> {
        DerObject::write(Self::TAG, value.len(), &mut value.iter(), sink)
    }
}
impl<'a> asn1_der::typed::DerTypeView<'a> for Asn1RawOid<'a> {
    const TAG: u8 = 6;

    fn object(&self) -> DerObject<'a> {
        self.object
    }
}
impl<'a> asn1_der::typed::DerEncodable for Asn1RawOid<'a> {
    fn encode<S: asn1_der::Sink>(&self, sink: &mut S) -> Result<(), asn1_der::Asn1DerError> {
        self.object.encode(sink)
    }
}
impl<'a> asn1_der::typed::DerDecodable<'a> for Asn1RawOid<'a> {
    fn load(object: DerObject<'a>) -> Result<Self, Asn1DerError> {
        // Validate the tag
        if object.tag() != Self::TAG {
            return Err(Asn1DerError::new(
                Asn1DerErrorVariant::InvalidData("DER object tag is not the object identifier tag."),
            ));
        }

        // Load self
        Ok(Self { object })
    }
}

(This is because the general approach of this crate is view-based; i.e. the core implementations do not own any bytes).

Now you can easily implement a native type upon it:

/// An 'rsaEncryption' OID
#[derive(Copy, Clone)]
struct Asn1OidRsaEncryption;
impl Asn1OidRsaEncryption {
    /// The DER encoding of the object identifier (OID) 'rsaEncryption' for
    /// RSA public keys defined for X.509 in [RFC-3279] and used in
    /// SubjectPublicKeyInfo structures defined in [RFC-5280].
    ///
    /// [RFC-3279]: https://tools.ietf.org/html/rfc3279#section-2.3.1
    /// [RFC-5280]: https://tools.ietf.org/html/rfc5280#section-4.1
    const OID: &'static[u8] = b"\x2A\x86\x48\x86\xF7\x0D\x01\x01\x01";
}
impl<'a> DerDecodable<'a> for Asn1OidRsaEncryption {
    fn load(object: DerObject<'a>) -> Result<Self, Asn1DerError> {
        match Asn1RawOid::load(object)?.oid() {
            Self::OID => Ok(Self),
            _ => Err(Asn1DerError::new(
                Asn1DerErrorVariant::InvalidData("DER object is not the 'rsaEncryption' identifier."),
            ))
        }
    }
}
impl DerEncodable for Asn1OidRsaEncryption {
    fn encode<S: Sink>(&self, sink: &mut S) -> Result<(), Asn1DerError> {
        Asn1RawOid::write(Self::OID, sink)
    }
}

If it makes things easier for you, I can also add a raw OID type to the core crate; I'm still a bit reluctant about something that understands OIDs, but an adapter to work with the raw bytes is no problem anymore IMO.

ASN1_OBJECT_IDENTIFIER_TAG,
ASN1_RSA_ENCRYPTION_OID.len(),
&mut ASN1_RSA_ENCRYPTION_OID.iter(),
sink,
)?;
Ok(())
}
}

impl FromDerObject for Asn1OidRsaEncryption {
fn from_der_object(o: DerObject) -> Result<Self, Asn1DerError> {
if o.tag != DerTag::x06 {
return Err(Asn1DerError::InvalidTag)
impl DerDecodable<'_> for Asn1OidRsaEncryption {
fn load(object: DerObject<'_>) -> Result<Self, Asn1DerError> {
if object.tag() != ASN1_OBJECT_IDENTIFIER_TAG {
return Err(Asn1DerError::new(
Asn1DerErrorVariant::InvalidData("DER object tag is not the object identifier tag."),
));
}
if o.value != OID_RSA_ENCRYPTION_DER.value {
return Err(Asn1DerError::InvalidEncoding)

if object.value() != ASN1_RSA_ENCRYPTION_OID {
return Err(Asn1DerError::new(
Asn1DerErrorVariant::InvalidData("DER object is not the 'rsaEncryption' identifier."),
));
}

Ok(Asn1OidRsaEncryption())
}
}

/// The ASN.1 AlgorithmIdentifier for "rsaEncryption".
#[derive(Asn1Der)]
struct Asn1RsaEncryption {
algorithm: Asn1OidRsaEncryption,
parameters: ()
}

impl asn1_der::typed::DerEncodable for Asn1RsaEncryption {
fn encode<S: asn1_der::Sink>(&self, sink: &mut S) -> Result<(), asn1_der::Asn1DerError> {
// TODO: Is there a way to write to `sink` directly?
let mut algorithm = Vec::new();
self.algorithm.encode(&mut algorithm)?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the way to go?

Copy link

@KizzyCode KizzyCode Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well in general yes; however I've added an additional API to directly create a DerObject from an encodable object:

/// The ASN.1 AlgorithmIdentifier for "rsaEncryption".
struct Asn1RsaEncryption {
    algorithm: Asn1OidRsaEncryption,
    parameters: ()
}
impl<'a> DerDecodable<'a> for Asn1RsaEncryption {
    fn load(object: DerObject<'a>) -> Result<Self, Asn1DerError> {
        let sequence = Sequence::load(object)?;

        Ok(Self {
            algorithm: sequence.get_as(0)?,
            parameters: sequence.get_as(1)?
        })
    }
}
impl DerEncodable for Asn1RsaEncryption {
    fn encode<S: Sink>(&self, sink: &mut S) -> Result<(), Asn1DerError> {
        // I've published an update to allow this; take a look at 0.7.4
        // The buffers are still necessary because a DerObject is a simple view
        let mut algorithm_buf = Vec::new();
        let algorithm = self.algorithm.der_object(VecBacking(&mut algorithm_buf))?;

        let mut parameters_buf = Vec::new();
        let parameters = self.parameters.der_object(VecBacking(&mut parameters_buf))?;
        
        // Create a sequence
        Sequence::write(&[algorithm, parameters], sink)
    }
}

I agree that this is not very straight forward/elegant; the API was mainly designed with serde_asn1_der in mind. However if you're interested I can also revive asn1_der_derive, if serde is no option for you?

// TODO: Is the decode after the encode step needed? Passing a `Vec<u8>` to
// `Sequence::write` would not encode the payload with the right tag.
let algorithm = DerObject::decode(&algorithm)?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the way to go?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2000 (comment) :)


let mut parameters = Vec::new();
self.parameters.encode(&mut parameters)?;
let parameters = DerObject::decode(&parameters)?;

asn1_der::typed::Sequence::write(&[algorithm, parameters], sink)
}
}

impl DerDecodable<'_> for Asn1RsaEncryption {
fn load(object: DerObject<'_>) -> Result<Self, Asn1DerError> {
let seq: Sequence = Sequence::load(object)?;
let r = Ok(Asn1RsaEncryption{
algorithm: Asn1OidRsaEncryption::load(seq.get(0)?)?,
parameters: <()>::load(seq.get(1)?)?,
});
r
}
}

/// The ASN.1 SubjectPublicKey inside a SubjectPublicKeyInfo,
/// i.e. encoded as a DER BIT STRING.
struct Asn1SubjectPublicKey(PublicKey);

impl IntoDerObject for Asn1SubjectPublicKey {
fn into_der_object(self) -> DerObject {
let pk_der = (self.0).0;
impl asn1_der::typed::DerEncodable for Asn1SubjectPublicKey {
fn encode<S: asn1_der::Sink>(&self, sink: &mut S) -> Result<(), asn1_der::Asn1DerError> {
let pk_der = &(self.0).0;
let mut bit_string = Vec::with_capacity(pk_der.len() + 1);
// The number of bits in pk_der is trivially always a multiple of 8,
// so there are always 0 "unused bits" signaled by the first byte.
bit_string.push(0u8);
bit_string.extend(pk_der);
DerObject::new(DerTag::x03, bit_string.into())
}
fn serialized_len(&self) -> usize {
DerObject::compute_serialized_len((self.0).0.len() + 1)
asn1_der::DerObject::write(3, bit_string.len(), &mut bit_string.iter(), sink)?;
Ok(())
}
}

impl FromDerObject for Asn1SubjectPublicKey {
fn from_der_object(o: DerObject) -> Result<Self, Asn1DerError> {
if o.tag != DerTag::x03 {
return Err(Asn1DerError::InvalidTag)
impl DerDecodable<'_> for Asn1SubjectPublicKey {
fn load(object: DerObject<'_>) -> Result<Self, Asn1DerError> {
if object.tag() != 3 {
return Err(Asn1DerError::new(
Asn1DerErrorVariant::InvalidData("DER object tag is not the bit string tag."),
));
}
let pk_der: Vec<u8> = o.value.data.into_iter().skip(1).collect();

let pk_der: Vec<u8> = object.value().into_iter().skip(1).cloned().collect();
// We don't parse pk_der further as an ASN.1 RsaPublicKey, since
// we only need the DER encoding for `verify`.
Ok(Asn1SubjectPublicKey(PublicKey(pk_der)))
}
}

/// ASN.1 SubjectPublicKeyInfo
#[derive(Asn1Der)]
#[allow(non_snake_case)]
struct Asn1SubjectPublicKeyInfo {
algorithmIdentifier: Asn1RsaEncryption,
subjectPublicKey: Asn1SubjectPublicKey
}

impl asn1_der::typed::DerEncodable for Asn1SubjectPublicKeyInfo {
fn encode<S: asn1_der::Sink>(&self, sink: &mut S) -> Result<(), asn1_der::Asn1DerError> {
let mut identifier = Vec::new();
self.algorithmIdentifier.encode(&mut identifier)?;
let identifier = DerObject::decode(&identifier)?;

let mut key = Vec::new();
self.subjectPublicKey.encode(&mut key)?;
let key = DerObject::decode(&key)?;

asn1_der::typed::Sequence::write(&[identifier, key], sink)
}
}

impl DerDecodable<'_> for Asn1SubjectPublicKeyInfo {
fn load(object: DerObject<'_>) -> Result<Self, Asn1DerError> {
let seq: Sequence = Sequence::load(object)?;
Ok(Asn1SubjectPublicKeyInfo {
algorithmIdentifier: seq.get_as(0)?,
subjectPublicKey: Asn1SubjectPublicKey::load(seq.get(1)?)?,
})
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -221,8 +276,9 @@ mod tests {
const KEY2: &'static [u8] = include_bytes!("test/rsa-3072.pk8");
const KEY3: &'static [u8] = include_bytes!("test/rsa-4096.pk8");

// TODO: Remove libp2p_core_v026. For compatibility testing only.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will remove this before merging. Leaving it for now to ensure I don't introduce a regression.

#[derive(Clone)]
struct SomeKeypair(Keypair);
struct SomeKeypair(Keypair, libp2p_core_v026::identity::rsa::Keypair);

impl fmt::Debug for SomeKeypair {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -233,7 +289,11 @@ mod tests {
impl Arbitrary for SomeKeypair {
fn arbitrary<G: Gen>(g: &mut G) -> SomeKeypair {
let mut key = [KEY1, KEY2, KEY3].choose(g).unwrap().to_vec();
SomeKeypair(Keypair::from_pkcs8(&mut key).unwrap())
let mut key2 = key.clone();
SomeKeypair(
Keypair::from_pkcs8(&mut key).unwrap(),
libp2p_core_v026::identity::rsa::Keypair::from_pkcs8(&mut key2).unwrap(),
)
}
}

Expand All @@ -246,9 +306,16 @@ mod tests {

#[test]
fn rsa_x509_encode_decode() {
fn prop(SomeKeypair(kp): SomeKeypair) -> Result<bool, String> {
fn prop(SomeKeypair(kp, old_kp): SomeKeypair) -> Result<bool, String> {
let pk = kp.public();
PublicKey::decode_x509(&pk.encode_x509())
let old_kp = old_kp.public();

let x509 = pk.encode_x509();
let x509_old = old_kp.encode_x509();

assert_eq!(x509, x509_old);

PublicKey::decode_x509(&x509)
.map_err(|e| e.to_string())
.map(|pk2| pk2 == pk)
}
Expand All @@ -257,10 +324,9 @@ mod tests {

#[test]
fn rsa_sign_verify() {
fn prop(SomeKeypair(kp): SomeKeypair, msg: Vec<u8>) -> Result<bool, SigningError> {
fn prop(SomeKeypair(kp, _): SomeKeypair, msg: Vec<u8>) -> Result<bool, SigningError> {
kp.sign(&msg).map(|s| kp.public().verify(&msg, &s))
}
QuickCheck::new().tests(10).quickcheck(prop as fn(_,_) -> _);
}
}

14 changes: 7 additions & 7 deletions core/src/identity/secp256k1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

//! Secp256k1 keys.

use asn1_der::{FromDerObject, DerObject};
use asn1_der::typed::{DerDecodable, Sequence};
use rand::RngCore;
use sha2::{Digest as ShaDigestTrait, Sha256};
use secp256k1::{Message, Signature};
Expand Down Expand Up @@ -110,21 +110,21 @@ impl SecretKey {
}

/// Decode a DER-encoded Secp256k1 secret key in an ECPrivateKey
/// structure as defined in [RFC5915].
/// structure as defined in [RFC5915], zeroing the input slice on success.
///
/// [RFC5915]: https://tools.ietf.org/html/rfc5915
pub fn from_der(mut der: impl AsMut<[u8]>) -> Result<SecretKey, DecodingError> {
// TODO: Stricter parsing.
let der_obj = der.as_mut();
let obj: Vec<DerObject> = FromDerObject::deserialize((&*der_obj).iter())
let obj: Sequence = DerDecodable::decode(der_obj)
.map_err(|e| DecodingError::new("Secp256k1 DER ECPrivateKey").source(e))?;
der_obj.zeroize();
let sk_obj = obj.into_iter().nth(1)
.ok_or_else(|| DecodingError::new("Not enough elements in DER"))?;
let mut sk_bytes: Vec<u8> = FromDerObject::from_der_object(sk_obj)
let sk_obj = obj.get(1)
.map_err(|e| DecodingError::new("Not enough elements in DER").source(e))?;
let mut sk_bytes: Vec<u8> = asn1_der::typed::DerDecodable::load(sk_obj)
.map_err(DecodingError::new)?;
let sk = SecretKey::from_bytes(&mut sk_bytes)?;
sk_bytes.zeroize();
der_obj.zeroize();
Ok(sk)
}

Expand Down