RustCrypto / formats

Cryptography-related format encoders/decoders: DER, PEM, PKCS, PKIX
229 stars 122 forks source link

Change RKeyId enum variant to use constructed encoding #1333

Closed andrewmah closed 6 months ago

andrewmah commented 6 months ago

The type RecipientKeyIdentifier is a SEQUENCE, so it should use constructed encoding in the enum variant KeyAgreeRecipientIdentifier::RKeyId(RecipientKeyIdentifier)

I discovered this because of the error in this code snippet.

use cms::{
    cert::x509::ext::pkix::SubjectKeyIdentifier,
    enveloped_data::{KeyAgreeRecipientIdentifier, RecipientKeyIdentifier},
};
use der::{asn1::OctetString, Decode, Encode};

fn main() {
    let kari = KeyAgreeRecipientIdentifier::RKeyId(RecipientKeyIdentifier {
        subject_key_identifier: SubjectKeyIdentifier(OctetString::new([0]).unwrap()),
        date: None,
        other: None,
    });

    let encoded = kari.to_der().unwrap();
    let decoded = KeyAgreeRecipientIdentifier::from_der(&encoded).unwrap();
    // Error { kind: Noncanonical { tag: Tag(0x80: CONTEXT-SPECIFIC [0] (primitive)) }, position: None }

    assert_eq!(kari, decoded);
}
tarcieri commented 6 months ago

Aah, because it's an IMPLICIT field in a CHOICE, right? /cc @carl-wallace

carl-wallace commented 6 months ago

I agree the PR looks right. It should definitely be a constructed context specific tag because the underlying type is constructed.