Skip to content

Fix CMSSignerInfo's unsignedAttrs tag - #307

Open
emaloney wants to merge 1 commit into
apple:mainfrom
emaloney:fix-cmssignerinfo-unsignedattrs-tag
Open

Fix CMSSignerInfo's unsignedAttrs tag#307
emaloney wants to merge 1 commit into
apple:mainfrom
emaloney:fix-cmssignerinfo-unsignedattrs-tag

Conversation

@emaloney

Copy link
Copy Markdown

Motivation

CMSSignerInfo could not round-trip unsignedAttrs: it serializes them as [1] IMPLICIT SET but the DER and BER parsers asked for the inner set with implicit tag [0], so parsing any SignerInfo with unsigned attributes threw unexpectedFieldType: [1].

Changes

Fixes issue #306 by:

  • Correcting the unsignedAttrs inner set identifier from [0] to [1] in the DER and BER initializers of CMSSignerInfo.
  • Adding a round-trip unit test for a CMSSignerInfo carrying unsignedAttrs.

Result

unsignedAttrs now round-trip correctly; no behavior change to the (already correct) serialized output.

@ericb-apple

Copy link
Copy Markdown

Neat - I have the exact fix on my vendored copy I was eventually going to submit

@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Jul 20, 2026

@Lukasa Lukasa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice, thank you @emaloney!

@Lukasa

Lukasa commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@emaloney Can you apply the license headers to the test file please?

@emaloney
emaloney force-pushed the fix-cmssignerinfo-unsignedattrs-tag branch from 1daa902 to 8dbf97b Compare July 28, 2026 13:11
In order to make `pkg-signer*` work, we needed to make some changes to `swift-certificates` exposing certain entities publicly and adding trusted timestamp support to CMS.
@emaloney
emaloney force-pushed the fix-cmssignerinfo-unsignedattrs-tag branch from 8dbf97b to 24a5816 Compare July 28, 2026 13:11
@emaloney

Copy link
Copy Markdown
Author

@Lukasa pushed an update adding the requested header to the test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants