zmap / zlint

X.509 Certificate Linter focused on Web PKI standards and requirements.
https://zmap.io
Apache License 2.0
353 stars 107 forks source link

Certs with CN containing capital letters flagged by `e_subject_common_name_not_from_san` false positive #186

Open szank opened 6 years ago

szank commented 6 years ago

Example: https://crt.sh/?id=276876975&opt=cablint,x509lint,zlint

No other linters in crt.sh are flagging this cert, and as far as I can tell, DNS names should be normalised before parsing/validation. Either way pasting Tours.AlliedTT.com to chrome works, as the dns name is automatically converted to lowercase (possibly by the IDNA engine)

The e_subject_common_name_not_from_san should perform ToLower on the common name before comparing it with SAN DNS names IMHO. Any objections?

titanous commented 6 years ago

Sounds good to me.

zakird commented 6 years ago

I agree that it shouldn't be an error. There isn't security vulnerability here.

That said, it is somewhat interesting to note that they have different capitalization. Might be worth having an info that they're not quite the same.

zakird commented 6 years ago

Now that we've seen that this would be considerably more complex (and partially undefined) when @szank went to implement this in #188, I think that we should close this issue, thus requiring capitalization to be consistent between the fields.

These two comments in particular make it more clear to me that "here be dragons", and because of that, there could be security problems associated having different capitalizations that we're not immediately seeing:

Common name (or SAN, if it have not been converted to punycode) might contain UTF-8 characters. Let's assume that common name is say, \u212A.com where \u212A is Kelvin symbol K) and the SAN DNS name is \u006B.com where \u006B is ASCII k. For go case insensitive comparison the strings are identical. Now, \u212A.com and \u006B.com are completely different domain names, but the linter would return PASS if the SAN DNS list contained only the first name and the CommonName contained the other.

SAN.dNSName is defined as an ASN.1 IA5String, so it can't hold a U-label. Whether or not the Subject.CN is permitted to hold a U-label that corresponds to a SAN.dNSName's A-label has been a topic of debate. The BRs say the CN should hold a "Fully‐Qualified Domain Name that is one of the values contained in the Certificate’s subjectAltName extension". But are a U-label and the corresponding A-label different "values"? Or are they different encodings of the same value? CABForum Ballot 202 from @pzb sought to clarify this issue (amongst other things) by explicitly requiring CNs to contain A-labels, but unfortunately the ballot narrowly failed (because certain CAs want to put U-labels in CNs). So the ambiguity persists.

I think that we should take the more cautious/conservative route given that we are a linter, and require the capitalization be consistent between the CN and SAN. This is the sane thing to do as an implementer anyway. Thoughts? If I don't hear too loud of objections the next few days, I will close this issue and PR #188.

pzb commented 6 years ago

https://tools.ietf.org/html/rfc5891#section-3.1 explains the rules for comparing domain names. Whether the match is case sensitive or not depends on whether you are looking at U-labels or A-labels.

szank commented 6 years ago

Hi @zakird, @pzb . Thanks for your input. I am happy with closing my PR, it is quite incomplete and if someone wants to tackle this issue, it would be better to start from a clean slate.

I would like to raise another related issue: What does the zlint team thing about validating the CommonName according to the IDNA rules for the public trust SSL certs?

The linter would implement the Registration Protocol from https://tools.ietf.org/html/rfc5891#section-4 I am not entirely sure that Registration Protocol is the better choice over the Domain Name Lookup Protocol (https://tools.ietf.org/html/rfc5891#section-5), but I believe that other people on this thread know much more than me about the issue and would be able to make the correct call.

This could be moved to a separate issue if there is an interest.

In general the DV checks should catch invalid domain names in CN, but if it was the case, but zlint is here to catch issues not caught elsewhere ;).

zakird commented 6 years ago

Can you move this into a new issue? Happy to have conversation there. On Mon, May 21, 2018 at 3:04 AM Maciej Galkowski notifications@github.com wrote:

Hi @zakird https://github.com/zakird, @pzb https://github.com/pzb . Thanks for your input. I am happy with closing my PR, it is quite incomplete and if someone wants to tackle this issue, it would be better to start from a clean slate.

I would like to raise another related issue: What does the zlint team thing about validating the CommonName according to the IDNA rules for the public trust SSL certs?

The linter would implement the Registration Protocol from https://tools.ietf.org/html/rfc5891#section-4 I am not entirely sure that Registration Protocol is the better choice over the Domain Name Lookup Protocol ( https://tools.ietf.org/html/rfc5891#section-5), but I believe that other people on this thread know much more than me about the issue and would be able to make the correct call.

This could be moved to a separate issue if there is an interest.

In general the DV checks should catch invalid domain names in CN, but if it was the case, but zlint is here to catch issues not caught elsewhere ;).

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/zmap/zlint/issues/186#issuecomment-390610966, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMSUFpP2dxWCUj65G3pjrPTPSUzscz2ks5t0pETgaJpZM4RAjtX .

cardonator commented 5 years ago

Regarding the issue mentioned here, doesn't (https://tools.ietf.org/html/rfc8399#section-2.4) override any previous RFC regarding subject fields and DN/DNS names in a cert? I think it does, and it reads:

Domain names may also be represented as distinguished names using
   domain components in the subject field, the issuer field, the
   subjectAltName extension, or the issuerAltName extension.  As with
   the dNSName in the GeneralName type, the value of this attribute is
   defined as an IA5String.  Each domainComponent attribute represents a
   single label.  To represent a label from an IDN in the distinguished
   name, the implementation MUST convert all U-labels to A-labels.

To me, this interprets as "ALL dNSNames in a cert must be an A-label". Given that, we probably shouldn't assume that having any sort of internationalized characters in the CN field is a blocker for resolving this issue.