Closed Bug 1107790 Opened 6 years ago Closed 6 years ago

Remove support for absolute hostnames in presented DNS IDs and name constraints

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: briansmith, Assigned: briansmith)

References

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file)

I think we're being a little too liberal in supporting the absolute DNS ID syntax for presented IDs (hostnames in the CN of the subject, and DNSNames in SubjectAltName). RFC 5280 and related standards do not suggest any syntax that supports the trailing dot.

We still have to support absolute *reference DNS IDs*, but we should remove the support for the other forms.
Attachment #8532349 - Flags: review?(dkeeler)
Comment on attachment 8532349 [details] [diff] [review]
remove-absolute-DNS-ID-cert-support.patch

Review of attachment 8532349 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM - just two items to address.

::: security/pkix/lib/pkixnames.cpp
@@ +757,5 @@
>          case GeneralNameType::dNSName:
>            matches = PresentedDNSIDMatchesReferenceDNSID(
>                        presentedID, ValidDNSIDMatchType::NameConstraint, base);
> +          if (!matches &&
> +              !IsValidDNSID(base, ValidDNSIDMatchType::NameConstraint)) {

I'm not sure I understand why we have '!matches &&' here - is that an optimization? (Because if matches is true, then we already know base is a valid DNSID as a name constraint.) We should probably document that, if so.

@@ +1072,5 @@
>        if (!isFirstPresentedByte && StartsWithIDNALabel(referenceDNSID)) {
>          return false;
>        }
> +    }
> +    else {

nit: '} else {' all on one line, unless there's a compelling reason not to
Attachment #8532349 - Flags: review?(dkeeler) → review+
https://hg.mozilla.org/mozilla-central/rev/cfe200a463ab
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.