Skip to content

Conversation

hablutzel1
Copy link
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
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
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
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
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
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 questions@cabforum.org (from the CA/B F email lists page).

@cpu
Copy link
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 <noreply@github.com>

* 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 <zakird@gmail.com>

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

Co-authored-by: GitHub <noreply@github.com>
Co-authored-by: Daniel McCarney <daniel@binaryparadox.net>
Co-authored-by: Zakir Durumeric <zakird@gmail.com>

Co-authored-by: Benjamin Wireman <ben@censys.io>
Co-authored-by: GitHub <noreply@github.com>
Co-authored-by: Daniel McCarney <daniel@binaryparadox.net>
Co-authored-by: Zakir Durumeric <zakird@gmail.com>
Co-authored-by: Jeff Cody <jeff@codyprime.org>
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