Fix .onion tests to only apply to EV certificates #449

sleevi · 2020-06-07T20:00:24Z

Before this change, ZLint would reject .onion names in non-EV certs
via the lint_san_dns_name_onion_not_ev_cert lint, and if that
was suppressed, then complain about the missing Tor Service
Descriptor extension. As of CA/Browser Forum Ballot SC27, it's
allowed for v3 onion names to appear in DV/OV/IV certificates, and
the Tor Service Descriptor extension is neither required nor
prohibited for these.

This change corrects the Tor Service Descriptor tests to properly
account for it being mandatory for EV, while optional for DV/OV/IV.
This does not introduce new lints to ensure that the address is
itself a well-formed V2 (if EV) or V3 (all types) address, which
will come in a follow-up change.

Before this change, ZLint would reject .onion names in non-EV certs via the `lint_san_dns_name_onion_not_ev_cert` lint, and if that was suppressed, then complain about the missing Tor Service Descriptor extension. As of CA/Browser Forum Ballot SC27, it's allowed for v3 onion names to appear in DV/OV/IV certificates, and the Tor Service Descriptor extension is neither required nor prohibited for these. This change corrects the Tor Service Descriptor tests to properly account for it being mandatory for EV, while optional for DV/OV/IV. This does not introduce new lints to ensure that the address is itself a well-formed V2 (if EV) or V3 (all types) address, which will come in a follow-up change. Closes zmap#440

sleevi · 2020-06-07T20:01:44Z

Updated comment to reflect #440 still needs the new lint. I figured that would be more appropriate as a separate PR, but happy to fold it in here based on reviewer load. I thought this was already a big enough change to warrant separating as its own PR.

sleevi · 2020-06-07T20:13:58Z

v2/util/time.go

@@ -57,6 +57,7 @@ var (
 	MozillaPolicy22Date         = time.Date(2013, time.July, 26, 0, 0, 0, 0, time.UTC)
 	MozillaPolicy24Date         = time.Date(2017, time.February, 28, 0, 0, 0, 0, time.UTC)
 	MozillaPolicy27Date         = time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC)
+	CABFBRs_1_6_9_Date          = time.Date(2020, time.March, 27, 0, 0, 0, 0, time.UTC)


One thing I debated: What to name this. The naming here for CAB related things is a bit weird. Up until CABV201Date, it seems like the naming for CABV actually tied to the version of the document (in which case, this should be CABV169Date

Except 201 was a ballot number, not a version number, since the EV Guidelines and the BRs are separately versioned.

I opted here to be explicit about document-and-version, in line with the ETSI date, but I could also see an argument for a descriptive date, like OnionV3Date, so that folks don't have to constantly figure out what changed in BRs 1.6.9 vs 1.7.0.

Alternatively, naming the Ballots, e.g. as CABFSC27, at least makes it easier to quickly look up what the diff was (and across documents).

Naming is hard, but I wanted to draw attention to this to see where folks felt.

Except 201 was a ballot number, not a version number, since the EV Guidelines and the BRs are separately versioned.

I think this one was my fault 😓 In retrospect I agree it seems better to match to the document version number instead of referencing a ballot here.

I opted here to be explicit about document-and-version,

That feels right for this case to me too 👍

sleevi · 2020-06-07T20:16:32Z

v2/lints/cabf_br/lint_ext_tor_service_descriptor_hash_invalid.go

+	// TorServiceDescriptorHash, so check if any of the onion subjects in the
+	// certificate don't have a TorServiceDescriptorHash for the eTLD+1 in the
+	// descriptorMap.
+	// See also https://github.com/cabforum/documents/issues/190


We don't really have any normative guidance on what to do if this extension appears in a non-EV cert. My interpretation is that despite it being unnecessary for V3 certs, because it's optional, it should be legal to include this for some but not all v3 onion names.

Similarly, if/when we fix it for EV, I imagine that it'll be required for V2 names, but optional for V3 names. While prohibited-for-v3 makes the linting slightly easier, I suspect it makes CAs' lives harder, so I doubt that'll happen in the EVGs.

sleevi · 2020-06-07T20:19:54Z

v2/lints/cabf_br/lint_ext_tor_service_descriptor_hash_invalid_test.go

@@ -16,7 +16,7 @@ func TestTorDescHashInvalid(t *testing.T) {
 	}{
 		{
 			Name:           "Onion subject, no service descriptor extension, before util.CABV201Date",
-			InputFilename:  "dnsNameOnionTLD.pem",
+			InputFilename:  "onionSANEVBefore201.pem",
 			ExpectedResult: lint.NE,
 		},
 		{


Note: onionSANMissingServDescHash.pem is updated to be an EV certificate, which does mean that coverage for the optionality (for DV) certs is missed. I was torn on whether to add a DV cert (or, more aptly, restore the existing cert as a DV test and add a new cert as an EV test). I wasn't sure to what degree we wanted to cover this with testing.

I'm comfortable with the coverage you have in branch. Thanks for calling this out 👍

cpu · 2020-06-09T21:44:39Z

v2/lints/cabf_br/lint_ext_tor_service_descriptor_hash_invalid_test.go

@@ -16,7 +16,7 @@ func TestTorDescHashInvalid(t *testing.T) {
 	}{
 		{
 			Name:           "Onion subject, no service descriptor extension, before util.CABV201Date",
-			InputFilename:  "dnsNameOnionTLD.pem",
+			InputFilename:  "onionSANEVBefore201.pem",
 			ExpectedResult: lint.NE,
 		},
 		{


I'm comfortable with the coverage you have in branch. Thanks for calling this out 👍

cpu · 2020-06-09T21:46:50Z

v2/util/time.go

@@ -57,6 +57,7 @@ var (
 	MozillaPolicy22Date         = time.Date(2013, time.July, 26, 0, 0, 0, 0, time.UTC)
 	MozillaPolicy24Date         = time.Date(2017, time.February, 28, 0, 0, 0, 0, time.UTC)
 	MozillaPolicy27Date         = time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC)
+	CABFBRs_1_6_9_Date          = time.Date(2020, time.March, 27, 0, 0, 0, 0, time.UTC)


Except 201 was a ballot number, not a version number, since the EV Guidelines and the BRs are separately versioned.

I think this one was my fault 😓 In retrospect I agree it seems better to match to the document version number instead of referencing a ballot here.

I opted here to be explicit about document-and-version,

That feels right for this case to me too 👍

sleevi commented Jun 7, 2020

View reviewed changes

cpu self-assigned this Jun 8, 2020

cpu approved these changes Jun 9, 2020

View reviewed changes

zakird approved these changes Jun 10, 2020

View reviewed changes

zakird merged commit 84a8a20 into zmap:master Jun 10, 2020

sleevi mentioned this pull request Jan 4, 2021

CABF EV Guidelines 1.7.0 Section 9.8.2 Organization ID Present #532

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix .onion tests to only apply to EV certificates #449

Fix .onion tests to only apply to EV certificates #449

sleevi commented Jun 7, 2020 •

edited

Loading

sleevi commented Jun 7, 2020

sleevi Jun 7, 2020

cpu Jun 9, 2020

sleevi Jun 7, 2020

sleevi Jun 7, 2020

cpu Jun 9, 2020

cpu Jun 9, 2020

cpu Jun 9, 2020

Fix .onion tests to only apply to EV certificates #449

Fix .onion tests to only apply to EV certificates #449

Conversation

sleevi commented Jun 7, 2020 • edited Loading

sleevi commented Jun 7, 2020

sleevi Jun 7, 2020

Choose a reason for hiding this comment

cpu Jun 9, 2020

Choose a reason for hiding this comment

sleevi Jun 7, 2020

Choose a reason for hiding this comment

sleevi Jun 7, 2020

Choose a reason for hiding this comment

cpu Jun 9, 2020

Choose a reason for hiding this comment

cpu Jun 9, 2020

Choose a reason for hiding this comment

cpu Jun 9, 2020

Choose a reason for hiding this comment

sleevi commented Jun 7, 2020 •

edited

Loading