1196853 - convert test_cert_signatures.js to generate certificates at build time

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

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

test_cert_signatures.js should generate certificates at build time. Also, while it covers tampered signatures, it doesn't cover tampered certificates (i.e. there are no cases where some part of the certificate is modified but the signature is kept intact). We should add those additional cases.

Comment 1

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

Attachment: MozReview Request: bug 1196853 - convert test_cert_signatures.js to generate certificates at build time

bug 1196853 - convert test_cert_signatures.js to generate certificates at build time

Also add additional testcases that weren't in the original test (tampered
signatures had been tested, but tampered certificates hadn't been covered).

Comment 2

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 8651941 [details]
MozReview Request: bug 1196853 - convert test_cert_signatures.js to generate certificates at build time

https://reviewboard.mozilla.org/r/16657/#review15857

Everything's nitpicky; it looks good. I apologize that my workload has kept me overdue on this.

::: security/manager/ssl/tests/unit/test_cert_signatures.js:51
(Diff revision 1)
> +  let base64 = readAndTamperWithNthByte(certificatePath, -8);

So the intent here is to modify one of the bytes representing the signature bit string at the end of the cert, right? Nit: May be good to say that.

::: security/manager/ssl/tests/unit/test_cert_signatures.js:65
(Diff revision 1)
> +// Since we want to modify the serial number, we need to change something from

Good walkthrough.

::: security/manager/ssl/tests/unit/test_cert_signatures.js:78
(Diff revision 1)
> -    do_print("cert_name=" + cert_name);
> +  let base64 = readAndTamperWithNthByte(certificatePath, 17);

If these methods were any further apart, I'd recommend using a constant for 17 so that it's clear your helpful comment above applied to both. Nit: Still would be a good idea for posterity.

Comment 3

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

Thanks for the review!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=70bacfe2c62e

Comment 4

[Pulsebot]

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

Comment 5

[Carsten Book [:Tomcat]]

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