1349705 - Add "TUBITAK Kamu SM SSL Kok Sertifikasi - Surum 1" to NSS

Status: RESOLVED FIXED

(NSS :: CA Certificates Code, task)

Description

[Kathleen Wilson]

Attachment: TUBITAKKamuSMSSLKokSertifikasi-Surum1.cer

This bug requests inclusion in the NSS root store of the following root certificate owned by the Government of Turkey, Kamu Sertifikasyon Merkezi (Kamu SM).
	 
Friendly Name: TUBITAK Kamu SM SSL Kok Sertifikasi - Surum 1
Cert Location: 
https://bugzilla.mozilla.org/attachment.cgi?id=8738995
http://depo.kamusm.gov.tr/ssl/SSLKOKSM.S1.cer
SHA-1 Fingerprint: 31:43:64:9B:EC:CE:27:EC:ED:3A:3F:0B:8F:0D:E4:E8:91:DD:EE:CA
SHA-256 Fingerprint: 	46:ED:C3:68:90:46:D5:3A:45:3F:B3:10:4A:B8:0D:CA:EC:65:8B:26:60:EA:16:29:DD:7E:86:79:90:64:87:16
Trust Flags: Websites
Test URL: https://testssl.kamusm.gov.tr/

Constraints: 
Please constrain this root to the following TLDs:
gov.tr, k12.tr, pol.tr, mil.tr, tsk.tr, kep.tr, bel.tr, edu.tr and org.tr

This CA has been assessed in accordance with the Mozilla project guidelines, and the certificates approved for inclusion in bug #1262809. 

The next steps are as follows:
1) A representative of the CA must confirm that all the data in this bug is correct, and that the correct certificate has been attached.
2) A Mozilla representative creates a patch with the new certificate, and provides a special test version of Firefox.
3) A representative of the CA uses the test version of Firefox to confirm (by adding a comment in this bug) that the certificate has been correctly imported and that websites work correctly.
4) The Mozilla representative requests that another Mozilla representative review the patch.
5) The Mozilla representative adds (commits) the patch to NSS, then closes this bug as RESOLVED FIXED.
6) At some time after that, various Mozilla products will move to using a version of NSS which contains the certificates. This process is mostly under the control of the release drivers for those products.

Comment 1

[Kathleen Wilson]

Tuğba, Please see step #1 above.

Comment 2

[Tuğba ÖZCAN]

Hi Kathleen,

Thanks for approving our request.

We have reviewed all the data in this bug. We confirm all the data is correct.Also, the correct certificate has been attached.

Best regards.

Comment 3

[Kai Engert (:KaiE:)]

(In reply to Kathleen Wilson from comment #0)
> 	 
> Please constrain this root to the following TLDs:
> gov.tr, k12.tr, pol.tr, mil.tr, tsk.tr, kep.tr, bel.tr, edu.tr and org.tr

I think this is the first time, where we have a request for inclusion, where the CA is to be constrained to certain domain, right from the beginning.

In the past, there were requests to apply a constraint after inclusion
  (see https://wiki.mozilla.org/CA:Root_Store_Trust_Mods#ANSSI )
and to include a CA that already contains a domain constraint embedded into the CA (HARICA bug 711594).

To implement these constraints for NSS, it will be required to implement changes at the NSS code level, potentially it can be done in a way similar to ANSSI.

Comment 4

[Kathleen Wilson]

(In reply to Kai Engert (:kaie) from comment #3)
> (In reply to Kathleen Wilson from comment #0)
> > 	 
> > Please constrain this root to the following TLDs:
> > gov.tr, k12.tr, pol.tr, mil.tr, tsk.tr, kep.tr, bel.tr, edu.tr and org.tr
> ...
> To implement these constraints for NSS, it will be required to implement
> changes at the NSS code level, potentially it can be done in a way similar
> to ANSSI.

Yes, please.

Comment 5

[Kai Engert (:KaiE:)]

Since this root addition isn't as straightforward as I had thought, it will take me a couple of days until I can help with it. If there's anyone else who would like to contribute the code changes, get that done quicker, it's appreciated.

Comment 6

[Kai Engert (:KaiE:)]

Attachment: tubitak.txt

I've prepared the addition for certdata.txt (attached), not yet the constraints.

Comment 7

[Kathleen Wilson]

(In reply to Kai Engert (:kaie) from comment #6)
> Created attachment 8851947 [details]
> tubitak.txt
> 
> I've prepared the addition for certdata.txt (attached), not yet the
> constraints.

Kai, would it help if I separate this request into two bugs? One bug for the root inclusion, so it can be done  with the March batch of root changes. And a separate bug for the constraints that will take extra work?

Comment 8

[Kai Engert (:KaiE:)]

But if we did the inclusion first, there would be a phase, where software is released that trusts the CA, without having the constraints applied. I had assumed that isn't acceptable.

Comment 9

[Kathleen Wilson]

(In reply to Kai Engert (:kaie) from comment #8)
> But if we did the inclusion first, there would be a phase, where software is
> released that trusts the CA, without having the constraints applied. I had
> assumed that isn't acceptable.

The preference is to constrain this root cert in the same NSS release that it is included in.

So, if you are able to do both the inclusion and the constraints at the same time, that would be great.

But, if you would like me to ask Keeler to add the constraints, would it be easier to handle as a separate bug?

Comment 10

[Kai Engert (:KaiE:)]

I hope to get this done today.

Comment 11

[Kai Engert (:KaiE:)]

I would like to document how I produced the name constraints extension code that we need to embed into NSS C code.

For reference, I compared with the existing one we have for ANSSI, which looks like this:
#define ANSSI_NAME_CONSTRAINTS                                                 \
    "\x30\x5D\xA0\x5B"                                                         \
    "\x30\x05\x82\x03" ".fr"                                                   \
    "\x30\x05\x82\x03" ".gp"                                                   \
    "\x30\x05\x82\x03" ".gf"                                                   \
    "\x30\x05\x82\x03" ".mq"                                                   \
    "\x30\x05\x82\x03" ".re"                                                   \
    "\x30\x05\x82\x03" ".yt"                                                   \
    "\x30\x05\x82\x03" ".pm"                                                   \
    "\x30\x05\x82\x03" ".bl"                                                   \
    "\x30\x05\x82\x03" ".mf"                                                   \
    "\x30\x05\x82\x03" ".wf"                                                   \
    "\x30\x05\x82\x03" ".pf"                                                   \
    "\x30\x05\x82\x03" ".nc"                                                   \
    "\x30\x05\x82\x03" ".tf"

I created a binary file with these ANSSI contents, and when dumper with the dumpasn1 tool, I get:
  0  93: SEQUENCE {
  2  91:   [0] {
  4   5:     SEQUENCE {
  6   3:       [2] 2E 66 72
       :       }
 11   5:     SEQUENCE {
 13   3:       [2] 2E 67 70
       :       }
 18   5:     SEQUENCE {
 20   3:       [2] 2E 67 66
       :       }
 25   5:     SEQUENCE {
 27   3:       [2] 2E 6D 71
       :       }
 32   5:     SEQUENCE {
 34   3:       [2] 2E 72 65
       :       }
 39   5:     SEQUENCE {
 41   3:       [2] 2E 79 74
       :       }
 46   5:     SEQUENCE {
 48   3:       [2] 2E 70 6D
       :       }
 53   5:     SEQUENCE {
 55   3:       [2] 2E 62 6C
       :       }
 60   5:     SEQUENCE {
 62   3:       [2] 2E 6D 66
       :       }
 67   5:     SEQUENCE {
 69   3:       [2] 2E 77 66
       :       }
 74   5:     SEQUENCE {
 76   3:       [2] 2E 70 66
       :       }
 81   5:     SEQUENCE {
 83   3:       [2] 2E 6E 63
       :       }
 88   5:     SEQUENCE {
 90   3:       [2] 2E 74 66
       :       }
       :     }
       :   }

Then I used NSS certutil --extNC, to create a certificate that contains a Name Constraints extension, with DNS names, and all of the names given in the initial comment of this bug.

Note that we should prefix each name with a dot ".". I assume that's intended, to restrict the created names to subdomains of the mentioned names, not to the mentioned subdomains by itself.

In other words, as an example, I assume the CA is permitted to create for hostnames such as example.gov.tr, but isn't allowed to create a certificate for hostname "gov.tr". Adding the "." prefix will achieve that, in my understanding. We did the same for ANSSI.

After I created the certificate with the extension, I extracted the binary extension and got:
00000000  30 81 80 a0 7e 30 0c 82  07 2e 67 6f 76 2e 74 72  |0...~0....gov.tr|
00000010  80 01 00 30 0c 82 07 2e  6b 31 32 2e 74 72 80 01  |...0....k12.tr..|
00000020  00 30 0c 82 07 2e 70 6f  6c 2e 74 72 80 01 00 30  |.0....pol.tr...0|
00000030  0c 82 07 2e 6d 69 6c 2e  74 72 80 01 00 30 0c 82  |....mil.tr...0..|
00000040  07 2e 74 73 6b 2e 74 72  80 01 00 30 0c 82 07 2e  |..tsk.tr...0....|
00000050  6b 65 70 2e 74 72 80 01  00 30 0c 82 07 2e 62 65  |kep.tr...0....be|
00000060  6c 2e 74 72 80 01 00 30  0c 82 07 2e 65 64 75 2e  |l.tr...0....edu.|
00000070  74 72 80 01 00 30 0c 82  07 2e 6f 72 67 2e 74 72  |tr...0....org.tr|
00000080  80 01 00                                          |...|

This binary file dumped with dumpasn1:
  0 128: SEQUENCE {
  3 126:   [0] {
  5  12:     SEQUENCE {
  7   7:       [2] '.gov.tr'
 16   1:       [0] 00
       :       }
 19  12:     SEQUENCE {
 21   7:       [2] '.k12.tr'
 30   1:       [0] 00
       :       }
 33  12:     SEQUENCE {
 35   7:       [2] '.pol.tr'
 44   1:       [0] 00
       :       }
 47  12:     SEQUENCE {
 49   7:       [2] '.mil.tr'
 58   1:       [0] 00
       :       }
 61  12:     SEQUENCE {
 63   7:       [2] '.tsk.tr'
 72   1:       [0] 00
       :       }
 75  12:     SEQUENCE {
 77   7:       [2] '.kep.tr'
 86   1:       [0] 00
       :       }
 89  12:     SEQUENCE {
 91   7:       [2] '.bel.tr'
100   1:       [0] 00
       :       }
103  12:     SEQUENCE {
105   7:       [2] '.edu.tr'
114   1:       [0] 00
       :       }
117  12:     SEQUENCE {
119   7:       [2] '.org.tr'
128   1:       [0] 00
       :       }
       :     }
       :   }

You may note a difference in the embedded content. The extension produced by NSS includes an additional zero attribute after each DNS name. This is reasonable when referring to RFC 5280, section 4.2.1.10:
  https://www.ietf.org/rfc/rfc5280.txt
  "Within this profile, the minimum and maximum fields are not used with
   any name forms, thus, the minimum MUST be zero, and maximum MUST be
   absent."

(I don't know which tool had been used to create the extension that was used for ANSSI, which didn't create add minimum value fields set to zero.)

I wrapped the above binary hexdump to

30 81 80 a0 7e
30 0c 82 07
2e 67 6f 76 2e 74 72
80 01 00
30 0c 82 07
2e 6b 31 32 2e 74 72
80 01 00
30 0c 82 07
2e 70 6f 6c 2e 74 72
80 01 00
30 0c 82 07
2e 6d 69 6c 2e 74 72
80 01 00
30 0c 82 07
2e 74 73 6b 2e 74 72
80 01 00
30 0c 82 07
2e 6b 65 70 2e 74 72
80 01 00
30 0c 82 07
2e 62 65 6c 2e 74 72
80 01 00
30 0c 82 07
2e 65 64 75 2e 74 72
80 01 00
30 0c 82 07
2e 6f 72 67 2e 74 72
80 01 00

then I replaced the relevant portions with more readable strings

30 81 80 a0 7e
30 0c 82 07
".gov.tr"
80 01 00
30 0c 82 07
".k12.tr"
80 01 00
30 0c 82 07
".pol.tr"
80 01 00
30 0c 82 07
".mil.tr"
80 01 00
30 0c 82 07
".tsk.tr"
80 01 00
30 0c 82 07
".kep.tr"
80 01 00
30 0c 82 07
".bel.tr"
80 01 00
30 0c 82 07
".edu.tr"
80 01 00
30 0c 82 07
".org.tr"
80 01 00

and then I transformed that to C code:

"\x30\x81\x80\xa0\x7e"
"\x30\x0c\x82\x07" ".gov.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".k12.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".pol.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".mil.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".tsk.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".kep.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".bel.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".edu.tr" "\x80\x01\x00"
"\x30\x0c\x82\x07" ".org.tr" "\x80\x01\x00"

Comment 12

[Kai Engert (:KaiE:)]

Attachment: tubitak-name-constraints-extension.der

Comment 13

[Kai Engert (:KaiE:)]

Attachment: tubitak-subject-dn.der

Comment 14

[Kai Engert (:KaiE:)]

Attachment: 1349705-constraints.patch

This is a patch to add the new name constraints.

I'll ask both Bob and David for review, to see who can do it first.
One review should be enough.

Comment 15

[Kai Engert (:KaiE:)]

Attachment: 1349705-constraints-v2.patch

Comment 16

[Ryan Sleevi]

Comment on attachment 8852873 [details] [diff] [review]
1349705-constraints-v2.patch

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

Kai: If a SEQUENCE value is the default value, it is not encoded. This is invalid DER because it explicitly encodes a DEFAULT value.

Comment 17

[Kai Engert (:KaiE:)]

Attachment: test.c

To verify that the string encodings match the expected binary data (which I extracted from certificates as attached), I used this test.c test program, which writes the encoded strings to files. The file comparison was successful.

Comment 18

[Kai Engert (:KaiE:)]

(In reply to Ryan Sleevi from comment #16)
> Kai: If a SEQUENCE value is the default value, it is not encoded. This is
> invalid DER because it explicitly encodes a DEFAULT value.

Are you referring to the additional zeros for the minimum values?

If that's invalid, then existing NSS code produces invalid NC extensions.

Comment 19

[Ryan Sleevi]

Yes, that is invalid DER to encode the minimum field of it is the DEFAULT value. That is why the ANSSI constraints did not.

Comment 20

[Kai Engert (:KaiE:)]

Thanks Ryan for your suggestion.
I've filed bug 1352064 to handle the question if the NSS tools are wrong and should be fixed.
I have the impression that it's allowed if the field is present, but let's discuss that in the other bug.

For this bug, I agree it seems better to exclude it from the encoding, as that's the way it was done in the past, and as it also matches what I've usually seen in other certs (such as the Hellenic 2011 root that we have added to NSS).

I'm trying to adjust certutil to exclude this value, so that I don't require to use a different tool to create the encoding.

Comment 21

[Kai Engert (:KaiE:)]

Attachment: tubitak-name-constraints-extension.der

Updated name constraints extension, which dumps as:

  0 101: SEQUENCE {
  2  99:   [0] {
  4   9:     SEQUENCE {
  6   7:       [2] '.gov.tr'
       :       }
 15   9:     SEQUENCE {
 17   7:       [2] '.k12.tr'
       :       }
 26   9:     SEQUENCE {
 28   7:       [2] '.pol.tr'
       :       }
 37   9:     SEQUENCE {
 39   7:       [2] '.mil.tr'
       :       }
 48   9:     SEQUENCE {
 50   7:       [2] '.tsk.tr'
       :       }
 59   9:     SEQUENCE {
 61   7:       [2] '.kep.tr'
       :       }
 70   9:     SEQUENCE {
 72   7:       [2] '.bel.tr'
       :       }
 81   9:     SEQUENCE {
 83   7:       [2] '.edu.tr'
       :       }
 92   9:     SEQUENCE {
 94   7:       [2] '.org.tr'
       :       }
       :     }
       :   }

Comment 22

[Kai Engert (:KaiE:)]

Attachment: test.c

Comment 23

[Kai Engert (:KaiE:)]

Attachment: 1349705-constraints-v3.patch

Comment 24

[Ryan Sleevi]

Comment on attachment 8852927 [details] [diff] [review]
1349705-constraints-v3.patch

Thanks Kai.

I examined the nameConstraints extension (didn't vet the subject encoding, but I presume you've verified it's byte-for-byte-equivalent), and the does properly restrict the issuance to labels at-or-below those specified domains.

That is, it prevents a "gov.tr" certificate from being issued. I agree it's unclear whether that's desired or not (and if not, it's just a matter of deleting the leading "."), so f+

Comment 25

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8852927 [details] [diff] [review]
1349705-constraints-v3.patch

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

LGTM - just the one comment.

::: lib/certdb/genname.c
@@ +1647,5 @@
> +    "\x30\x09\x82\x07" ".org.tr"
> +
> +static const SECItem builtInNameConstraints[][2] = {
> +    NAME_CONSTRAINTS_ENTRY(ANSSI),
> +    NAME_CONSTRAINTS_ENTRY(TUBITAK1) };

Might be nice to have clang-format handle this declaration/definition if it can.

Comment 26

[Kathleen Wilson]

(In reply to Kathleen Wilson from comment #0)
> 3) A representative of the CA uses the test version of Firefox to confirm
> (by adding a comment in this bug) that the certificate has been correctly
> imported and that websites work correctly.

Tuğba,

The test build with this root cert included is here:
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-331b1a3ecff20c883f4f5b135332cb1869243439/

Please test as described here:
https://wiki.mozilla.org/CA:How_to_apply#Testing_Inclusion

Comment 27

[Tuğba ÖZCAN]

Kathleen,

We tested packages in Mac OS, windows 64 bit and 32 bit platforms. There seems no problem. 

It is OK.   

Thanks.

Comment 28

[Kai Engert (:KaiE:)]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #25)
> > +static const SECItem builtInNameConstraints[][2] = {
> > +    NAME_CONSTRAINTS_ENTRY(ANSSI),
> > +    NAME_CONSTRAINTS_ENTRY(TUBITAK1) };
> 
> Might be nice to have clang-format handle this declaration/definition if it
> can.

ok

Comment 29

[Kai Engert (:KaiE:)]

Comment on attachment 8852927 [details] [diff] [review]
1349705-constraints-v3.patch

The name constraints patch has been checked in:
https://hg.mozilla.org/projects/nss/rev/f52feaa506006947b546dde5b954b59aae1bad98

Comment 30

[Kai Engert (:KaiE:)]

I've documented these new constraints on our wiki page:
https://wiki.mozilla.org/CA:Root_Store_Trust_Mods

Comment 31

[Kathleen Wilson]

Thanks, Kai!

Comment 32

[Kai Engert (:KaiE:)]

Constraints checked in to NSS 3.30 branch for NSS 3.30.2, because Kathleen has requested to include these fixes with Firefox 54.
https://hg.mozilla.org/projects/nss/rev/1fefea6530e1

Comment 33

[Kai Engert (:KaiE:)]

Constraints checked in to NSS 3.28 branch for NSS 3.28.5, because Kathleen has requested to include these fixes with Firefox 52.2.
https://hg.mozilla.org/projects/nss/rev/68fbb1b5cc75

Comment 34

[Tuğba ÖZCAN]

Hi Kai,

As we know our new root is included in NSS 3.30.2 as we see in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_3.30.2_release_notes. So, we could not understand why constraints are applied in a lower version? Is this situation affects our SSL certificates issued from old root if it constrained in lower version?

Thanks.

Comment 35

[Kai Engert (:KaiE:)]

The constraints are specifically limited to your new root CA, because they are tied to the specific subject name that is contained in your new root.

The reason why this has been checked in to an older branch, is because Mozilla maintains two different branches of Firefox.

(a) Firefox feature updates like 54, 55, 56, 57, etc.

(b) The more stable Firefox ESR release, which is kept feature stable for about one. The current branch is Firefox ESR 52, and next the upcoming versions are 52.2, 52.3, 52.3, 52.4, etc.

NSS 3.30.2 is used for (a)

NSS 3.28.5 is used for (b)

Comment 36

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:kaie) from comment #35)
> The constraints are specifically limited to your new root CA, because they
> are tied to the specific subject name that is contained in your new root.
> 
> The reason why this has been checked in to an older branch, is because
> Mozilla maintains two different branches of Firefox.
> 
> (a) Firefox feature updates like 54, 55, 56, 57, etc.
> 
> (b) The more stable Firefox ESR release, which is kept feature stable for
> about one

year

>. The current branch is Firefox ESR 52, and next the upcoming
> versions are 52.2, 52.3, 52.3, 52.4, etc.
> 
> NSS 3.30.2 is used for (a)
> 
> NSS 3.28.5 is used for (b)