900727 - add tests for name constraints at the psm layer

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Camilo Viecco (:cviecco)]

There are no tests for verifying that names constraints are being enforced in psm

Comment 1

[Camilo Viecco (:cviecco)]

Attachment: add-name-constraints-checks-in-psm

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 784637 [details] [diff] [review]
add-name-constraints-checks-in-psm

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

* Please rename test cases to use names that describe what is being tested. e.g. cert name could be "name-constraints-self-issued.der" or whatever.
* Please add a test case that name constraints do not prevent a self-issued certificate from working according to the special exception in RFC 5280.
* In general, make sure the important cases are covered.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 784637 [details] [diff] [review]
add-name-constraints-checks-in-psm

Clearing review flag until requested changes are made.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This will be blocking the insanity::pkix integration, so I think we should do this soon.

Comment 5

[Camilo Viecco (:cviecco)]

Putting on my weeks to check list.

Comment 6

[Camilo Viecco (:cviecco)]

(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #2)
> Comment on attachment 784637 [details] [diff] [review]
> add-name-constraints-checks-in-psm
> 
> Review of attachment 784637 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> * Please rename test cases to use names that describe what is being tested.
> e.g. cert name could be "name-constraints-self-issued.der" or whatever.
> * Please add a test case that name constraints do not prevent a self-issued
> certificate from working according to the special exception in RFC 5280.

verify(self-signed + nc, usageCA) -> OK
verify(self-signed + nc, usageSSL) -> OK? (I would say not OK)

Camilo

> * In general, make sure the important cases are covered.

Comment 7

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints

Another approach, this time more comprehensive and using the new framework (I think a look would be better)

Comment 8

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints (v1.1)

Comment 9

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints (v2)

Added the alt+cn option. There is no CN empty option as I think this is an invalid x500 name.

Comment 10

[Camilo Viecco (:cviecco)]

Attachment: tests-with-roots-constrained (part 2)

Comment 11

[Camilo Viecco (:cviecco)]

Attachment: add-sslCA-usage-test-to-nc-test

Comment 12

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8371690 [details] [diff] [review]
tests-with-roots-constrained (part 2)

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

Looks like you might have forgotten to "hg add" some files before creating this patch, e.g. cn-www.example.com-alt-example.com-int-ca-nc-perm-example.com.der

Comment 13

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8350742 [details] [diff] [review]
xpcshell-name-constraints (v2)

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

::: security/manager/ssl/tests/unit/test_name_constraints.js
@@ +8,5 @@
> +do_get_profile(); // must be called before getting nsIX509CertDB
> +const certdb = Cc["@mozilla.org/security/x509certdb;1"]
> +                 .getService(Ci.nsIX509CertDB);
> +const certdb2 = Cc["@mozilla.org/security/x509certdb;1"]
> +                  .getService(Ci.nsIX509CertDB2);

const certdb = Cc["@mozilla.org/security/x509certdb;1"]
                 .getService(Ci.nsIX509CertDB).QueryInterface(Ci.nsIX509CertDB2);

AFAICT, you don't need both certdb and certdb2.

@@ +10,5 @@
> +                 .getService(Ci.nsIX509CertDB);
> +const certdb2 = Cc["@mozilla.org/security/x509certdb;1"]
> +                  .getService(Ci.nsIX509CertDB2);
> +
> +

See my comments below about why this code should be removed.

<code-to-remove>

@@ +93,5 @@
> +  }
> +  do_check_eq(check_count, ee_count);
> +
> +}
> +

</code-to-remove>

@@ +108,5 @@
> +  let hasEVPolicy = {};
> +  let verifiedChain = {};
> +  let error = certdb.verifyCertNow(cert, certificateUsageSSLServer,
> +                                   NO_FLAGS, verifiedChain, hasEVPolicy);
> +  do_check_eq(error,  expected_error);

whitespace.

@@ +111,5 @@
> +                                   NO_FLAGS, verifiedChain, hasEVPolicy);
> +  do_check_eq(error,  expected_error);
> +}
> +
> +

too many blank lines.

@@ +120,5 @@
> +
> +  for (let i = 0 ; i < certList.length; i++) {
> +    load_cert(certList[i], ',,');
> +  }
> +  fillNameToCert();

I suggest a different way of doing the cert loading below.

@@ +128,5 @@
> +
> +add_test(function () {
> +  check_cert_err(nameToCert['cn-www.example.com-int-ca-nc-perm-example.com'],0);
> +  run_next_test();
> +});

Instead of using nameToCert, why not do this:

function constructCertFromFile(filename) {
  let der = readFile(do_get_file("test_name_constraints/" + filename, false));
  return certDB.constructX509(der, der.length);
}

function check_ok(x) {
  return check_cert_err(x, 0);
}

// The spec says <...>.
function test_SomeRequirement() {
  // The end-entity has <...> and the name constraint in the
  // intermediate <...> and we're verifying for <...> and
  // <other conditions>.
  check_ok(constructCertFromFile("cn-www.example.com-int-ca-nc-perm-example.com.der"));

  // The end-entity has <...> and <other explanation>.
  check_ok(constructCertFromFile("<whatever>.der"));

  // The end-entity has <...> ...
  check_cert_err(constructCertFromFile("<foobar>.der"),
                 SEC_ERROR_CERT_NAME_NOT_IN_NAME_SPACE);
}

// The spec says that there's an exception for ...
function test_WhateverExceptionTheSpecCallsOut()
{
  // ...
}

function run_test()
{
  // ... <setup> ...

  test_SomeRequirement();
  test_WhateverExceptionTheSpecCallsOut();
  ...

  // ... <cleanup>
}

Note in particular these tests don't need to be asynchronous, so you don't need to wrap everything in add_test() and you don't need to use run_next_test; you can put the calls to check_cert_err(...) sequentially in run_test. Undoing this would simplify the tests a lot and make them easier to read and debug (speaking from experience).

Note that constructX509 was only recently added by mmc; it didn't exist when you first wrote the tests. Note that it is idempotent so you don't have to worry about calling it multiple times for the same file. (We don't care if we read the file multiple times, because the performance impact of that is going to be zero, and readability of the test is more important.)

@@ +254,5 @@
> +                      Ci.nsIX509CertDB.TRUSTED_OBJSIGN);
> +  check_ee_for_ev("ev-valid", true);
> +  run_next_test();
> +});
> +*/

Please remove this.

::: security/manager/ssl/tests/unit/test_name_constraints/generate.py
@@ +5,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +import tempfile, os, sys
> +import random
> +import pexpect

I filed bug 969931. But, in the interest of expediency, let's not worry about bug 969931 now. I will suffer through it with my hacked-together VM infrastructure.

(Much compromise! Such pragmatism! Wow!)

@@ +25,5 @@
> +CA_full_ku = ("keyUsage = digitalSignature, nonRepudiation, keyEncipherment, " +
> +              "dataEncipherment, keyAgreement, keyCertSign, cRLSign\n")
> +
> +CA_eku = ("extendedKeyUsage = critical, serverAuth, clientAuth, " +
> +          "emailProtection, codeSigning, 1.3.6.1.5.5.7.3.9\n")

See bug 969938. Please correct this.

@@ +30,5 @@
> +
> +authority_key_ident = "authorityKeyIdentifier = keyid, issuer\n"
> +subject_key_ident = "subjectKeyIdentifier = hash\n"
> +
> +def generate_family(db_dir, dst_dir, ca_key, ca_cert, base_name):

Please write a comment describing what a "family" is.

@@ +33,5 @@
> +
> +def generate_family(db_dir, dst_dir, ca_key, ca_cert, base_name):
> +    key_type = 'rsa'
> +    ee_ext_base = EE_basic_constraints + authority_key_ident;
> +    #cn =example.com

# CN=example.com

(ditto below).

@@ +54,5 @@
> +                                    ca_key,
> +                                    ca_cert,
> +                                    'www.example.org')
> +    #cn = example.com, alt= example.org
> +    alt_name_ext = 'subjectAltName =DNS:*.example.org'

Is the space before "=" necessary? If not, I suggest removing it. (ditto below).

@@ +100,5 @@
> +                                    'www.example.org')
> +
> +
> +
> +def self_sign_csr(db_dir, dst_dir, csr_name, key_file, serial_num, ext_text,

Please add a comment explaining the purpose of this.

@@ +125,5 @@
> +                                                        1,
> +                                                        key_type,
> +                                                        'ca-nc',
> +                                                         ca_ext)
> +    #now the constrained via perm

I don't understand this comment. s/perm/permittedSubtrees/. Also, grammar. (ditto below).

It would be good to get more descriptive comments in this function in general.

@@ +171,5 @@
> +                                    name,
> +                                    ca_ext + name_constraints + authority_key_ident,
> +                                    ca_key,
> +                                    ca_cert)
> +    generate_family(db, srcdir, int_key, int_cert, name) 

trailing whitespace.

Comment 14

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Camilo, besides the comments above, I suggest three things:

1. Go ahead and fold all three parts together.
2. It is important to add a comment to each test case (or set of test cases) explaining what is being tested, and/or explaining how one can figure out what is being tested based on the names of the certificates.
3. It is especially important to add a comment to the test cases where we think the NSS functions were and/or are getting things wrong, because those are the cases that I need to adjust for insanity::pkix.

I will update mozilla-inbound with NSS 3.16 beta 1 as soon as I am finished leaving this comment.

Comment 15

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Sorry, the NEEDINFO was for this:

Are all the test cases that you added in the NSS test suite recently being tested in this test suite? It would be good to make sure that this test suite is the same or a superset of those tests.

Comment 16

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8372682 [details] [diff] [review]
add-sslCA-usage-test-to-nc-test

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

::: security/manager/ssl/tests/unit/test_name_constraints.js
@@ +131,5 @@
>  
> +///
> +add_test(function () {
> +  let cert = certdb.findCertByNickname(null, 'int-ca-nc-perm-example.com');
> +  check_cert_err_generic(cert, 0 , certificateUsageSSLCA);

If I am understanding this correctly, this is verifying an intermediate certificate. This is an important test case to have so we let's make sure we keep it.

It would be good to have a case where we're validating an intermediate and we expect the validation to fail too.

Comment 17

[Camilo Viecco (:cviecco)]

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #15)
> Sorry, the NEEDINFO was for this:
> 
> Are all the test cases that you added in the NSS test suite recently being
> tested in this test suite? It would be good to make sure that this test
> suite is the same or a superset of those tests.

No, we have different coverages. Thse tests are missing:
1. test for constraining the SubjectName (ie O=foo, e=bar)
2. test where root (no NC) -> int (nc) -> int (non-nc, but in bad namespace) -> ee

And this have some that NSS does not:
1. test where root is NC

Let me figure out a good naming scheme and add those to the tests.

Comment 18

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8350742 [details] [diff] [review]
xpcshell-name-constraints (v2)

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

::: security/manager/ssl/tests/unit/test_name_constraints/generate.py
@@ +22,5 @@
> +CA_basic_constraints = "basicConstraints = critical, CA:TRUE\n"
> +EE_basic_constraints = "basicConstraints = CA:FALSE\n"
> +
> +CA_full_ku = ("keyUsage = digitalSignature, nonRepudiation, keyEncipherment, " +
> +              "dataEncipherment, keyAgreement, keyCertSign, cRLSign\n")

Please shorten this to keyCertSign, cRLSign. It is confusing to have the other KUs here when they don't affect the tests.

@@ +25,5 @@
> +CA_full_ku = ("keyUsage = digitalSignature, nonRepudiation, keyEncipherment, " +
> +              "dataEncipherment, keyAgreement, keyCertSign, cRLSign\n")
> +
> +CA_eku = ("extendedKeyUsage = critical, serverAuth, clientAuth, " +
> +          "emailProtection, codeSigning, 1.3.6.1.5.5.7.3.9\n")

Actually, please remove CA_eku completely. It doesn't affect the test.

@@ +28,5 @@
> +CA_eku = ("extendedKeyUsage = critical, serverAuth, clientAuth, " +
> +          "emailProtection, codeSigning, 1.3.6.1.5.5.7.3.9\n")
> +
> +authority_key_ident = "authorityKeyIdentifier = keyid, issuer\n"
> +subject_key_ident = "subjectKeyIdentifier = hash\n"

Please remove this because it doesn't affect the test results.

In general, abbreviate the script to the minimum (within reason) needed to generate certs with the fewest extensions, to make it clear exactly what is being tested.

Comment 19

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The Marketplace certificate stuff doesn't really require name constraint enforcement, so we can defer these tests until later. Also, in order for insanity::pkix to pass these tests, improvements to the error prioritization is needed; see bug 970810.

Comment 21

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints (v3)

almost superset of nss tests

Comment 22

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints (v 3.1)

This is missing the insanity switch and explicit comments on the tests. But since there are 150 certs doing by hand is extemely tedius. Seems it is time to automate the generation of the certs AND the basic structure of the test suite.

Comment 23

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: name-constraints-xpcshell.patch

Camilo, your patch is really good. I fixed up some nits. I have some questions, which are marked as "XXX" in this patch. Please address those.

Comment 24

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Camilo, see this TBPL run:
https://tbpl.mozilla.org/?tree=Try&rev=1637f6e895db

Some of the Windows builds are failing. I suspect that the problem is that some of the file path names are too long. I suggest that you try to shorten the file path names and see if the Windows builds succeed on TBPL. I didn't have any trouble locally, so I suspect that it is an issue with how our build automation does things. For example, the build automation may put the inputs and/or outputs of the build in a path that is longer than the path I use; I use c:\p\firefox\src as $srcdir and c:\p\firefox\ff-dbg as $OBJDIR.

Comment 25

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #24)
> Camilo, see this TBPL run:
> https://tbpl.mozilla.org/?tree=Try&rev=1637f6e895db

Also, only pay attention to the Windows build failures in that run. The other failures, the xpcshell failures in particular, are due to a problem with a patch of mine that was also in the push.

Comment 26

[Camilo Viecco (:cviecco)]

Thanks for the windows build fail. I renamed some of the certs. And seems like I fixed some but busted some

Comment 27

[Camilo Viecco (:cviecco)]

https://tbpl.mozilla.org/?tree=Try&rev=41b6f6424400

Comment 28

[Camilo Viecco (:cviecco)]

Attachment: xpcshell-name-constraints (v3.2)

Addressed your comments, I also changed the loading of the intermediates, so that they are loaded with inherited trust into the db.

Comment 29

[Camilo Viecco (:cviecco)]

 https://tbpl.mozilla.org/?tree=Try&rev=536cb7cbd59f

Comment 30

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8380843 [details] [diff] [review]
xpcshell-name-constraints (v3.2)

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

I'm assuming that the cert generation script didn't change from last time except for the shortening of names.

Thanks for getting this done. I know it must have been extremely tedious!

::: security/manager/ssl/tests/unit/test_name_constraints.js
@@ +11,5 @@
> +
> +function constructCertFromFile(filename) {
> +  let der = readFile(do_get_file("test_name_constraints/" + filename, false));
> +  return certdb.constructX509(der, der.length);
> +}

Note that constructCertFromFile now exists in head_psm.js (as of today), so it would be good to choose a non-conflicting name like "certFromFile" and implement it in terms of the constructCertFromFile in head_psm.js.

@@ +56,5 @@
> +
> +  // Testing with a unconstrained root, and intermediate constrained to PERMIT
> +  // foo.com. All failures on this section are doe to the cert DNS names
> +  // not being under foo.com.
> +  check_ok_ca(load_cert('int-nc-perm-foo.com-ca-nc', ',,'));

It's cool that we are calling check_ok_ca on the intermediate certificates now.

However, I am concerned about the JS GC causing the intermediate certificate to be GC'd before the following lines ran. The reason I suggested that we do:

let int_nc_perm_foo_com_ca_nc = load_cert('int-nc-perm-foo.com-ca-nc', ',,');
check_ok_ca(int_nc_perm_foo_com_ca_nc);

is so that the certificate is **guaranteed** to stay in memory while the following lines run. If you are certain that the GC can't run here then I guess it is OK, but I don't know how we could be certain, so I think that the style I suggested is more correct and we should change to the style I suggested.

@@ +75,5 @@
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-nc-perm-foo.com-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-nc-perm-foo.com-ca-nc.der')); 
> +  // multiple subjectAltnames
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-nc-perm-foo.com-ca-nc.der'));
> +

extra blank line.

@@ +81,5 @@
> +  // Testing with an unconstrained root and intermediate constrained to
> +  // EXCLUDE DNS:example.com. All failures on this section are due to the cert
> +  // DNS names containing example.com. The dirname does not affect evaluation.
> +  check_ok_ca(load_cert('int-nc-excl-foo.com-ca-nc', ',,'));
> +  // no dirName 

trailing whitespace.

@@ +120,5 @@
> +  check_ok(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.com-int-nc-c-us-ca-nc.der'));
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-nc-c-us-ca-nc.der'));
> +  check_ok(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-nc-c-us-ca-nc.der'));
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-nc-c-us-ca-nc.der'));
> +

extra blank line.

@@ +141,5 @@
> +  check_ok(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.com-int-nc-foo.com-int-nc-c-us-ca-nc.der'));
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-nc-foo.com-int-nc-c-us-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-nc-foo.com-int-nc-c-us-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-nc-foo.com-int-nc-c-us-ca-nc.der'));
> +

extra blank line.

@@ +182,5 @@
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.com-int-nc-perm-c-uk-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-nc-perm-c-uk-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-nc-perm-c-uk-ca-nc.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-nc-perm-c-uk-ca-nc.der'));
> +

extra blank line.

@@ +243,5 @@
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-nc-foo.com-int-nc-foo.com_a.us.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-nc-foo.com-int-nc-foo.com_a.us.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-nc-foo.com-int-nc-foo.com_a.us.der'));
> +
> +

extra blank line.

@@ +262,5 @@
> +  check_ok(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.com-int-ca-nc-perm-foo.com.der'));
> +  check_ok(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-int-ca-nc-perm-foo.com.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.org_o-bar_c-us-alt-foo.org-int-ca-nc-perm-foo.com.der'));
> +  check_fail(constructCertFromFile('cn-www.foo.com_o-bar_c-us-alt-foo.com-a.a.us-b.a.us-int-ca-nc-perm-foo.com.der'));
> +

extra blank line.

@@ +271,5 @@
> +  load_cert("ca-nc", "CTu,CTu,CTu");
> +
> +  test_certs_all();
> +}
> +

many extra blank lines here.

::: security/manager/ssl/tests/unit/test_name_constraints/generate.py
@@ +271,5 @@
> +                                    int_cert,
> +                                    '/C=US/CN='+ name)
> +    generate_family(db, srcdir, int_key, int_cert, name)
> +
> +

extra blank line.

@@ +338,5 @@
> +                                    ca_key,
> +                                    ca_cert)
> +    generate_family(db, srcdir, int_key, int_cert, name)
> +
> +

extra blank lines.

@@ +360,5 @@
> +                                    name,
> +                                    ca_ext + name_constraints + authority_key_ident,
> +                                    ca_key,
> +                                    ca_cert)
> +    generate_family(db, srcdir, int_key, int_cert, name) 

trailing whitespace.

@@ +362,5 @@
> +                                    ca_key,
> +                                    ca_cert)
> +    generate_family(db, srcdir, int_key, int_cert, name) 
> +
> +

extra blank line.

::: security/manager/ssl/tests/unit/xpcshell.ini
@@ +75,2 @@
>  
> +

extra blank line.

Comment 31

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Removing dependency on bug 967153 since enough of bug 967153 (Update to NSS 3.16) already landed for this, so that we can clean up the dependency tree for bug 915930 to better track the burn down to enabling insanity::pkix.

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/89b4178ff5e7

Comment 33

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Uplifted to Mozilla-Aurora, a=test-only:
https://hg.mozilla.org/releases/mozilla-aurora/rev/05fa60d7cc5c