453466 - in ff3 an imported self-signed server certificate can't be marked trusted (edit button disabled)

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[al_9x]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 (.NET CLR 3.5.30729)

this is when importing as opposed to adding an exception

regression from ff2 where importing works just fine


Reproducible: Always

Comment 1

[al_9x]

This is possibly related:

when adding a exception through certificate import the server url is set to "*" and there is no way to change it.  Perhaps that is the problem.

Comment 2

[Johnathan Nightingale [:johnath]]

(In reply to comment #1)
> when adding a exception through certificate import the server url is set to "*"
> and there is no way to change it.  Perhaps that is the problem.

This should be as-designed - when you add an exception from an error page, we don't want to allow the site to sneak in a cert with a hundred altNames for paypal, ebay, etc., so we restrict it to only the server/port it came from.  When you import a cert though, you're telling us to trust it absent any context, so we don't restrict it to a particular server (obviously though, we will only trust it for servers it claims to represent), hence the "*".

I can't recreate this with a randomly selected cert, can you point to a certificate and site that has this problem?

Comment 3

[al_9x]

(In reply to comment #2)
> (In reply to comment #1)
> > when adding a exception through certificate import the server url is set to "*"
> > and there is no way to change it.  Perhaps that is the problem.
> 
> This should be as-designed - when you add an exception from an error page, we
> don't want to allow the site to sneak in a cert with a hundred altNames for
> paypal, ebay, etc., so we restrict it to only the server/port it came from. 
> When you import a cert though, you're telling us to trust it absent any
> context, so we don't restrict it to a particular server (obviously though, we
> will only trust it for servers it claims to represent), hence the "*".
> 
> I can't recreate this with a randomly selected cert, can you point to a
> certificate and site that has this problem?


I was dealing with my own makecert created certs but here is a public site with the same problem:
https://nntp.aioe.org/
Here are exact instructions (FF 3.0.1 on XP SP3):

1) Start the exception process, but don't confirm, instead view the cert and export it in PEM format
2) In the servers tab of the cert manager import the saved cert
3) go to the site, and you should get sec_error_untrusted_cert

Comment 4

[Kai Engert [:KaiE:]]

al_9x:

Importing a certificate does not assign trust to it!
The only thing you achieve, we "know" the cert before connecting to the site.
But that doesn't mean we trust it.

In the servers tab, there is an "edit" button.
When you click (selected) your imported cert showing "*", the edit button should be enabled.

Verify the cert and add the explicit trust you desire.
Connect again and it should work now.

Comment 5

[Kai Engert [:KaiE:]]

al_9x please test and give feedback, thanks.

I propose this is NOTABUG.

Comment 6

[al_9x]

(In reply to comment #4)
> al_9x:
> 
> Importing a certificate does not assign trust to it!
> The only thing you achieve, we "know" the cert before connecting to the site.
> But that doesn't mean we trust it.
> 
> In the servers tab, there is an "edit" button.
> When you click (selected) your imported cert showing "*", the edit button
> should be enabled.

the edit button is not enabled, you should be able to reproduce it with Comment #3

Comment 7

[al_9x]

> When you click (selected) your imported cert showing "*", the edit button
> should be enabled.

You are probably using redhat, could this be a windows thing?  I am on XP SP3 and it's disabled, tried it on a new profile as well.

Comment 8

[al_9x]

(In reply to comment #5)
> al_9x please test and give feedback, thanks.
> 
> I propose this is NOTABUG.

I'll changed the title to reflect the actual problem: the peer cert can't be marked trusted, because the edit button is disabled.

Can you reproduce this bug?  You said the edit button should be enabled? But is it for you?

Comment 9

[al_9x]

(In reply to comment #5)
> al_9x please test and give feedback, thanks.
> 

Kai, Johnathan

You asked me a week ago to give feedback, I have, the bug is still unconfirmed.  Is there anything more I can do to help you reproduce it?  As I said, for me, on XP SP3 with FF 3.0.1 and a new profile, the edit button is disabled for an imported self-signed server certificate.

Comment 10

[al_9x]

(In reply to comment #5)
> al_9x please test and give feedback, thanks.
> 

why ask for feedback and then ignore it?

Comment 11

[Kai Engert [:KaiE:]]

I apologize for the delay. I'm not ignoring, I'm just being slow, it's just that there is more work to do than time slots available.

Comment 12

[Johnathan Nightingale [:johnath]]

Likewise, it is not my intent to ignore, I do appreciate the work you've put in to helping diagnose these problems.  I'm failing at having enough hours in the day, really.

Comment 13

[al_9x]

Thank you both for replying.  To confirm this bug:

1) Import a self-signed server certificate to the servers tab
2) Select it and see if the edit button is enabled

This should not consume much time at all.

Comment 14

[Kai Engert [:KaiE:]]

Indeed, I can confirm the bug.
The edit button is not enabled.

Comment 15

[Kai Engert [:KaiE:]]

I think this is a regression.

The code in certManager.js, function websites_enableButtons()
does:
        var ti = serverTreeView.getTreeItem();
        if (!ti.hostPort.length) {
          enable_edit = true;
        }

My testing shows, the string hostPort contains ":443".

Apparently some code decides to provide a port number, even if there is no hostname. That's a bug.

Comment 16

[Kai Engert [:KaiE:]]

This might be a regression from bug 431909.

In the past the hostPort element was always the empty string. I think the new code from bug 413909 changed that, it didn't consider the possibility to have an empty hostname.

Comment 17

[Kai Engert [:KaiE:]]

Attachment: Patch v1 [Checkin comment 23]

Comment 18

[Honza Bambas (:mayhemer)]

Comment on attachment 343158 [details] [diff] [review]
Patch v1 [Checkin comment 23]

I would probably move the 'if (aPort == -1) aPort = 443' inside of 'if (!hostPort.IsEmpty()) {}' block. But this patch fixes the problem as is.

Comment 19

[al_9x]

(In reply to comment #17)
> Created an attachment (id=343158) [details]
> Patch v1

Is this planned for 3.1/3.5?  It's not in 3.1b3

Comment 20

[Kai Engert [:KaiE:]]

We forgot to land this regression patch in time.

Now we need approval or blocker flags...

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 343158 [details] [diff] [review]
Patch v1 [Checkin comment 23]


Could you update this cvs patch to a current hg one ? Thanks.

Comment 22

[Johnathan Nightingale [:johnath]]

Comment on attachment 343158 [details] [diff] [review]
Patch v1 [Checkin comment 23]

This has now been marked a blocker, so it doesn't require patch-level approval. It does need to land on mozilla-central before landing on branch, so that should happen very soon, please?

Comment 23

[Honza Bambas (:mayhemer)]

http://hg.mozilla.org/mozilla-central/rev/6d71ec255150

Comment 24

[Johnathan Nightingale [:johnath]]

Landed on branch:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fc6cb3fba56f