Skip to content

CmsSignedData: Don't assume the signed content is a Asn1OctetString object - #63

Closed
qmfrederik wants to merge 2 commits into
bcgit:masterfrom
qmfrederik:fixes/CmsSignedData
Closed

CmsSignedData: Don't assume the signed content is a Asn1OctetString object#63
qmfrederik wants to merge 2 commits into
bcgit:masterfrom
qmfrederik:fixes/CmsSignedData

Conversation

@qmfrederik

Copy link
Copy Markdown

This PR fixes an issue in the CmsSignedData where the constructor would assume that the signed content is an Asn1OctetString object. That's not always true - in my case, the data was a DerSequence.

@peterdettmanpeterdettman self-assigned this Jun 23, 2016
@peterdettman

Copy link
Copy Markdown
Collaborator

The proposed change would change (break) the behaviour when the signed content is an Asn1OctetString, since GetOctets is not the same as GetEncoded.

The BC Java version has extra code to allow a sequence to be handled here, which would be reasonably straightforward to port over, but it is not quite as simple as calling GetEncoded on the sequence (in fact it appears to just encode the elements of the sequence). So I doubt if the proposed change is correct for sequences either.

It would be helpful if you could give more information on what type of files you are trying to work with, and if you know it, the relevant applicable RFC or other standards document that we could refer to.

@qmfrederik

Copy link
Copy Markdown
Author

Thanks for the feedback!

The files I'm working with are security catalogs for drivers (.cat files). Amongst others, Windows drivers ship with .cat files which contain the digital signature for the driver. An example of such a .cat file is androidwinusb86.zip.

As far as I understand, RFC 2315 section 5 does not place any restriction on the content:

The use of content types defined outside this document is possible, but is subject to bilateral agreement between parties exchanging content.

(The Java branch seems to refer to this content as "PKCS#7 ANY").

And hence, Microsoft defined the Certificate Trust List with OID 1.3.6.1.4.1.311.10.1, which is the content type for this file.

I've amended the PR to be backward compatibility. Let me know if you require additional information from my side!

@nmoinvaz

Copy link
Copy Markdown

I think RFC 5652 section 5.2.1, Compatibility with PKCS #7, applies here as well:

PKCS #7 defines content as:

content [0] EXPLICIT ANY DEFINED BY contentType OPTIONAL

The CMS defines eContent as:

eContent [0] EXPLICIT OCTET STRING OPTIONAL

@Genbox

Copy link
Copy Markdown

Authenticode signed files are PKCS#7 and not CMS, which will result in BC breaking inside CmsSignedDataParser. BC Java Beta (156b03) have implemented PKCS#7 support, which should solve this.

@peterdettman

Copy link
Copy Markdown
Collaborator

I think this was addressed by 2a508f3 .

Sign up for freeto 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.

4 participants

@qmfrederik@peterdettman@nmoinvaz@Genbox