435743 - Extension manager should load updates served from https signed by any installed CA

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[:Mook]

Attachment: random xpcshell test

See patch.

If the user permanently accepts a cert for https purposes, that server is completely usable in the browser (or other app).  Except in the case of the extension update mechanism, which explicitly checks for having a built-in root cert.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/mozapps/shared/src/badCertHandler.js&rev=1.5&mark=59-60#44

See also bug 340198 where this behaviour was implemented.  Note that the current implementation rejects user click-through certs (because of mismatches, expired, whatever), as intended.  Additionally, it also rejects any sort of corporate / CCK self-signed certs, anything an extension might install, or otherwise anything that wasn't from a dialog popped up during random surfing.

Attached patch is git-format hg diff (because it holds the binary security bits from bug 426867 comment 3 modified to actually work).  Also, it's crap for 1) needing to randomly launch ssltunnel to do the proxying, 2) needing things to end up being the right order, 3) needing the phase of the moon to be just right.

Comment 1

[Nils Maier [:nmaier]]

bug #378216 added "insecure extension updates".
It has some commentary on self-signed/local CA-signed certificates as well.

shaver asked to file this, so I guess a blocking flag should be considered.
See: http://groups.google.com/group/mozilla.dev.extensions/browse_thread/thread/6aad62d4e3bd21ad

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

No, that I asked for something to be filed doesn't mean it should block. :)  I think we should accept user-installed CAs (not exception certs) for extension update, though, in 3.next.

Comment 5

[Peter Nemsak]

The thing is, you can't even approve the certificate manually in the addon manager, it just says "there's been an error". Even I had to look for why this is happening and what this "error" means, and I'm by no means just a regular user. The user should have an option of allowing the extension to be updated, one security dialog from time to time won't kill anyone. There's quite some of them now anyway. For me it's just like entering a non-secure or unknown-ca alert for a location - you can step through, or leave. But in the addon manager you don't even have the chance, just to install the extension anew, which actually can be done from an unknown-ca location.

Comment 6

[Dave Townsend [:mossop]]

I do not believe that yet another incomprehensible dialog is a good way to go here. I would generally agree with comment 2, we should allow updates if the server's cert is signed by an installed CA (whether built in or not), but we should ignore servers who have had an exception added for them.

Comment 7

[Johnathan Nightingale [:johnath]]

(In reply to comment #5)
> The user should have an option of allowing the extension to be updated,
> one security dialog from time to time won't kill anyone.

I disagree with this sentiment pretty strongly...

(In reply to comment #6)
> I do not believe that yet another incomprehensible dialog is a good way to go
> here. I would generally agree with comment 2, we should allow updates if the
> server's cert is signed by an installed CA (whether built in or not), but we
> should ignore servers who have had an exception added for them.

...but I agree with most of this.  I can entertain a meaningful distinction in my head between "level of verifiable identity needed to let my browser render your content" and "level of same needed to let my machine execute your code."  Given that McCoy gives devs a way to manage secure updates without a CA-issued cert, and that organizations have the Internal CA approach available to them, I think we needn't go out of our way to enable other broken-cert scenarios.

tbh, I'm a bit surprised we aren't already working with added CAs (if I understand that to be part of the complaint) - I'd have thought that would come along for free.

Having said all that, though, I would certainly support us having a better message than just "error" - if that is indeed what we have.

Comment 8

[:Mook]

(In reply to comment #7)
> tbh, I'm a bit surprised we aren't already working with added CAs (if I
> understand that to be part of the complaint) - I'd have thought that would come
> along for free.
See the URL linked in comment 0; the whole point of this bug is that the root certificate issuer _must_ be called "Builtin Object Token" (literally, not localizable).  So either your organization happens to be called that, or you must hack NSS (I think that's edit http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/pki1/oids.txt and run oidgen.perl).  You can't just externally install a new CA using some sort of CCK.  I would imagine this would also happen to break the normal redistribution clauses (no longer shipping the stock binary) and would need explicit agreements (that I assume would be granted, but it _is_ a needlessly complicated step).

(In reply to comment #6)
> I do not believe that yet another incomprehensible dialog is a good way to go
> here. I would generally agree with comment 2, we should allow updates if the
> server's cert is signed by an installed CA (whether built in or not), but we
> should ignore servers who have had an exception added for them.
There must be some distinction between "non-built-in installed CA" and "servers with exception" I am unaware of; my (quite possibly flawed) understanding was that NSS (or possibly, PSM) treats them as the same.  Heck, I didn't even think there was any distinction (other than not getting saved) for "accept for session" certs.

Not that that actually matters, given how many of http://mxr.mozilla.org/mozilla-central/search?string=scheme.*https&regexp=on&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central is hard-coded scheme checking (instead of "do I trust this" checking)...

(I am very carefully avoiding any discussion on what sort of UI needs to be necessary; I just care that this becomes possible.  Most folks are smarter than me anyway.)

Comment 10

[patrickdrd]

so guys,
what's the process with this one?

there are many people having problems in updating extensions
(including me of course)
and firefox itself

is there a patch we could use or we should wait for a new build,
which would allow updating of extensions and fx as well?

thanks in advance!

Comment 11

[Dave Townsend [:mossop]]

(In reply to comment #10)
> so guys,
> what's the process with this one?

If you read through the bug comments you'll see that we are still deciding exactly how best to improve this situation without allowing users to compromise themselves.

> there are many people having problems in updating extensions
> (including me of course)
> and firefox itself

This bug is about extension updates only, Nothing we do here will impact Firefox updates.

> is there a patch we could use or we should wait for a new build,
> which would allow updating of extensions and fx as well?

Since we are still discussing and there are no patches attached to this bug no. You will need to wait for someone to implement an appropriate fix.

Comment 12

[patrickdrd]

ok thanks,
I hope there is a solution soon,
because my addons/extensions should be outdated
(haven't been updated for some time now)

apart from that though,
I can't check for firefox updates itself and I'm almost sure
the reason is the same:
even though I added the site in certificate exceptions,
refused to check for updates,
because of this new certificate handling

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> apart from that though,
> I can't check for firefox updates itself and I'm almost sure
> the reason is the same:
> even though I added the site in certificate exceptions,
> refused to check for updates,
> because of this new certificate handling

Yes, the firefox auto-updates have the same restrictions, but I don't believe there is any intention to fix that at this time, but like I say that is a different issue.

Also this is not a new behaviour, Firefox has done this since Firefox 2 and later versions of Firefox 1.5

Comment 14

[patrickdrd]

thanks again dave for you answer,
but what if I don't want to user https to get my updates,
can I disable that?

let's say I don't care about all this security stuff,
there should be a dumm option to totally disable https/ssl updates and pass them through simple http,
it there one?

Comment 15

[Johnathan Nightingale [:johnath]]

(In reply to comment #14)
> thanks again dave for you answer,
> but what if I don't want to user https to get my updates,
> can I disable that?
> 
> let's say I don't care about all this security stuff,
> there should be a dumm option to totally disable https/ssl updates and pass
> them through simple http,
> it there one?

Dave is, of course, welcome to tell me I'm wrong, but I'm pretty certain that we don't, and won't, support subvertible updates.  Addons have privilege equivalent to Firefox itself - they can manipulate the user's data, or even damage the user's computer.  When a user decides to install an addon, they're deciding to trust the author not to do any of those unsavoury things, but they are not trusting the internet at large, which is what an http update represents.

If such an option existed, it would create a strong temptation for lazy addon authors to flip it, rather than having to bother with a secure solution.  Moreover though, there is no really compelling reason for doing so - free options for signing updates or hosting addons on SSL sites exist, so I don't believe the bar we currently set is too high.  Fixing bugs like this one feels like a much better use of resources than developing ways around our security measures.

Comment 16

[P]

First to answer last comment on http being 'the internet at large':
no it is not: you can check the adress in your browser.
i downloaded firefox 3.0 from filehippo.com over normal http, 
and in fact I think I will be uninstalling it very soon indeed.
Why?
trying to connect to services.addons.mozilla.org:443 i get:
The certificate is not trusted because the issuer certificate is not trusted.
which issuer? could ff be more specific? Anyway i added the exception.
if firefox themselves can't get their certificate in order, who can?

but in these discussions one remark just makes sense: 
Verisign MUST have paid firefox for this nonsense, as it does NOT make browsing safer. Safety comes from Distinguishing unsafe from reasonably safe to secure.

So MANY websites use SSL with certificates that will NOT meet the criteria, that users will get USED to the adding of exceptions, just like klicking away the UAC warnings in Vista... (eg. if vista would describe which process is trying to do the intented action and what this action will change to my system, i could make an INFORMED decision)

What would HELP is actually making the distiction mentioned above between paid 3rd party 'trusted' certificates, and other types.

For me the if the url is correct, i assume i am pretty safe, or am i wrong?
and if you really would like to 'protect' the users, why not disable installing add-ons in the first place, because the act of installing one from unknown sourse is still possible and this poses much greater risk then just updating an add-on that has been functioning properly and been trusted for years.

for me the decision is taken. ff 3.0 has fallen to corruption of the big bugs
(bugs as in cash from verisign)

Comment 17

[Dave Townsend [:mossop]]

(In reply to comment #14)
> thanks again dave for you answer,
> but what if I don't want to user https to get my updates,
> can I disable that?

No, when the add-on developer asks Firefox to use a secure update URL we will and we will block anything that suggests that secure URL or your connection has been compromised.

> let's say I don't care about all this security stuff,
> there should be a dumm option to totally disable https/ssl updates and pass
> them through simple http,
> it there one?

It is up to the add-on developer to decide whether to use https or http for their add-on updates. Likewise I believe Mozilla will only ever offer updates over https.

(In reply to comment #15)
> Dave is, of course, welcome to tell me I'm wrong, but I'm pretty certain that
> we don't, and won't, support subvertible updates.

There is an option extensions.checkUpdateSecurity to allow installing add-ons that don't meet the security restrictions of Firefox 3, but that doesn't help you if the add-on does use secure updates (which any add-on that is intended to be used on Firefox 3 will).

(In reply to comment #16)
I'm only going to respond to bits of this comment in the context of this bug, you seem to be talking about the much wider issues that have been discussed widely elsewhere. As as this as it happens but there we go.

> First to answer last comment on http being 'the internet at large':
> no it is not: you can check the adress in your browser.
> i downloaded firefox 3.0 from filehippo.com over normal http

Sounds like you agree that http is the internet at large here.

> trying to connect to services.addons.mozilla.org:443 i get:
> The certificate is not trusted because the issuer certificate is not trusted.
> which issuer? could ff be more specific? Anyway i added the exception.
> if firefox themselves can't get their certificate in order, who can?

This is really a separate bug that a few people have reported but we've been unable to confirm. Perhaps it would be worth you commenting in bug 445957 so we can try to track this down.

> So MANY websites use SSL with certificates that will NOT meet the criteria,
> that users will get USED to the adding of exceptions, just like klicking away
> the UAC warnings in Vista... (eg. if vista would describe which process is
> trying to do the intented action and what this action will change to my system,
> i could make an INFORMED decision)

I'm not sure the number is actually that high, but if it is then this is actually a good argument on why we should continue to not install add-on updates that come from user-accepted ssl sites IMO.

> For me the if the url is correct, i assume i am pretty safe, or am i wrong?

Yes you are wrong, unless the url is also https and the certificate is valid for the url then you could really be getting data from aywhere

> and if you really would like to 'protect' the users, why not disable installing
> add-ons in the first place, because the act of installing one from unknown
> sourse is still possible and this poses much greater risk then just updating an
> add-on that has been functioning properly and been trusted for years.

This is absolutely correct. The focus of the security restrictions is to ensure that once you have an add-on installed you cannot become compromised by installing an update to it. It is still important that you only install add-ons from sites and authors that you trust. We do have a bug open to look at placing security restrictions on the install process, see bug 429319

Comment 18

[P]

I will have a look at the bug445957. Dave thanks.

about http I agree it's 'possibly' unsafe, but what are the chances?
if http really juct could from anywhere, show me some proof, and some figures about how often this occurs (DNS failing and users being fooled) please...
if i trust www.filehippo.com over http, you might call it 'unsafe' but really what are my chances of really downloading from them and not someone else?
right: 99.9% if you not agree maybe we could do some testing?
personally I worry more about Microsoft implementing keyloggers and traffic loggers to windows. (why the constant harddisk activity since XP, do you remember nice and quit 95?)

Anyway to my AMAZEMENT two of the sites that regularly used to bug me as in ff1.5 as I had certificates set to: 'ask every time', and sites like klm.com, moneybookers.com, skype.com and other biggies used to ask for secure connections to sites like google.analytics.com and ad.tracker.com etc etc. which i manually had to cancel every single time. It kept bugging me that booking my flights i had to click away secure connections to all kinds of third party sites (klm used 4! =4 clicks for every single step/page)

it seems that the hardwired impossibility to connect to sites if not all the certificates are in order have lead these sites to very quickly (last 2 days) DROP these links to secure ad/statistics servers from their sites, as atleast from my old ff1.5 installation i do NOT get these annoyances anymore.

this might be a possitive result!

Which leaves me in doubt now (until further testing) if ff3.0 will even give me the ability to block these unwanted connections... will it?

I loved this solution: http://www.cs.cmu.edu/~perspectives/firefox.html
it will look at 4 different servers that keep certificates and find out if there is a man-in-the-middle or not with almost certainty and not bug you if the self-published certificate is found in good order (=unchanged over several days over several pathways over the internet.)

over and out for this thead! thank you for working on this.

Comment 19

[notagain]

Just a question in order to know if my problem is actually this bug (and prevent posting a duplicate of it):

When I try to update my addons, after the check is done, I see the following msg.:

"An error occurred while trying to find updates for [name_of_addon]"

I've set the 'extensions.logging.enabled' to true in about:config and noticed the following error in the error console:

"Datasource: Addon Update Ended: {addon ID}, status: 4"

(Sometimes, for some addons it shows "status: 8")...and the msg is followed by:

"RDFItemUpdater::onXMLLoad: TypeError: tokenNames is null"

Well, is this the same issue or am I facing some other?

Comment 20

[Dave Townsend [:mossop]]

(In reply to comment #19)
> Just a question in order to know if my problem is actually this bug (and
> prevent posting a duplicate of it):
> 
> (Sometimes, for some addons it shows "status: 8")...and the msg is followed by:
> "RDFItemUpdater::onXMLLoad: TypeError: tokenNames is null"
> Well, is this the same issue or am I facing some other?

Yes that looks like it is likely the same issue.

Comment 21

[Dave Townsend [:mossop]]

(In reply to comment #20)
> (In reply to comment #19)
> > Just a question in order to know if my problem is actually this bug (and
> > prevent posting a duplicate of it):
> > 
> > (Sometimes, for some addons it shows "status: 8")...and the msg is followed by:
> > "RDFItemUpdater::onXMLLoad: TypeError: tokenNames is null"
> > Well, is this the same issue or am I facing some other?
> 
> Yes that looks like it is likely the same issue.

I don't think that error should be happening though. File it as a new bug please. It looks to be caused by bug 401292 so copy in the assignee from there.

Comment 22

[youness44]

6 months ago, now... 
* Unable to update from https
* Unable to update with CLI mccoy
https://bugzilla.mozilla.org/show_bug.cgi?id=396525
Does someone has a simple solution for updating from a no-x server located on a lan?

Comment 23

[Dave Townsend [:mossop]]

Attachment: New https servers [checked in]

I have a set of testcases for xpi installation and extension updates that verify the current behaviour is as we expect and can easily be tweaked to test the behaviour we want to move to. This first patch is an update to server-locations and the cert database for the test ssl server. It adds a self-signed https server and a https server signed by a CA that the test Firefox instance doesn't know about so both are considered errors and need exceptions to work.

The commands used to modify the cert database were:

certutil -S -n "selfsigned" -s "CN=self-signed.example.com" -x -t "P,," -m 1 -v 120 -d .
certutil -S -s "unknown_ca" -s "CN=Unknown CA" -t "C,," -x -m 1 -v 120 -n "Unknown CA" -2 -d .
certutil -S -n "untrusted" -s "CN=untrusted.example.com" -c "Unknown CA" -t "P,," -k rsa -g 1024 -m 923727398177686238923340102948 -v 120 -d .

Comment 24

[Dave Townsend [:mossop]]

Attachment: chrome tests [checked in]

These chrome tests verify the current behaviour. Specifically for both update.rdf and xpi installs they test:

regular http
valid https (signed by a non-built in CA so currently mostly fails as expected)
https with a mismatched certificate
https with a self-signed certificate
https signed by an unknown CA

combinations of the above starting on one and redirecting to the others.

The tests are run once, then the 3 error https servers have exceptions added and the tests are run again to verify the expected change in behaviour (not much, we only care about exceptions for the https request if the original request was to http and it was redirected to the https with exception).

Comment 25

[(not currently active) Ted Mielczarek]

Comment on attachment 355964 [details] [diff] [review]
New https servers [checked in]

+http://nocert.example.com:80         privileged
+http://self-signed.example.com:80    privileged
+http://untrusted.example.com:80      privileged

Do you really need these, or is this just to work around bug 468087? (If so, can you add a comment there, so we can remove them once that's landed?)

Comment 26

[Dave Townsend [:mossop]]

(In reply to comment #25)
> (From update of attachment 355964 [details] [diff] [review])
> +http://nocert.example.com:80         privileged
> +http://self-signed.example.com:80    privileged
> +http://untrusted.example.com:80      privileged
> 
> Do you really need these, or is this just to work around bug 468087? (If so,
> can you add a comment there, so we can remove them once that's landed?)

Yeah they are only there so it works, if that bug is fixed before this lands I'll just remove them.

Comment 27

[Robert Strong (they/them - no direct email)]

Comment on attachment 355965 [details] [diff] [review]
chrome tests [checked in]

Dave, I will most likely get to this by Tuesday... is that soon enough for you?

Comment 28

[Dave Townsend [:mossop]]

(In reply to comment #27)
> (From update of attachment 355965 [details] [diff] [review])
> Dave, I will most likely get to this by Tuesday... is that soon enough for you?

Yeah that's fine. They're just tests that I know pass already.

Comment 29

[Dave Townsend [:mossop]]

Comment on attachment 355964 [details] [diff] [review]
New https servers [checked in]

I've landed the servers patch as http://hg.mozilla.org/mozilla-central/rev/6b0b886f541d and included a 2k increase in the expected leaks for browser chrome tests. This is explained in bug 441359 comment 24 and bug 473802 is on file to try to fix it.

The actual tests patch cannot land for now as it seems to be leaking or exposing some real leaks.

Comment 30

[Dave Townsend [:mossop]]

The leak increase actually went in as http://hg.mozilla.org/mozilla-central/rev/c989bb1f272d

Comment 31

[Dave Townsend [:mossop]]

These bugs resolve the leaks that the testcases were exposing. Once they are fixed the tests can land.

Comment 32

[Dave Townsend [:mossop]]

Comment on attachment 355965 [details] [diff] [review]
chrome tests [checked in]

Pushed the tests as http://hg.mozilla.org/mozilla-central/rev/9b79ff7b4707

Comment 33

[Josh Aas]

Attachment: test failures #1

tests were disabled because they caused other test failures, at least some are in this text file

http://hg.mozilla.org/mozilla-central/rev/69c0f4f17728

Comment 34

[Dave Townsend [:mossop]]

(In reply to comment #33)
> Created an attachment (id=362275) [details]
> test failures #1
> 
> tests were disabled because they caused other test failures, at least some are
> in this text file
> 
> http://hg.mozilla.org/mozilla-central/rev/69c0f4f17728

I'm pretty sure I saw a clear run of these tests on Linux, and the failures are already intermittently failing as described in bug 478241 since well before these tests were landed.

Comment 35

[Dave Townsend [:mossop]]

(In reply to comment #34)
> (In reply to comment #33)
> > Created an attachment (id=362275) [details] [details]
> > test failures #1
> > 
> > tests were disabled because they caused other test failures, at least some are
> > in this text file
> > 
> > http://hg.mozilla.org/mozilla-central/rev/69c0f4f17728
> 
> I'm pretty sure I saw a clear run of these tests on Linux, and the failures are
> already intermittently failing as described in bug 478241 since well before
> these tests were landed.

In fact there are definitely clear runs. Perhaps these tests are doing something that makes bug 478241 worse or something though.

Comment 36

[Dave Townsend [:mossop]]

I've re-enabled the tests in http://hg.mozilla.org/mozilla-central/rev/4e9612353fab and added some logging to the tests that might have been failing with this to try to narrow down what is going on.

Comment 37

[Dave Townsend [:mossop]]

Disabled the tests again. It looks like what is happening is the mochichrome tests are somehow causing the ssltunnel process to stay alive. As the browser chrome tests start we get the following in the log:

failed to bind socket
Shutting down...

After that attempts to use the ssl tunnel will fail giving us bad test results.

Comment 38

[(not currently active) Ted Mielczarek]

Ah, guess I didn't catch that in the logs. Interesting. Wasn't bug 464191 supposed to make shutting down ssltunnel work reliably?

Comment 39

[Dave Townsend [:mossop]]

(In reply to comment #38)
> Ah, guess I didn't catch that in the logs. Interesting. Wasn't bug 464191
> supposed to make shutting down ssltunnel work reliably?

My current guess is that using SIGKILL to kill ssltunnel might be a bit abrupt. Perhaps the process is actually getting killed but the port is still in use as far as the OS is concerned? I know I've hit this before in the albeit distant past on windows. I'm considering whether ssltunnel should have a signal handler and cleanly close up all connections when sent the kill signal or anything else appropriate. I have a small WIP patch to do this, but since I still can't reproduce this locally I don't really have any good idea that it will help. Nor am I really sure yet why these tests trigger this problem.

Comment 40

[Jeff Walden [:Waldo]]

We can probably do a "gentle" shutdown if we look for an incoming connection that doesn't start off with a CONNECT foobar:12345 line without a whole lot of trouble, then do basically the same thing as we do to shut down the httpd.js server with a Python request.

Comment 41

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

You can't handle SIGKILL, but doing a close/shutdown of the socket in response to SIGTERM would probably work.

Comment 42

[Jason W]

There's another use case that I don't see mentioned here - where the user is behind a transparent proxy that does man in the middle for HTTPS. In this case, the user is without extension updates even with the proxy CA cert installed.

Comment 43

[Johan Ferner]

I just got bit by Comment #42, the transparent proxy. I have installed the CA cert and it works for normal web browsing, but updating extensions fails with:

*** RDFItemUpdater::onXMLLoad: cert issuer is not built-in

on a number of important plugins after switching on extensions logging. FF 3.5.1/Linux.
 
Is a fix in the pipe for 3.5?

Comment 44

[Dave Townsend [:mossop]]

(In reply to comment #43)
> I just got bit by Comment #42, the transparent proxy.
> 
> Is a fix in the pipe for 3.5?

There is no fix planned that would solve your problem. I can't really see how we could resolve that without losing all the security we have built up.

Comment 45

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We could authenticate the content, rather than the transport, by signing the update data.  Since the vast majority of update pings give back the same "no update" content, a very simple cache would alleviate almost all of the performance issues.

We could also permit the use of CA roots if they are selected by the user as being usable for code-signing, since that's effectively what we're doing with them.

Comment 46

[Dave Townsend [:mossop]]

(In reply to comment #45)
> We could authenticate the content, rather than the transport, by signing the
> update data.  Since the vast majority of update pings give back the same "no
> update" content, a very simple cache would alleviate almost all of the
> performance issues.

We already have the capability to do this. Individual add-ons can use a non-https mechanism as they like. We could include a default key for AMO perhaps, but your assertion isn't totally correct. The regular update pings return the latest version information for the add-on, not "no update", so caching may be more problematic making this not viable, the AMO guys would need to make a call there.

> We could also permit the use of CA roots if they are selected by the user as
> being usable for code-signing, since that's effectively what we're doing with
> them.

Well the planned fix is to allow any trusted CA (not just built in ones) to work for update checks. Assuming the user had some way to find the CA for the transparent proxy and trust it then the problem would be solved, I just don't think there is an easy way to do that right now is there?

Not sure where code-signing comes into this.

Comment 47

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Why wouldn't they be able to find the CA for the transparent proxy? They had to install the root, and the trust control is in the PSM UI, I'm pretty sure.

Comment 48

[Johan Ferner]

My organization publishes the trusted CA so I have imported this as a trusted CA into Firefox otherwise I get loads of "man-in-the-middle" https warnings. But it is ignored when updating addons. I was hoping this bug was about that.

Comment 49

[Dave Townsend [:mossop]]

(In reply to comment #48)
> My organization publishes the trusted CA so I have imported this as a trusted
> CA into Firefox otherwise I get loads of "man-in-the-middle" https warnings.
> But it is ignored when updating addons. I was hoping this bug was about that.

Ok that is a slightly different circumstance to the one I was thinking of, yes this bug is about fixing that case, but still I'm not sure if a fix will be coming on the 3.5 line, perhaps in 3.6.

Comment 50

[Jason W]

(In reply to comment #48)
> My organization publishes the trusted CA so I have imported this as a trusted
> CA into Firefox otherwise I get loads of "man-in-the-middle" https warnings.
> But it is ignored when updating addons. I was hoping this bug was about that.

Just for clarification, this the use case I am running into. Like Johan, I have installed a root cert from our proxy device and still see the above issue - all other HTTPS transactions are happy. To update extensions, I have to bypass the proxy device, which isn't always feasible, so I don't think that would be a long-term solution.

Not knowing anything about how Firefox is developed or the effort it would take, I would think the fix would be to trust user-install root certs for extension updates.

Comment 51

[Dave Townsend [:mossop]]

Bug 538008 fixes the problem with these tests.

Comment 52

[Dave Townsend [:mossop]]

Attachment: Add expired ssl certificate [checked in]

This adds an expired ssl certificate to the store using the following command:

certutil -d . -S -n "expired" -s "CN=expired.example.com" -c "pgo temporary ca" -t "P,," -m 2387 -k rsa -g 1024 -v 0

Comment 53

[Dave Townsend [:mossop]]

Attachment: enable tests and add expired ssl tests

This adds testing of expired ssl certs and re-enables the tests.

Comment 54

[(not currently active) Ted Mielczarek]

Comment on attachment 420270 [details] [diff] [review]
Add expired ssl certificate [checked in]

Consider this a blanket-r+ on adding SSL certs to the testing db + associated changes to server-locations.txt. (Tell your friends!)

Hard to review binary blobs anyway. Just make sure you include the command you used to generate them in the commit message or something.

Comment 55

[Dave Townsend [:mossop]]

Comment on attachment 420270 [details] [diff] [review]
Add expired ssl certificate [checked in]

Pushed the certdb changes: http://hg.mozilla.org/mozilla-central/rev/91815742f281

Comment 56

[Robert Strong (they/them - no direct email)]

Comment on attachment 420272 [details] [diff] [review]
enable tests and add expired ssl tests

>diff --git a/toolkit/mozapps/extensions/test/test_bug435743_2.xul b/toolkit/mozapps/extensions/test/test_bug435743_2.xul
>--- a/toolkit/mozapps/extensions/test/test_bug435743_2.xul
>+++ b/toolkit/mozapps/extensions/test/test_bug435743_2.xul
>@@ -78,16 +78,24 @@ function addCertOverrides() {
>   try {
>     req = new XMLHttpRequest();
>     req.open("GET", "https://untrusted.example.com/" + xpi, false);
>     req.channel.notificationCallbacks = new badCertListener("untrusted.example.com",
>                                                             Ci.nsICertOverrideService.ERROR_UNTRUSTED);
>     req.send(null);
>   }
>   catch (e) { }
>+  try {
>+    req = new XMLHttpRequest();
>+    req.open("GET", "https://.example.com/" + xpi, false);
Seems like you want https://expired.example.com/ here and elsewhere.

Comment 57

[Dave Townsend [:mossop]]

Attachment: expired ssl tests rev 2 [checked in]

This would be the correct version of the patch which actually passes.

Comment 58

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #52)
> Created an attachment (id=420270) [details]
> Add expired ssl certificate
> 
> This adds an expired ssl certificate to the store using the following command:
> 
> certutil -d . -S -n "expired" -s "CN=expired.example.com" -c "pgo temporary ca"
> -t "P,," -m 2387 -k rsa -g 1024 -v 0

You regenerated the cert8.db and key3.db files using 'make genservercert' in build/pgo/, rather than making manual modifications, right?  Otherwise those modifications will get blown away the next time somebody runs that (which needs to be done regularly to prevent the non-expired certs from expiring).

Comment 59

[Dave Townsend [:mossop]]

Comment on attachment 420404 [details] [diff] [review]
expired ssl tests rev 2 [checked in]

Checked in the tests: http://hg.mozilla.org/mozilla-central/rev/965b4d380482

Comment 60

[Dave Townsend [:mossop]]

(In reply to comment #58)
> (In reply to comment #52)
> > Created an attachment (id=420270) [details] [details]
> > Add expired ssl certificate
> > 
> > This adds an expired ssl certificate to the store using the following command:
> > 
> > certutil -d . -S -n "expired" -s "CN=expired.example.com" -c "pgo temporary ca"
> > -t "P,," -m 2387 -k rsa -g 1024 -v 0
> 
> You regenerated the cert8.db and key3.db files using 'make genservercert' in
> build/pgo/, rather than making manual modifications, right?  Otherwise those
> modifications will get blown away the next time somebody runs that (which needs
> to be done regularly to prevent the non-expired certs from expiring).

I filed bug 538294 on making sure we can regenerate everything in the certificate db

Comment 61

[Serge Gautherie (:sgautherie)]

Comment on attachment 420404 [details] [diff] [review]
expired ssl tests rev 2 [checked in]

>diff --git a/toolkit/mozapps/extensions/test/Makefile.in b/toolkit/mozapps/extensions/test/Makefile.in
>@@ -62,18 +62,18 @@ _CHROME_FILES = \
> # disabled due to potentially breaking other tests

Shouldn't this comment be removed?

>-# libs:: $(_CHROME_FILES)
>-# 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)
>+libs:: $(_CHROME_FILES)
>+	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)

Comment 62

[Dave Townsend [:mossop]]

(In reply to comment #61)
> (From update of attachment 420404 [details] [diff] [review])
> >diff --git a/toolkit/mozapps/extensions/test/Makefile.in b/toolkit/mozapps/extensions/test/Makefile.in
> >@@ -62,18 +62,18 @@ _CHROME_FILES = \
> > # disabled due to potentially breaking other tests
> 
> Shouldn't this comment be removed?

Yes, good catch, I'll just remove it after tinderbox has done some green runs.

Comment 63

[Serge Gautherie (:sgautherie)]

(In reply to comment #62)
> Yes, good catch, I'll just remove it after tinderbox has done some green runs.

http://hg.mozilla.org/mozilla-central/rev/0a062eef0c8c

Comment 64

[Timothy Nikkel (:tnikkel)]

There was a timeout in a test for this bug:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263072347.1263080135.16728.gz

Comment 65

[Henri Sivonen (:hsivonen)]

Timed out again:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263184094.1263191954.20963.gz

Comment 66

[Dave Townsend [:mossop]]

Filed bug 539033 on the test timeouts.

Comment 67

[Dave Townsend [:mossop]]

Attachment: test refactoring

This does some refactoring of the tests to make them far easier to read. No actual changes should be going on here.

Comment 68

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This (finally) fixes this bug, at least mostly. It adds two new preferences:

extensions.install.requireBuiltInCerts
extensions.update.requireBuiltInCerts

These are hidden and default to true. When set to false they allow non-built-in certs to be used for downloading XPIs and update.rdfs respectively (the latter may be slightly mis-named as it doesn't apply to the whole update process). The updated tests verify all the cases.

Comment 69

[Robert Strong (they/them - no direct email)]

Comment on attachment 471865 [details] [diff] [review]
test refactoring

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_installssl.js b/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>--- a/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>+++ b/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>...
>@@ -86,297 +105,133 @@ function run_install_tests(callback) {
> // Add overrides for the bad certificates
> function addCertOverrides() {
>   addCertOverride("nocert.example.com", Ci.nsICertOverrideService.ERROR_MISMATCH);
>   addCertOverride("self-signed.example.com", Ci.nsICertOverrideService.ERROR_UNTRUSTED);
>   addCertOverride("untrusted.example.com", Ci.nsICertOverrideService.ERROR_UNTRUSTED);
>   addCertOverride("expired.example.com", Ci.nsICertOverrideService.ERROR_TIME);
> }
> 
>+// Runs tests with non-built in certificates not allowed, no certificate
>+// exceptions and no hashes
I think it reads better as "with built-in certificates required" vs. "non-built in certificates not allowed" but I'm fine either way. If you do change it please change it throughout.

Comment 70

[Robert Strong (they/them - no direct email)]

Comment on attachment 471871 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_installssl.js b/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>--- a/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>+++ b/toolkit/mozapps/extensions/test/browser/browser_installssl.js
>...
>@@ -167,19 +175,84 @@ add_test(function() {
>   add_install_test(EXPIRED,    NOCERT,     NETWORK_FAILURE);
>   add_install_test(EXPIRED,    SELFSIGNED, NETWORK_FAILURE);
>   add_install_test(EXPIRED,    UNTRUSTED,  NETWORK_FAILURE);
>   add_install_test(EXPIRED,    EXPIRED,    NETWORK_FAILURE);
> 
>   run_install_tests(run_next_test);
> });
> 
>+// Runs tests with non-built in certificates allowed, no certificate
>+// exceptions and no hashes
nit: I think this reads better as "without requiring built-in certificates" vs. "with non-built in certificates allowed". If you decide to change it please do so throughout.

Comment 71

[Dave Townsend [:mossop]]

Comment on attachment 471871 [details] [diff] [review]
patch rev 1

This is a small, well tested, low-risk patch that has no effect unless certain preferences are set however without it we cannot really test many cases of SSL downloads of extensions/updates.

Comment 72

[Dave Townsend [:mossop]]

landed: http://hg.mozilla.org/mozilla-central/rev/975d22824233
http://hg.mozilla.org/mozilla-central/rev/297b77bb8868

Comment 73

[Chris P. Ross]

Now that these changes are landed, but not marked as blocking 1.9.2, will they get into the next (or a soon) 3.6.x release?  If not, is there any way that this could be accelerated?

I have an internal https server with an internally signed cert.  I can get the internal CA into the list on all clients, but without this change I still can't manage updating my extension.

Comment 74

[Dave Townsend [:mossop]]

(In reply to comment #73)
> Now that these changes are landed, but not marked as blocking 1.9.2, will they
> get into the next (or a soon) 3.6.x release?  If not, is there any way that
> this could be accelerated?

This fix will probably not be making it into a 3.6.x release. The patch would have to be completely rewritten to apply there and this does not fall into teh category of fixes that we take on the branch at the moment.

Comment 75

[Chris P. Ross]

I'm sorry to hear that, but do understand.

I will research setting up an http server with signature hashs instead, as there seems to be no workaround to have it delivered via https.

Thank you.

Comment 76

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave, what's the best way for QA to test this fix? Seems like it's not that easy.

Comment 77

[Dave Townsend [:mossop]]

(In reply to comment #76)
> Dave, what's the best way for QA to test this fix? Seems like it's not that
> easy.

It would require an ssl webserver using a certificate issued by a CA that we don't trust. I recall that IT have a Mozilla CA that is used for some internal sites, it is possible that that might be useful here.

Comment 78

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed the installation part with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20101006 Firefox/4.0b7pre by using the self-signed cert on my server. I have not tested the update path because I think that we can trust the automated tests.

Comment 79

[Chris Cunningham]

Is it intended that the new hidden preferences apply only to addon updates and not to browser updates? The same bug still seems to exist for browser updates. Is that a separate bug?

Comment 80

[Dave Townsend [:mossop]]

(In reply to comment #79)
> Is it intended that the new hidden preferences apply only to addon updates
> and not to browser updates? The same bug still seems to exist for browser
> updates. Is that a separate bug?

Yes this bug is all about the extension manager, you'd need to file a different bug for app updates.