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.

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

WIP: add test for mixed DER/BER encoding CMS #827

Closed
wants to merge 2 commits into from

Conversation

smndtrl
Copy link
Contributor

@smndtrl smndtrl commented Dec 30, 2022

As requested in #813 a test for mixed encoding CMS.

@tarcieri I didn't find my original BER file from the Apple MDM Signature so I just checked the example at https://lapo.it/asn1js/# and it's very similar in that it starts with an indefinite length tag, uses multiple along the way and also uses constructed undefined length OctetStrings.

Both the DER/BER file have the same e_content. Currently the DER encoded file is used in the test and works fine.

Comment on lines +146 to +165
let path = "./tests/examples/cms_der.bin";
let bytes = fs::read(&path).expect(&format!("Failed to read from {}", &path));

let content = ContentInfo::from_der(&bytes).expect("expected valid data");

match content {
ContentInfo::SignedData(Some(data)) => {
assert_eq!(
data.encap_content_info
.e_content
.unwrap()
.decode_into::<OctetStringRef>()
.unwrap()
.as_bytes()
.len(),
10034
)
}
_ => panic!("expected ContentInfo::SignedData(Some(_))"),
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let path = "./tests/examples/cms_der.bin";
let bytes = fs::read(&path).expect(&format!("Failed to read from {}", &path));
let content = ContentInfo::from_der(&bytes).expect("expected valid data");
match content {
ContentInfo::SignedData(Some(data)) => {
assert_eq!(
data.encap_content_info
.e_content
.unwrap()
.decode_into::<OctetStringRef>()
.unwrap()
.as_bytes()
.len(),
10034
)
}
_ => panic!("expected ContentInfo::SignedData(Some(_))"),
}
let bytes = include_bytes!("examples/cms_der.bin");
let content = match ContentInfo::from_der(bytes) {
Ok(ContentInfo::SignedData(Some(data))) => data,
other => panic!("unexpected result: {:?}", other),
};
assert_eq!(
content
.encap_content_info
.e_content
.unwrap()
.decode_into::<OctetStringRef>()
.unwrap()
.as_bytes()
.len(),
10034
);

@tarcieri
Copy link
Member

tarcieri commented Jan 2, 2023

Looking at the BER example, it looks like it's using indefinite lengths fairly pervasively for several (nested) fields.

Making this work will probably involve modifying custom derive to be able to generate Decode impls for BER-tolerant SEQUENCEs.

tarcieri added a commit that referenced this pull request Jan 2, 2023
Per #823 and #827, making PKCS#7 work interoperably will involve
supporting at limited number of BER productions, one of which is
indefinite lengths.

The current built-in `Length` type rejects them, as a proper DER parser
is expected to.

This commit adds a separate `IndefiniteLength` type as a newtype of
`Option<Length>` with support for parsing indefinite lengths.
@tarcieri
Copy link
Member

tarcieri commented Jan 2, 2023

Here's a stepping stone towards a solution: #830

tarcieri added a commit that referenced this pull request Jan 2, 2023
Per #823 and #827, making PKCS#7 work interoperably will involve
supporting at limited number of BER productions, one of which is
indefinite lengths.

The current built-in `Length` type rejects them, as a proper DER parser
is expected to.

This commit adds a separate `IndefiniteLength` type as a newtype of
`Option<Length>` with support for parsing indefinite lengths.
tarcieri added a commit that referenced this pull request Jan 2, 2023
Per #823 and #827, making PKCS#7 work interoperably will involve
supporting at limited number of BER productions, one of which is
indefinite lengths.

The current built-in `Length` type rejects them, as a proper DER parser
is expected to.

This commit adds a separate `IndefiniteLength` type as a newtype of
`Option<Length>` with support for parsing indefinite lengths.
@tarcieri
Copy link
Member

tarcieri commented Jan 9, 2023

Merged in #840

@tarcieri tarcieri closed this Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants