Remove custom cert check code from app update

RESOLVED FIXED in Firefox 51

Status

()

Toolkit
Application Update
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

After mar signing is used on Linux (it already is on Mac and Windows) the cert checks can be disabled in bug 1151485. After that has baked for a short while the custom cert checking code can be removed from app update.
The Firefox preferences should also be cleaned up in this bug
Filed bug 1189843 and bug 1189845 for Thunderbird and SeaMonkey to remove the cert checks. This bug doesn't depend on those bugs being fixed since app update will still work for those apps when this code is removed though it will remove the security mitigation.
If for whatever reason mar signing is not implemented then real cert pinning should be implemented. We didn't do this for Firefox aus since it has the same problems as the custom cert check. See bug bug 1063111
Created attachment 8780467 [details] [diff] [review]
nsISecurityUITelemetry changes
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Created attachment 8781904 [details] [diff] [review]
client changes rev1
Attachment #8780468 - Attachment is obsolete: true
Comment on attachment 8780467 [details] [diff] [review]
nsISecurityUITelemetry changes

Hi Dan, as you might recall we no longer use the custom certificate attribute check code since it causes issues with some use cases and rely on mar signing instead. The patches in this bug remove the checks and there is one change in nsISecurityUITelemetry.idl that I am hoping you are ok with reviewing. Thanks!
Attachment #8780467 - Flags: review?(dveditz)
Attachment #8781904 - Attachment description: client changes in progress → client changes rev1
Attachment #8781904 - Flags: review?(mhowell)
Comment on attachment 8781905 [details] [diff] [review]
test changes rev1

Try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1386b4a18f94&selectedJob=25870034
Attachment #8781905 - Attachment description: test changes in progress → test changes rev1
Attachment #8781905 - Flags: review?(mhowell)

Updated

2 years ago
Attachment #8781904 - Flags: review?(mhowell) → review+
Comment on attachment 8781905 [details] [diff] [review]
test changes rev1

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

Everything looks good and the try push is green, so I'm happy. Thanks!
Attachment #8781905 - Flags: review?(mhowell) → review+
Bug 1295896 removes several tests that would also need to be modified so adding dependency.
Depends on: 1295896
Created attachment 8782159 [details] [diff] [review]
test changes rev2

Forgot to hg remove a couple of tests. Carrying forward r+
Attachment #8781905 - Attachment is obsolete: true
Attachment #8782159 - Flags: review+
Created attachment 8782172 [details] [diff] [review]
Firefox preference

Matt, forgot to remove this no longer available preference from firefox.js
Attachment #8782172 - Flags: review?(mhowell)
Comment on attachment 8782172 [details] [diff] [review]
Firefox preference

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

No problem; I saw your comment saying this needed to be removed, and then I also forgot about it.
Attachment #8782172 - Flags: review?(mhowell) → review+
Comment on attachment 8780467 [details] [diff] [review]
nsISecurityUITelemetry changes

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

r=dveditz
Attachment #8780467 - Flags: review?(dveditz) → review+

Comment 16

2 years ago
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bd0ad73562b
nsISecurityUITelemetry.idl - Remove custom cert check code from app update. r=dveditz
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bf5bb2dc20d
client code - Remove custom cert check code from app update. r=mhowell
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b648a542bfb
test code - Remove custom cert check code from app update. r=mhowell
https://hg.mozilla.org/integration/mozilla-inbound/rev/b717faf86fe7
Firefox preference removal - Remove custom cert check code from app update. r=mhowell

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4bd0ad73562b
https://hg.mozilla.org/mozilla-central/rev/1bf5bb2dc20d
https://hg.mozilla.org/mozilla-central/rev/4b648a542bfb
https://hg.mozilla.org/mozilla-central/rev/b717faf86fe7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.