Skip to content

Fix IV countryName lint checkApplies, add IV personal name lint history#1027

Merged
christopher-henderson merged 2 commits into
zmap:masterfrom
mhyder13:iv-personal-name
Mar 22, 2026
Merged

Fix IV countryName lint checkApplies, add IV personal name lint history#1027
christopher-henderson merged 2 commits into
zmap:masterfrom
mhyder13:iv-personal-name

Conversation

@mhyder13

@mhyder13 mhyder13 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixed a bug in e_cert_policy_iv_requires_country where IV-issuing policy constrained CAs were inadvertently linted
  • Added unit tests to e_cert_policy_iv_requires_country to cover checkApplies() and effective date
  • Updated the citation string in e_cert_policy_iv_requires_country to the current location
  • Updated the policy language and added a citation history for e_cert_policy_iv_requires_country
  • Added a citation history for e_cab_iv_requires_personal_name
  • Minor cleanup of citation comment in e_cab_iv_requires_personal_name_strict

Explanation

e_cert_policy_iv_requires_country was written to enforce the country requirement of Subscriber Certificates issued with Individual Validation. While it's true that Subordinate CAs are also required to include the countryName in their subject, that requirement has always been separate from the requirement enforced and cited by this lint. In v1.3.1 (when IV was added) the requirement for IV was in §7.1.6.1 while the CA subject requirement was in §7.1.2.2h. In the current version v2.2.5, these are still separate as §7.1.2.7.3 and §7.1.2.10.2 respectively. Additionally, the other IV name lints already exclude CAs, which shows this was likely unintentional. Lastly, we already have an implementation of the CA requirement in e_ca_country_name_missing, so this lint is only adding an incorrect additional error when it triggers on CA certs.

It's not abnormal for Subordinate CA certs to assert the 2.23.140.1.2.3 IV policy. In the current version of the BRs, TLS Subordinate CAs are required to include the certificate policies extension, and Subordinates which are not operated by affiliates of the issuer must be "Policy Restricted." To be Policy Restricted, exactly one Reserved Certificate Policy, such as 2.23.140.1.2.3, must be included. The rules at the time of v1.3.1 did not explicitly require the use of a Reserved Certificate Policy and did not yet use the term "Policy Restricted," but it did require CAs issued to non-affiliates to include "one or more explicit policy identifiers that indicates the Subordinate CA’s adherence to and compliance with these Requirements" and allowed the use of "the CA/Browser Forum reserved identifiers" for this purpose. As such, this lint should never have applied to CAs because the appearance of this policy in a CA has a different meaning than in a Subscriber Certificate, and this has been true for the entire lifetime of this requirement. See v1.3.1 §7.1.6.3.1 and v2.2.5 §7.1.2.10.5.

v1.3.1

image

v2.2.5

image

Doc History

All three lints follow the same section history, but e_cab_iv_requires_personal_name is replaced with the strict variant in v2.0.0.

v1.3.1

image

v1.7.3 (edit shown from redline)

image image

v2.2.5 (link to section, screenshot only shows relevant table sections)

image image image

* Fixed a bug in e_cert_policy_iv_requires_country where IV-issuing policy constrained CAs were inadvertently linted
* Added unit tests to e_cert_policy_iv_requires_country to cover checkApplies() and effective date
* Updated the citation string in e_cert_policy_iv_requires_country to the current location
* Updated the policy language and added a citation history for e_cert_policy_iv_requires_country
* Added a citation history for e_cab_iv_requires_personal_name
* Minor cleanup of citation comment in e_cab_iv_requires_personal_name_strict

@christopher-henderson christopher-henderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for conducting what must be a slow and tedious process!

I've tried several times at automating something like this, but the context is too large for our fancy little robots to handle even halfway well (or perhaps I need to be more token efficient 🤷 ).

t.Errorf("%s: expected %s, got %s", inputPath, expected, out.Status)
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is perfectly fine, but just a heads up that this codebase leverages table driven testing quite extensively (plus the Golang ecosystem tends to prefer it as well).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Given that every test in this packages boils down to "run on this file and match this result" that makes sense. I was just following the style of the existing tests in this file, but I'll rewrite my next PR in that style if you prefer.

@christopher-henderson christopher-henderson enabled auto-merge (squash) March 22, 2026 20:48
@christopher-henderson christopher-henderson merged commit f804eca into zmap:master Mar 22, 2026
4 checks passed
@mhyder13 mhyder13 deleted the iv-personal-name branch March 22, 2026 23:12
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.

2 participants