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

microsoft seems to use a different EncapsulatedContentInfo struct #61

Closed
dvc94ch opened this issue Jan 22, 2022 · 5 comments
Closed

microsoft seems to use a different EncapsulatedContentInfo struct #61

dvc94ch opened this issue Jan 22, 2022 · 5 comments

Comments

@dvc94ch
Copy link
Contributor

dvc94ch commented Jan 22, 2022

microsoft msix and appx packages contain a CodeIntegrity.cat and AppxSignature.p7x file, which while undocumented seems to be cms encoded SignedData. however it seems like it's not quite spec compliant and maybe has an extra field or something.

thread 'test_msix_p7x' panicked at 'called `Result::unwrap()` on an `Err` value: FieldError { name: "SignedData.encap_content_info", error: "Field `EncapsulatedContentInfo.content`: Unexpected extra data found: length `264` bytes" }', standards/cms/tests/test_cms.rs:44:62
thread 'test_msix_cat' panicked at 'called `Result::unwrap()` on an `Err` value: FieldError { name: "SignedData.encap_content_info", error: "Field `EncapsulatedContentInfo.content`: Unexpected extra data found: length `1303` bytes" }', standards/cms/tests/test_cms.rs:58:62

replacing content with Any works.

@dvc94ch
Copy link
Contributor Author

dvc94ch commented Jan 22, 2022

This seems to fix the issue without breaking the existing tests. @XAMPPRocky is this a sensible change?

diff --git a/standards/cms/src/lib.rs b/standards/cms/src/lib.rs
index 55d006f..14bddab 100644
--- a/standards/cms/src/lib.rs
+++ b/standards/cms/src/lib.rs
@@ -208,7 +208,7 @@ pub struct OtherRevocationInfoFormat {
 #[derive(AsnType, Clone, Debug, Decode, Encode, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct EncapsulatedContentInfo {
     pub content_type: ContentType,
-    #[rasn(tag(explicit(0)))]
+    #[rasn(tag(context, 0))]
     pub content: Option<OctetString>,
 }

@XAMPPRocky
Copy link
Collaborator

Thank you for your issue! Right now I don't think I would want to make the change, since the IETF RFC is what should be followed, rather than Microsoft's undocumented formats. I would prefer to add a seperate type for Microsoft types over having spec non-compliant types.

@dvc94ch
Copy link
Contributor Author

dvc94ch commented Jan 23, 2022

from RFC5652 it is acknowledged that cms breaks compatibility with pkcs7, which is also an IETF RFC.

5.2.1.  Compatibility with PKCS #7

   This section contains a word of warning to implementers that wish to
   support both the CMS and PKCS #7 [PKCS#7] SignedData content types.
   Both the CMS and PKCS #7 identify the type of the encapsulated
   content with an object identifier, but the ASN.1 type of the content
   itself is variable in PKCS #7 SignedData content type.

   PKCS #7 defines content as:

      content [0] EXPLICIT ANY DEFINED BY contentType OPTIONAL

   The CMS defines eContent as:

      eContent [0] EXPLICIT OCTET STRING OPTIONAL

   The CMS definition is much easier to use in most applications, and it
   is compatible with both S/MIME v2 and S/MIME v3.  S/MIME signed
   messages using the CMS and PKCS #7 are compatible because identical
   signed message formats are specified in RFC 2311 for S/MIME v2
   [MSG2], RFC 2633 for S/MIME v3 [MSG3], and RFC 3851 for S/MIME v3.1
   [MSG3.1].  S/MIME v2 encapsulates the MIME content in a Data type
   (that is, an OCTET STRING) carried in the SignedData contentInfo
   content ANY field, and S/MIME v3 carries the MIME content in the
   SignedData encapContentInfo eContent OCTET STRING.  Therefore, in
   S/MIME v2, S/MIME v3, and S/MIME v3.1, the MIME content is placed in
   an OCTET STRING and the message digest is computed over the identical
   portions of the content.  That is, the message digest is computed
   over the octets comprising the value of the OCTET STRING, neither the
   tag nor length octets are included.

https://datatracker.ietf.org/doc/html/rfc5652#page-12

@XAMPPRocky
Copy link
Collaborator

@dvc94ch Thanks for the info, I think I'd like to go with having a separate type for PKCS#7 EncapsulatedContentInfo, SignedData, DigestData, and AuthenticatedData stored in a pkcs7_compat module in cms.

@XAMPPRocky
Copy link
Collaborator

Closing as resolved in #62

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

No branches or pull requests

2 participants