1187029 - convert test_bug480509.html to an xpcshell test

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

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

See bug 1174286. test_bug480509.html doesn't need to be a mochitest.

Comment 1

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

Attachment: MozReview Request: bug 1187029 - convert test_bug480509.html to an xpcshell test r?jcj

bug 1187029 - convert test_bug480509.html to an xpcshell test r?jcj

Comment 2

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

Comment on attachment 8638184 [details]
MozReview Request: bug 1187029 - convert test_bug480509.html to an xpcshell test r?jcj

https://reviewboard.mozilla.org/r/14045/#review12827

r=jcj with comments addressed.

::: security/manager/ssl/tests/unit/test_cert_embedded_null.js:21
(Diff revision 1)
> +  checkCertErrorGeneric(certdb, cert, SSL_ERROR_BAD_CERT_DOMAIN,

I feel like we should also test that it's not www.bad-guy.com. Ditto in the SAN test.

::: security/manager/ssl/tests/unit/test_cert_embedded_null/embeddedNullSAN.pem.certspec:2
(Diff revision 1)
> +subject:embedded NUL in SAN

Typically certs with a SAN still have a common name that is one of the listed dNSnames. If the validation code is sufficiently separated that you don't think you need to test nulls in both at once, then I'm good with this as-is.

If there's a chance that there's connections in the logic, I would recommend you make a 3rd test case that has a 2-entry SAN and a CN, such that the CN and SAN[0] are reasonable, and SAN[1] has an embedded null -- that is exactly what I'd do, if I were trying to confuse a browser.

Comment 3

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

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

Comment 4

[Pulsebot]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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