883682 - Test EV certificate is enabled for EV outside of testing

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

When we added the test EV certificate in bug 864633, we were supposed to make it so that the test EV certificate is enabled only when an environment variable is set, and then change the test suite so that that environment variable is set.

However, it seems we forgot to add all the stuff regarding the environment variable. Consequently, we seem to be giving EV status to the test certificate when we shouldn't.

Since that landed in mozilla23, the fix for this bug will also have to be uplifted to mozilla23.

sec-moderate based on dveditz's re-rating of other address bar spoofing bugs I've rated previously.

Comment 1

[Camilo Viecco (:cviecco)]

Attachment: disable for non-debug

Proposal 1: put ev only for debug builds (notice that users still need to add the cert)

Comment 2

[Camilo Viecco (:cviecco)]

 https://tbpl.mozilla.org/?tree=Try&rev=6001d35cda5a

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 770480 [details] [diff] [review]
disable for non-debug

Review of attachment 770480 [details] [diff] [review]:
-----------------------------------------------------------------

I r+d this because we need a simple patch to uplift to -aurora and -beta.

We should figure out a real solution to this that lets us test the EV functionality in release builds too, by using an environment variable or similar to enable/disable the EV trust for the test cert.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Can we get uplift nomination for this beta/aurora simplified patch?

Comment 5

[Camilo Viecco (:cviecco)]

Merged to Central

https://hg.mozilla.org/mozilla-central/rev/77a5ea5dc277

Comment 6

[Camilo Viecco (:cviecco)]

Comment on attachment 770480 [details] [diff] [review]
disable for non-debug

[Approval Request Comment]
This is a potential security issue for users as it is possible to import a test certificate key and have it displayed as EV verified. 

Bug caused by (feature/regressing bug #): This was caused by a test in bug 864633 where to test that we actually display EV when needed.

User impact if declined: If declined an attacker that tricks the user into insalling the test certificate and can have certs shown as EV signed by 'Mozilla - EV debug test CA'.

Testing completed (on m-c, etc.): Yes, landed on central.
Risk to taking this patch (and alternatives if risky): Not much, this patch just removes a entry in a list.

String or IDL/UUID changes made by this patch: None needed.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 770480 [details] [diff] [review]
disable for non-debug

[Triage Comment]
We need this on Beta too - please re-nom if this patch doesn't apply cleanly and a different patch is needed.

Comment 8

[Camilo Viecco (:cviecco)]

Landed Aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/857e92069571

Comment 9

[Camilo Viecco (:cviecco)]

Beta landing:
https://hg.mozilla.org/releases/mozilla-beta/rev/0d0a4155b914