1173565 - convert test_pinning_dynamic.js and test_pinning_header_parsing.js to generate certificates at build time

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Dana Keeler (she/her) [:keeler]]

Same deal as bug 1166976, bug 1171557, and bug 1171819. test_pinning_dynamic/ has a few certificates we should convert to the new system. test_pinning_header_parsing.js uses certificates in test_pinning_dynamic/, so it has to be modified slightly as well.

Comment 1

[Dana Keeler (she/her) [:keeler]]

Attachment: MozReview Request: bug 1173565 - convert test_pinning_dynamic.js to generate certificates at build time

bug 1173565 - convert test_pinning_dynamic.js to generate certificates at build time

Also fixes up references to test_pinning_dynamic certificates in test_pinning_header_parsing.js

Comment 2

[:Cykesiopka]

Comment on attachment 8621732 [details]
MozReview Request: bug 1173565 - convert test_pinning_dynamic.js to generate certificates at build time

https://reviewboard.mozilla.org/r/11035/#review9705

r+ with comments addressed.

::: security/manager/ssl/tests/unit/pycert.py:27
(Diff revision 1)
>  

subjectAlternativeName should be documented as a known extension.

::: security/manager/ssl/tests/unit/test_pinning_dynamic.js:38
(Diff revision 1)
> -const PINNING_ROOT_KEY_HASH ="kXoHD1ZGyMuowchJwy+xgHlzh0kJFoI9KX0o0IrzTps=";
> +const PINNING_ROOT_KEY_HASH ="VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=";

Nit: Add space between = and "

::: security/manager/ssl/tests/unit/test_pinning_header_parsing.js:80
(Diff revision 1)
> -const PINNING_ROOT_KEY_HASH ="kXoHD1ZGyMuowchJwy+xgHlzh0kJFoI9KX0o0IrzTps=";
> +const PINNING_ROOT_KEY_HASH ="VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=";

Nit: Add space between = and "

::: security/manager/ssl/tests/unit/test_pinning_dynamic/cn-a.pinning2.example.com-pinningroot.pem.certspec:2
(Diff revision 1)
> +subject:a.pinning2.example.com

Looks like this should have "subjectKey:alternate" as well?

::: security/manager/ssl/tests/unit/test_pinning_dynamic.js:52
(Diff revision 1)
> -  writeLine("b.pinning2.example.com:HPKP\t0\t0\t" + (now + 100000) + ",1,1,kXoHD1ZGyMuowchJwy+xgHlzh0kJFoI9KX0o0IrzTps=\n", outputStream);
> +  writeLine("b.pinning2.example.com:HPKP\t0\t0\t" + (now + 100000) + ",1,1," + PINNING_ROOT_KEY_HASH + "\n", outputStream);

Optional: these lines might look nicer using template strings.

::: security/manager/ssl/tests/unit/test_pinning_dynamic.js:96
(Diff revision 1)
> -  checkFail(certFromFile('cn-a.pinning2.example.com-badca.der'), "a.pinning2.example.com");
> +  checkFail(certFromFile('cn-a.pinning2.example.com-badca.pem'), "a.pinning2.example.com");

Optional: Append ".pem" in certFromFile() and loadCert() instead of repeating it here and below.

If the certs switch formats again some time in the future, hopefully less changes will be needed.

If not, it makes the lines shorter at least.

Comment 3

[Dana Keeler (she/her) [:keeler]]

https://reviewboard.mozilla.org/r/11035/#review9765

Thanks for the review!

> Optional: Append ".pem" in certFromFile() and loadCert() instead of repeating it here and below.
> 
> If the certs switch formats again some time in the future, hopefully less changes will be needed.
> 
> If not, it makes the lines shorter at least.

Sounds like a plan.

> Looks like this should have "subjectKey:alternate" as well?

Good catch.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c03a601e58e

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6c03a601e58e