847495 - nsISSLStatus is broken

Status: RESOLVED INVALID

(Core :: Security: PSM, defect)

Description

[Gervase Markham [:gerv]]

I have a small extension - Expiry Canary - whose simple function is to tell people when certs on sites they are visiting are about to expire:
https://addons.mozilla.org/en-US/firefox/addon/expiry-canary/

Sometime between Firefox 19 and 20, the following code, which runs when
you hit an SSL site:

41  var ui = gBrowser.securityUI;
42  sp = ui.QueryInterface(Components.interfaces.nsISSLStatusProvider);
43  status = sp.SSLStatus;
44  status = status.QueryInterface(Components.interfaces.nsISSLStatus);
45  var cert = status.serverCert;
...

stopped working with this error:

Timestamp: 01/03/13 17:30:36
Error: TypeError: status.QueryInterface is not a function
Source File: chrome://canary/content/canary.js
Line: 44

The way to test if it works is to install Expiry Canary, set the pref extensions.canary.warningPeriod to a very large number, then visit any SSL site. If the extension is working, you'll see a black warning at the top of the page. If it's not, due to this bug, you'll see nothing.

It works in FF 19 (currently release), and not in FF 20 (currently beta). I used mozregression to narrow down a range on mozilla-central:

Last good nightly: 2012-12-22
First bad nightly: 2012-12-23

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf26f61a0748&tochange=84320dffec6e

That range includes the upgrade to NSS 3.14.2 beta 1 (bug 823705, http://hg.mozilla.org/mozilla-central/rev/9475349dcd81), which seems like the prime candidate for breaking it, although I don't know enough about NSS to figure out what broke it. mozregression lets me compile more builds to narrow it down more, so I'm doing that, and I'll update this bug.

Gerv

Comment 1

[Gervase Markham [:gerv]]

No success yet; it started doing branch-aware bisecting into the m-i -> m-c merge, but then my machine unaccountably shut down (overheating?) and I lost my progress.

Gerv

Comment 2

[Gervase Markham [:gerv]]

I have no idea how it could be, and it makes me think that maybe I did it wrong, but the mozregression script says:

The first bad revision is:
changeset:   116851:c3b8f166c3b5
user:        Jeff Walden <[e-mail]>
date:        Wed Dec 19 15:57:18 2012 -0500
summary:     Bug 823283 - Don't do anything special for writable [Replaceable] properties, a Gecko-specific abomination.  r=bz
Regression found using mozcommitbuilder 0.4.10 on linux2 at 2013-03-07 17:16:52

CCing jwalden and bz. Am I on crack? 

(This regression finding took about 3 days to do, because mozcommitbuilder doesn't seem to default to a sensible value for -j.) 

Gerv

Comment 3

[Jeff Walden [:Waldo]]

It seems unlikely that would be the regressing change, as those properties are really only found/tested for on the window object.  But I've done nothing more than read comment 0, so possibly I'm missing something, or comment 0 isn't quite the right thing to point out.  Dunno.  In any case, I mostly doubt that caused this.

Comment 4

[Gervase Markham [:gerv]]

Jeff: you are right. I have no idea what went wrong with my bisection. I'm now attempting to find the problem manually.

Gerv

Comment 5

[Alex Keybl [:akeybl]]

Honza - can you take a look given the STR in comment 0? We want to start investigating even without a regression range, given how close we are to release of FF20 with NSS 3.14.

Who knows what cherished security add-ons make use of nsISSLStatus - we should resolve before release.

Comment 6

[Alex Keybl [:akeybl]]

Gerv - if you run out of steam, please add qawanted to help find a regression window.

Comment 7

[Honza Bambas (:mayhemer)]

I'll jump on this on Monday.

Comment 8

[Gervase Markham [:gerv]]

Events have overtaken me, but so far I've discovered that mozilla-inbound is good at least as far as:

e4b389a23a95: Bug 824516 - Fix regression in text selection introduced in bug 708048. r=kats

I test by installing Expiry Canary in a clean profile, setting the timeout pref to a large value, and setting the homepage to https://www.paypal.com/. Then when I've built, I just run. If I get an infobar, it's GOOD. If not, it's BAD (and I check the JS console).

But the fact that my bisection failed _might_ mean the problem is intermittent. I've retested some revisions I earlier marked as BAD when bisecting and they now work...

Hope that's useful.

Gerv

Comment 9

[Honza Bambas (:mayhemer)]

Fix your extension:

var Canary = {};

Canary.checkExpiryDate = function(aEvent, location) {
    var ui = gBrowser.securityUI;
-   sp = ui.QueryInterface(Components.interfaces.nsISSLStatusProvider);
+   var sp = ui.QueryInterface(Components.interfaces.nsISSLStatusProvider);
    status = sp.SSLStatus;
    status = status.QueryInterface(Components.interfaces.nsISSLStatus);

INVALID.

Comment 10

[Jeff Walden [:Waldo]]

Strict mode would have saved you here, or at least made the error obvious:

https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Functions_and_function_scope/Strict_mode

Comment 11

[Gervase Markham [:gerv]]

So why did it suddenly stop working, after working for years? And why does the error I got have nothing to do with the problem?

Gerv

Comment 12

[Jeff Walden [:Waldo]]

Maybe we started calling that method more often?  I dunno, I'm totally guessing and haven't looked at what function's being called, or when, there -- don't have time to look more closely.

Comment 13

[Honza Bambas (:mayhemer)]

Somebody probably introduced 'sp' global var or simply something has changed in the JS engine rules that now prevents assignment to an undeclared variable.