Closed Bug 1055238 Opened 10 years ago Closed 10 years ago

[B2G][Browser] Self-Signed/Untrusted/Expired SSL certificate warnings are not displaying

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: jdegeus, Assigned: keeler)

References

Details

(Keywords: regression, Whiteboard: [2.1-flame-test-run-1] [2.1-flame-test-run-2])

Attachments

(6 files, 1 obsolete file)

Attached file logcat_20140818_1332.txt (deleted) —
Description: When navigating to a website that has a untrusted/expired SSL certificate, users will observe the page loads to a blank screen and not display the SSL certificate. Repro Steps: 1) Update a Flame to 20140818040201 2) Navigate to https://tv.eurosport.com 3) Observe lack of SSL warning Actual: Expired SSL certificate webpage is not displaying Expected: Webpage with warnings for the expired SSL certificate displays Environmental Variables: Device: Flame Master (319mb) Build ID: 20140818040201 Gaia: aa8aace12d65956dd9525da5dac66e0d3b28597f Gecko: 0aaa2d3d15cc Version: 34.0a1 (Master) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Repro frequency: 3/3 Link to failed test case: https://moztrap.mozilla.org/manage/case/6757/ See attached: Screenshot and logcat
Attached image 2014-08-18-13-42-25.png (deleted) —
This issue DOES occur on Flame 2.1 (512mb) and Buri 2.1 Actual: Expired/untrusted SSL certificate warning page is not displaying Flame 2.1 (512mb) Environmental Variables: Device: Flame Master BuildID: 20140818040201 Gaia: aa8aace12d65956dd9525da5dac66e0d3b28597f Gecko: 0aaa2d3d15cc Version: 34.0a1 (Master) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Buri 2.1 Environmental Variables: Device: Buri 2.1 Master BuildID: 20140818073016 Gaia: ba1992f2addc5a84afc2eab426f222a6bf2962ba Gecko: bf27e27c994d Version: 34.0a1 (2.1 Master) Firmware: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 -------------------------------------------------------------------------------- This issue DOES NOT occur on Flame 2.0 (319mb), Flame 1.4 (319mb), Buri 2.1, Buri 2.0, Buri 1.4, Open C 2.0, and Open C 1.4 Actual: Expired SSL Certificate displays correctly Flame 2.0 (319mb) Device: Flame 2.0 BuildID: 20140818000201 Gaia: fb2dd31abed2803eb7ad67eb4c52abb48de1e0f7 Gecko: 09f7a7184c71 Version: 32.0 (2.0) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Buri 2.0 Environmental Variables: Device: Buri 2.0 Build ID: 20140818063008 Gaia: 640ce38ca03f1e26a4524ff4215b8b3f7731e2f0 Gecko: 692c93509dc9 Version: 32.0 (2.0) Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?]
Keywords: regression
Component: Gaia::Browser → Gaia::System
Summary: [B2G][Browser] Expired SSL certificate warnings are not displaying → [B2G][Browser] Self-Signed/Untrusted/Expired SSL certificate warnings are not displaying
Please correct your branch checks. You are missing environmental variables.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(jdegeus)
Please ignore comment 1 as my environmental variables were not correct. This DOES OCCUR on Flame 2.1(512mb), Buri 2.1, Open C 2.1 Actual: Expired/untrusted SSL certificate warning page is not displaying Flame 2.1 (512mb) Environmental Variables: Device: Flame Master BuildID: 20140820040203193 Gaia: df39c463259d348396ef7f143c2c780eeb8f02d8 Gecko: ffdd1a398105 Version: 34.0a1 (Master) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Buri 2.1 Environmental Variables: Device: Buri Master Build ID: 20140820073027 Gaia: 33d4b999f464fbad1c23d488da4689c5de9967ec Gecko: cbbc380f1e1c Version: 34.0a1 (Master) Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Open C 2.1 Environmental Variables: Device: Open_C Master Build ID: 20140820040203 Gaia: df39c463259d348396ef7f143c2c780eeb8f02d8 Gecko: ffdd1a398105 Version: 34.0a1 (Master) Firmware Version: P821A10V1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 --------------------------------------------------------------------- This does NOT occur on Flame 2.0(319mb), Flame 1.4 (319mb), Buri 2.0, Open C 2.0 Flame 2.0 (319mb) Environmental Variables: Device: Flame 2.0 BuildID: 20140820000201 Gaia: 88db39a0826086024631049d83ae6aa397f0918d Gecko: 2092ac87eceb Version: 32.0 (2.0) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Flame 1.4 (319mb) Environmental Variables: Device: Flame 1.4 Build ID: 20140820003001 Gaia: 4f92950e6d96326785a249e8acb704da3647616b Gecko: e1de5a959089 Version: 30.0 (1.4) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 Buri 2.0 Environmental Variables: Device: Buri 2.0 BuildID: 20140819183013 Gaia: 88db39a0826086024631049d83ae6aa397f0918d Gecko: 2092ac87eceb Version: 32.0 (2.0) Firmware: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Open C 2.0 Environmental Variables: Device: Open_C 2.0 BuildID: 20140820000201 Gaia: 88db39a0826086024631049d83ae6aa397f0918d Gecko: 2092ac87eceb Version: 32.0 (2.0) Firmware Version: P821A10V1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(jdegeus) → needinfo?(ktucker)
[Blocking Requested - why for this release]: Possibly a dupe of bug 1055328 This is a regression from 2.0 and the page just loads to a blank page so nominating to block 2.1?
blocking-b2g: --- → 2.1?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Contact: ckreinbring
Regression window Last working BuildID: 20140816143114 Gaia: 325f68045e7abacdd80f28cce53315d469e82469 Gecko: c3eb1b5ad4e4 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 First broken BuildID: 20140816144415 Gaia: 325f68045e7abacdd80f28cce53315d469e82469 Gecko: 94ba78a42305 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Working Gaia / Broken Gecko = Repro Gaia: 325f68045e7abacdd80f28cce53315d469e82469 Gecko: 94ba78a42305 Broken Gaia / Working Gecko = No repro Gaia: 325f68045e7abacdd80f28cce53315d469e82469 Gecko: c3eb1b5ad4e4 Gecko pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c3eb1b5ad4e4&tochange=94ba78a42305 Mozilla inbound Last working BuildID: 20140815111214 Gaia: d0d773c277a9105288ee35da2121f4ae62709be8 Gecko: fd08e61066de Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 First broken BuildID: 20140815112915 Gaia: d0d773c277a9105288ee35da2121f4ae62709be8 Gecko: 229181c88a3c Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Working Gaia / Broken Gecko = Repro Gaia: d0d773c277a9105288ee35da2121f4ae62709be8 Gecko: 229181c88a3c Broken Gaia / Working Gecko = No repro Gaia: d0d773c277a9105288ee35da2121f4ae62709be8 Gecko: fd08e61066de Gecko pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fd08e61066de&tochange=229181c88a3c
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Broken by bug 1029155 - Garrett?
Blocks: 1029155
Flags: needinfo?(jmitchell) → needinfo?(grobinson)
First of all, initial comment is incorrect - the test case URL's cert is not expired, it is invalid for the given domain. Note: this test case wfm on desktop, and passed try when it landed a few weeks ago. I see a MozTrap test is failing - what are those, and why aren't they included in the results from try? Anyway, I would guess the problem is here, where you guys re-implement certificate checks for error pages: http://dxr.mozilla.org/mozilla-central/source/b2g/components/ErrorPage.jsm#75 While this code has the potential to be incorrect (no guarantee that you'll get the same cert on a subsequent request to one that served a bad one), it should work most of the time. Specifically, you are checking whether the channel can provide an nsISSLStatus, and using that as the signal as to whether your connection succeeded or not. When I wrote the patch for bug 1029155, I took specific care not to disturb that (unfortunate) assumption - when I did, it broke unit tests in PSM. It seems strange that those tests would pass but the problem would occur elsewhere in the code. Perhaps there are some other differences in cert handling on B2G. Anyway, I'll try to look into this more later but I am leaving Mozilla and today is my last day, so there are no guarantees. I'm going to punt this to keeler, who is familiar with PSM (sorry keeler).
Flags: needinfo?(grobinson) → needinfo?(dkeeler)
Hmmm - looks like ErrorPage.jsm needs to be re-written in the style of bug 1025332 (i.e. pass in the failed channel and get its sslStatus - don't attempt an xhr to get the sslStatus). In general, I would be wary of modeling any new code on old code in security/manager/pki - there's a lot that needs to be updated/replaced there, and if we add new code that behaves like it, that just means more work.
Flags: needinfo?(dkeeler)
Josh - Can you finish triaging the QAnalyst-Triage flag here?
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
doug, can you help with triage here and an assignee if we have to fix this for 2.1?
Flags: needinfo?(dougt)
Assignee: nobody → kk1fff
Flags: needinfo?(dougt)
Thinker, can you guys take care of this bug?
Flags: needinfo?(tlee)
I can take care after two days.
Actually, I was wrong about what was going on here. When loading a page, the parent notifies the child of events and includes a serialized security info structure. The child deserializes this. When this happens, if the serialization includes an nsIX509Cert, a different object is created than if it were in the parent process (see nsNSSCertificate vs nsNSSCertificateFakeTransport). This is because we don't want to initialize NSS in the child process (any more than we already do due to webrtc, etc., but that's another story), and so we basically create a fake certificate object. It can't do anything, really, but at least it won't cause NSS initialization (or, rather, cause assertion failures because it attempts to do so in the child process). When we added the failedCertChain (backed by nsIX509CertList/nsNSSCertList), we neglected to include a fake nsNSSCertList that could be created in the child process that would prevent assertions as a result of attempting NSS initialization.
Assignee: kk1fff → dkeeler
Component: Gaia::System → Security: PSM
Product: Firefox OS → Core
(Note that this can be reproduced with e10s on desktop.)
OS: Gonk (Firefox OS) → All
Hardware: ARM → All
Attached patch patch 1/2: some clean-up (deleted) — Splinter Review
This is just some clean-up I thought it would be nice to do while we're here.
Attachment #8487573 - Flags: review?(rlb)
Attached patch patch 2/2: nsNSSCertListFakeTransport (obsolete) (deleted) — Splinter Review
This is the actual fix.
Attachment #8487575 - Flags: review?(rlb)
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(ktucker)
Whiteboard: [2.1-flame-test-run-1] → [2.1-flame-test-run-1] [2.1-flame-test-run-2]
Flags: needinfo?(tlee)
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(ktucker)
Comment on attachment 8487573 [details] [diff] [review] patch 1/2: some clean-up Review of attachment 8487573 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/manager/ssl/src/nsNSSCertificateFakeTransport.cpp @@ +113,3 @@ > { > NS_NOTREACHED("Unimplemented on content process"); > return NS_ERROR_NOT_IMPLEMENTED; Seems like it would be handy to macro-ize these method bodies, but not essential.
Attachment #8487573 - Flags: review?(rlb) → review+
Comment on attachment 8487575 [details] [diff] [review] patch 2/2: nsNSSCertListFakeTransport Review of attachment 8487575 [details] [diff] [review]: ----------------------------------------------------------------- The only thing that was unclear to me was how an nsNSSCertListFakeTransport object gets created. Keeler pointed out to me the "CONSTRUCTOR_BY_PROCESS" line in nsNSSModule.cpp, but I wonder if it might be helpful to have a comment somewhere to this effect. (Or maybe it's obvious.) ::: security/manager/ssl/src/nsNSSCertificateFakeTransport.cpp @@ +448,5 @@ > + > + for (size_t i = 0; i < certListLen; i++) { > + nsCOMPtr<nsIX509Cert> cert = mFakeCertList[i]; > + nsCOMPtr<nsISerializable> serializableCert = do_QueryInterface(cert); > + rv = aStream->WriteCompoundObject(serializableCert, NS_GET_IID(nsIX509Cert), Line break before NS_GET_IID? This wraps in splinter at least. @@ +482,5 @@ > + return rv; > +} > + > +NS_IMETHODIMP > +nsNSSCertListFakeTransport::GetEnumerator(nsISimpleEnumerator**) Might be a little more readable for these methods to be with the other nsIX509CertList methods above.
Attachment #8487575 - Flags: review?(rlb) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Triage group reviewed, blocking+ because regression.
blocking-b2g: 2.1? → 2.1+
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(dkeeler)
Blocks: 1063178
Comment on attachment 8490433 [details] [diff] [review] patch 2/2: nsNSSCertListFakeTransport v2 Approval Request Comment [Feature/regressing bug #]: bug 1029155 [User impact if declined]: certificate overrides are broken in b2g/e10s [Describe test coverage new/current, TBPL]: there are tests for this in non-e10s, so we shouldn't break desktop cert overrides, but unfortunately we have no direct tests for this in b2g/e10s [Risks and why]: low - I can't really think of a way this would make things worse [String/UUID change made/needed]: none
Attachment #8490433 - Flags: approval-mozilla-aurora?
Flags: needinfo?(dkeeler)
Comment on attachment 8487573 [details] [diff] [review] patch 1/2: some clean-up Approval Request Comment (see previous comment - this is just some limited cleanup and changes no functionality)
Attachment #8487573 - Flags: approval-mozilla-aurora?
Attachment #8487573 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8490433 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attached video Verify_Video_Flame v2.1.MP4 (deleted) —
This issue has been verified successfully on Flame 2.1. See attachment: Verify_Video_Flame v2.1.MP4 Reproducing rate: 0/10 Flame v2.1 version: Gaia-Rev afdfa629be209dd53a1b7b6d6c95eab7077ffcd9 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6 Build-ID 20141123001201 Version 34.0
This issue has been verified as pass on latest build of Flame 2.2 and nexus5 2.2. Repro Steps: 1) Update a Flame to 20140818040201 2) Navigate to https://tv.eurosport.com 3) Observe lack of SSL warning Rate:0/5 Device: Flame2.2[pass] Build ID 20150615162504 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8045028bf400 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150615.194936 Firmware Date Mon Jun 15 19:49:47 EDT 2015 Bootloader L1TC000118D0 Device: Nexus5 2.2[pass] Build ID 20150615002503 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f278c675d51f Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150615.040818 Firmware Date Mon Jun 15 04:08:47 EDT 2015 Bootloader HHZ12d
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: