409091 - SSL Client Auth prompt should indicate port number

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Kai Engert [:KaiE:]]

Christina Fu reported the following usability issue:

When using Firefox session restore, multiple tabs open at the same time, more than one of them requires SSL client auth.

The browser is configured to prompt the user.

This is further complicated, because a single host runs 2 servers on different ports, and both of them require a cert.


Problem: The prompt does not show information to distinguish the different ports.

This bug requests that the server port number gets added to the display.

Comment 1

[Kai Engert [:KaiE:]]

Attachment: Patch v1

Proposed solution.
I was looking for a minimal change, so we can get this in quickly.
That means, let's simply change strings, do not try to change the UI.

Note the current UI will show the server cert's CN field to the user.

I think that's not sufficient anyway, and might actually be quite confusing.
Imagine a server cert which uses SubjectAltName extensions, and the CN is something else than the hostname we are connecting to.

I think we should display both CN and the hostname we are connecting to.
I think we should always show the port number, because this code might be used for any kind of SSL connection (not just https).


The patch does the following:

If CN and hostname are identical, it will simply append the portnumber, and look like:
  www.hostname.com:443

If CN and hostname are different, it will display both, like this:
  what CN says (www.hostname.com:443)

Comment 2

[Kai Engert [:KaiE:]]

Attachment: Patch v2

in this patch I removed some unnecessary code...

Comment 3

[Kai Engert [:KaiE:]]

Please set the approval1.9? flag on the patch once it received review. thanks.

Comment 4

[Kai Engert [:KaiE:]]

Attachment: Patch v3

third attempt to attach the correct patch...

Comment 5

[Robert Relyea]

Comment on attachment 293937 [details] [diff] [review]
Patch v3

the patch is mostly fine, but for one minor thing -- you are referencing the freed ccn variable. We really should fix this, since it's likely to skate thorugh testing and bite us much later....

(simple solution: free ccn after your test).

Also note, it appears ccn can be NULL, strcmp would not like that;).

bob

Comment 6

[Kai Engert [:KaiE:]]

Attachment: Patch v4

Thanks for catching those mistakes.

I switched to a charCleaner for ccn, which will call PR_FREEIF. I checked that both PORT_Free and PR_FREEIF eventually call the same cleanup function.

I added a non-null check to protect the strcmp.

Comment 7

[Robert Relyea]

Comment on attachment 296994 [details] [diff] [review]
Patch v4

r+ rrelyea

Comment 8

[Kai Engert [:KaiE:]]

Comment on attachment 296994 [details] [diff] [review]
Patch v4

requesting approval for a UI string enhancement

Comment 9

[Kai Engert [:KaiE:]]

fixed