1171819 - Convert test_cert_eku-*.js to generate certificates at build time

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[:Cykesiopka]

The EKU tests are another set of tests that can be converted.

This depends on Bug 1171557 because Bug 1171557 will probably add some necessary KeyUsage parsing, and to avoid conflicts as noted in Bug 1171557 comment 0.

Comment 1

[:Cykesiopka]

Attachment: MozReview Request: Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 1: interesting bits).

Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 1: interesting bits).

Comment 2

[:Cykesiopka]

Attachment: MozReview Request: Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 2: repetitive bits).

Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 2: repetitive bits).

Comment 3

[:Cykesiopka]

I've split the patch into two parts temporarily to make review easier. I'll fold the patches before requesting check-in though.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f87aed7d99d7

Comment 4

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8617945 [details]
MozReview Request: Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 1: interesting bits).

https://reviewboard.mozilla.org/r/10709/#review9407

Looks great - just a few comments.

::: security/manager/ssl/tests/unit/test_cert_eku/generate.py:8
(Diff revision 1)
>  # classic NSS that actual testing on it is not very useful

Since we're not testing classic verification at all, this comment doesn't make much sense anymore. This raises the question of if we should add in tests for codeSigning/certificateUsageObjectSigner. Since we do specifically handle it in CertVerifier, I think we should. This can be a follow-up, though.

::: security/manager/ssl/tests/unit/pycert.py:22
(Diff revision 1)
> -extKeyUsage:[serverAuth,clientAuth,codeSigning,emailProtection]
> +extKeyUsage:[serverAuth,clientAuth,codeSigning,emailProtection,nsSGC,

We should probably document what "nsSGC" stands for here.

Comment 5

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8617946 [details]
MozReview Request: Bug 1171819 - Convert test_cert_eku-*.js to generate certificates at build time (Temp Part 2: repetitive bits).

https://reviewboard.mozilla.org/r/10711/#review9415

LGTM.

Comment 6

[:Cykesiopka]

Attachment: bug1171819_gen-test_cert_eku-certs-at-build_v2.patch

+ Add comment documenting what "nsSGC" stands for.

Comment 7

[:Cykesiopka]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> Since we're not testing classic verification at all, this comment doesn't
> make much sense anymore. This raises the question of if we should add in
> tests for codeSigning/certificateUsageObjectSigner. Since we do specifically
> handle it in CertVerifier, I think we should. This can be a follow-up,
> though.

SGTM - I filed Bug 1173659.

Comment 8

[:Cykesiopka]

Thanks for the review!

(Try push is in comment 3)

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a78836138b82

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a78836138b82

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Generating all the certs takes a fair 30-40 seconds on my system, and seems to happen for every ./mach build, even with no code changes.
Is there any way of generating them only when the xpcshell-test needs them, or at build time, but _only_ if they don't already exist?

Comment 12

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

We're exploring options in bug 1179660.