Closed Bug 1196853 Opened 5 years ago Closed 5 years ago

convert test_cert_signatures.js to generate certificates at build time


(Core :: Security: PSM, defect)

Not set



Tracking Status
firefox43 --- fixed


(Reporter: keeler, Assigned: keeler)




(1 file)

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.
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).
Attachment #8651941 - Flags: review?(jjones)
Comment on attachment 8651941 [details]
MozReview Request: bug 1196853 - convert test_cert_signatures.js to generate certificates at build time

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.
Attachment #8651941 - Flags: review?(jjones) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.