Skip to content

Policy qualifiers shouldn't be present if the subordinate CA is an affiliate of the entity that controls the Root CA#289

Closed
hablutzel1 wants to merge 2 commits intozmap:masterfrom
hablutzel1:add_lint_to_notice_when_policy_qualifiers_exist_in_affiliate_subcas
Closed

Policy qualifiers shouldn't be present if the subordinate CA is an affiliate of the entity that controls the Root CA#289
hablutzel1 wants to merge 2 commits intozmap:masterfrom
hablutzel1:add_lint_to_notice_when_policy_qualifiers_exist_in_affiliate_subcas

Conversation

@hablutzel1
Copy link
Copy Markdown
Contributor

From BRs, "7.1.2.2. Subordinate CA Certificate":

The following fields MAY be present if the Subordinate CA is not an Affiliate of the entity that controls the Root CA.
certificatePolicies:policyQualifiers:policyQualifierId (Optional)
...
certificatePolicies:policyQualifiers:qualifier:cPSuri (Optional)
...

It is indicating that policy qualifiers (e.g. CPS Pointer) shouldn't be present at all if the subordinate CA is affiliated to the root CA, isn't it?. In that case, this PR includes a new linter that would emit a notice when there are any policy qualifiers in subordinate CAs to help the subordinate CA signers to double check if they should be really including these policy qualifiers.

Finally, if this lint is OK to be pulled, let me know to write its corresponding test.

…filiate of the entity that controls the Root CA
@hablutzel1 hablutzel1 changed the title Policy qualifiers shouldn't be present if the subordinate CA is an Affiliate of the entity that controls the Root CA Policy qualifiers shouldn't be present if the subordinate CA is an affiliate of the entity that controls the Root CA Jun 25, 2019
@cpu
Copy link
Copy Markdown
Member

cpu commented Jun 27, 2019

👋 Hi @hablutzel1,

It is indicating that policy qualifiers (e.g. CPS Pointer) shouldn't be present at all if the subordinate CA is affiliated to the root CA, isn't it?

I think that's a reasonable reading but I don't know that the notice level lint you're proposing is going to be helpful overall. I suspect since you can't determine if any given sub-CA is "an Affiliate of the entity that controls the Root CA" programmatically the lint will necessarily create false-positive notices.

Does anyone else have an opinion?

@cardonator
Copy link
Copy Markdown
Contributor

cardonator commented Jun 27, 2019

I agree that the interpretation is reasonable but the wording in the BR doesn't specify whether it is "MUST NOT" vs. "SHOULD NOT". Since it's not explicitly stated, it's not clear what the correct behavior should be. Notice level would be appropriate for this type of informational only lint.

Also agree that any lint would be very likely to produce false positives.

@hablutzel1
Copy link
Copy Markdown
Contributor Author

It is clear that it will provide false positives for the subordinate CAs under a third party root (which I think that are not the most), but given that this is just a notice, isn't that acceptable?.

I think that's a reasonable reading but I don't know that the notice level lint you're proposing is going to be helpful overall.

It would help to remind root CAs signing their own subordinate CAs to not include policy qualifiers, but it seems that several CAs are doing it anyway. Some random examples:

Notice level would be appropriate for this type of informational only lint.

This is actually a Pass or Notice lint.

Regardless of the previous, and even more important, what is the reason for this requirement in the BRs? , do you know?.

@zakird
Copy link
Copy Markdown
Member

zakird commented Jun 28, 2019

I agree with @cpu here that I don't think that this is going to be helpful overall. Even as a notice, it's going to cause a fair amount of noise. There's just no way for us to know whether the subCA is affiliated. In general, I'd rather be conservative about adding noise since it will cause CAs to just ignore the notices due to warning fatigue.

@cpu
Copy link
Copy Markdown
Member

cpu commented Jun 28, 2019

what is the reason for this requirement in the BRs? , do you know?.

@hablutzel1 I don't know, sorry. Perhaps you could submit a "question from the public" to [email protected] (from the CA/B F email lists page).

@cpu
Copy link
Copy Markdown
Member

cpu commented Jul 11, 2019

It seems like between @zakird, @cardonator and myself the consensus is that this won't be a net win for zlint. I'm going to close the PR for now to keep the queue tidy. If dissenting opinions come forth we can always re-open it.

@cpu cpu closed this Jul 11, 2019
aaomidi pushed a commit to aaomidi/zlint that referenced this pull request Nov 29, 2022
…ng (zmap#298)

* backporting asn1, pkix to allow permissive parsing (zmap#284)

* forking from golang.org/x/crypto/cryptobyte to allow permissive parsing

* Allow permissive asn1 parsing on UTF8, integer and NameConstraints (zmap#287)

* Allow permissive asn1 parsing on UTF8, integer and NameConstraints

* Allow permissive asn1 parsing on UTF8, integer and NameConstraints, NumericString

* Allow permissive parsing: IA5, integer min len (zmap#289)

* Fix Name.String() to legacy behavior, permissive parsing asn1.IA5String (zmap#292)

* deps: update publicsuffix-go for 2021-05-11T10:35:34 UTC

* deps: update publicsuffix-go for 2021-05-13T08:40:11 UTC

* deps: update publicsuffix-go for 2021-05-21T08:41:21 UTC

* deps: update publicsuffix-go for 2021-05-27T09:03:28 UTC

* Allow permissive parsing: IA5, integer min len

* Fix Name.String() to legacy behavior

Co-authored-by: GitHub <[email protected]>

* Fix RDNSequence.String() to print user friendly names (zmap#294)

* Merge branch master into feature/parse_certs (zmap#296)

* deps: update publicsuffix-go for 2021-05-11T10:35:34 UTC

* deps: update publicsuffix-go for 2021-05-13T08:40:11 UTC

* deps: update publicsuffix-go for 2021-05-21T08:41:21 UTC

* deps: update publicsuffix-go for 2021-05-27T09:03:28 UTC

* deps: update publicsuffix-go for 2021-06-01T15:03:13 UTC

* Fix RDNSequence.String() to print user friendly names

* Porting ocsp package from the latest standard lib (zmap#279)

Co-authored-by: Zakir Durumeric <[email protected]>

* deps: update publicsuffix-go for 2021-06-07T12:03:41 UTC

Co-authored-by: GitHub <[email protected]>
Co-authored-by: Daniel McCarney <[email protected]>
Co-authored-by: Zakir Durumeric <[email protected]>

Co-authored-by: Benjamin Wireman <[email protected]>
Co-authored-by: GitHub <[email protected]>
Co-authored-by: Daniel McCarney <[email protected]>
Co-authored-by: Zakir Durumeric <[email protected]>
Co-authored-by: Jeff Cody <[email protected]>
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.

4 participants