Skip to content

Update CMP ASN.1 structures#100

Open
Guiliano99 wants to merge 12 commits intorusshousley:masterfrom
Guiliano99:master
Open

Update CMP ASN.1 structures#100
Guiliano99 wants to merge 12 commits intorusshousley:masterfrom
Guiliano99:master

Conversation

@Guiliano99
Copy link

Fix some Certificate Management Protocol (CMP) related structures.

Changes

  • I updated the ASN.1 Python modules for RFC 9480 and RFC 9810 to align with the standards.
  • I also updated the references to use the correct structures.
  • Removed duplicated structure.

Note

  • If you have a some time, could you please review my branch and check the file rfc9883_example_data.py?
    I'm trying to understand why the signature verification is failing.

tag.tagFormatConstructed, 1)))
namedtype.NamedType(
"dpn",
rfc5280.DistributionPointName().subtype(implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this change aligns with the ASN.1 module in the RFC. The ASN.1 module uses EXPLICIT tagging by default. CRLSource is defined as:

CRLSource ::= CHOICE {
   dpn          [0] DistributionPointName,
   issuer       [1] GeneralNames }

Which means each of the arms of the CHOICE are explicitly tagged.

Given this, I believe the current definition is correct.

Copy link
Author

@Guiliano99 Guiliano99 Jan 31, 2026

Choose a reason for hiding this comment

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

But the older definition said EncryptedKey, inside the code.

Copy link
Author

Choose a reason for hiding this comment

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

But i will fix the tagging.

rfc5280.DistributionPointName().subtype(implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0)),
),
namedtype.NamedType(
"issuer", rfc5280.GeneralNames().subtype(implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Likewise here.

namedtype.NamedType('issuer', EncryptedKey().subtype(
explicitTag=tag.Tag(tag.tagClassContext,
tag.tagFormatConstructed, 1)))
namedtype.NamedType(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The updated ASN.1 module in RFC 9810 also uses EXPLICIT tagging by default, so the comments in rfc9480.py apply here as well.

Copy link
Author

Choose a reason for hiding this comment

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

Like above

"dpn",
rfc5280.DistributionPointName().subtype(implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0)),
),
namedtype.NamedType(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

@Guiliano99
Copy link
Author

Guiliano99 commented Jan 31, 2026

@CBonnell thanks for your reply. The structures references are wrong. Please have a look at my comment from above.

@Guiliano99 Guiliano99 closed this Jan 31, 2026
@Guiliano99 Guiliano99 reopened this Jan 31, 2026
@Guiliano99 Guiliano99 requested a review from CBonnell January 31, 2026 06:42
@russhousley russhousley closed this Feb 4, 2026
@Guiliano99
Copy link
Author

@russhousley Hey, I have a quick question, what was the reason for closing this PR? Happy to revise or change it, if you point me to the issue.

namedtype.OptionalNamedType('errorDetails', PKIFreeText())
)

class CertResponse(univ.Sequence):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was the reference to rfc4210.CertResponse removed and the same definition re-defined here?

Copy link
Owner

Choose a reason for hiding this comment

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

CertResponse was not changed from RFC 4210, so it is imported from rfc4210.py. See line 217.

Copy link
Author

Choose a reason for hiding this comment

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

Because the CertifiedKeyPairstructure changed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Guiliano99, ah, thanks for pointing this out. I agree this re-definition is needed.

Copy link
Author

Choose a reason for hiding this comment

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

@CBonnell Should the structure also be updated inside the rfc4210.py file or is it correct to only change it in the newer RFC files?

Copy link
Owner

Choose a reason for hiding this comment

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

I see. I missed the change from EncryptedKey to EncryptedValue in the CertOrEncCert structure.

@CBonnell
Copy link
Collaborator

CBonnell commented Feb 6, 2026

After reviewing, I believe all proposed edits are not necessary except for the definition of CertProfileValue, as this PR correctly defines it as a SEQUENCE OF UTF8STRING.

@russhousley I think this PR should be re-opened to fix the CertProfileValue definition to align with the RFC(s). But all changes not related to CertProfileValue should be removed from this PR as they are not necessary.

@russhousley
Copy link
Owner

Agree, it should be corrected:

  CertProfileValue ::= SEQUENCE SIZE (1..MAX) OF UTF8String

@russhousley russhousley reopened this Feb 6, 2026
@Guiliano99
Copy link
Author

@russhousley and @russhousley The CRLSource says EncryptedKey inside the old code. So I corrected the structure to GeneralNames.

@Guiliano99
Copy link
Author

@russhousley Should the file rfc4210 than also be updated to reference the new CertifiedKeyPair structure?

@russhousley
Copy link
Owner

I suggest:

class CertProfileValue(univ.SequenceOf):
    componentType = char.UTF8String()
    subtypeSpec=constraint.ValueSizeConstraint(1, MAX)

@Guiliano99
Copy link
Author

@russhousley Should I remove the comment from the class? In older modules, you also included the definition inside the docstrings, should I also not do that as well?

@russhousley
Copy link
Owner

The CRLSource says EncryptedKey inside the old code. So I corrected the structure to GeneralNames.

Sorry, I missed this one. It needs to be changed too.

class CRLSource(univ.Choice):
    componentType = namedtype.NamedTypes(
        namedtype.NamedType('dpn', DistributionPointName().subtype(
            explicitTag=tag.Tag(tag.tagClassContext,
                tag.tagFormatConstructed, 0))),
        namedtype.NamedType('issuer', GeneralNames().subtype(
            explicitTag=tag.Tag(tag.tagClassContext,
                tag.tagFormatConstructed, 1)))
    )

@Guiliano99
Copy link
Author

@russhousley Should the CertifiedKeyPair structure inside the rfc4210.py file also be modified or only in the new RFCs?

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.

3 participants