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

CMS signer identifier is incorrectly encoded #203

Closed
Lukasa opened this issue Oct 15, 2024 · 0 comments · Fixed by #218
Closed

CMS signer identifier is incorrectly encoded #203

Lukasa opened this issue Oct 15, 2024 · 0 comments · Fixed by #218
Labels
good first issue Good for newcomers kind/enhancement Improvements to existing feature.

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Oct 15, 2024

We have the following code for serializing and deserializing the subjectKeyIdentifier option in CMSSignerIdentifier:

case .subjectKeyIdentifier(let subjectKeyIdentifier):
try coder.serialize(
ASN1OctetString(contentBytes: subjectKeyIdentifier.keyIdentifier),
explicitlyTaggedWithIdentifier: Self.skiIdentifier
)

self = try DER.explicitlyTagged(
node,
tagNumber: Self.skiIdentifier.tagNumber,
tagClass: Self.skiIdentifier.tagClass
) { node in
.subjectKeyIdentifier(.init(keyIdentifier: try ASN1OctetString(derEncoded: node).bytes))
}

Both of these perform explicit tagging of the field. That's informed by this declaration from RFC 5652 § 12.1:

SignerIdentifier ::= CHOICE {
  issuerAndSerialNumber IssuerAndSerialNumber,
  subjectKeyIdentifier [0] SubjectKeyIdentifier }

However, this misses that the tags default for the document is specified as:

   DEFINITIONS IMPLICIT TAGS ::=

So the default tag is implicit unless one of the exceptions from ITU 680 § 30.6 applies. None of them do, so this should be implicitly tagged.

@Lukasa Lukasa added kind/enhancement Improvements to existing feature. good first issue Good for newcomers labels Oct 15, 2024
@Lukasa Lukasa closed this as completed in 8210859 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers kind/enhancement Improvements to existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant