423002 - Incorrect error message with certificates which contain subjectAltName

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[Takeshi Nishimura]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: 3.0b4

Regression of Bug 408432

When host name mismatchs with certificate, error page tells me
"The certificate is not valid for any server names."
but it IS valid for server name(s).

Reproducible: Always

Steps to Reproduce:
1. Access https://72.14.223.191/ (IP address of www.blogger.com).
2. See the error page.
Actual Results:  
Error page says "The certificate is not valid for any server names."

Expected Results:  
Error page should say:
"The certificate is only valid for the following names:
  blogger.com , *.blogger.com  "

I suggest to change nsNSSIOLayer.cpp#GetSubjectAltNames() as follows.
- Get the pointer of variable |nameCount|.
- Count up |nameCount| even if allNames.IsEmpty().

Comment 1

[Kai Engert [:KaiE:]]

Attachment: Patch v1

Oops. Looks this got in untested. Which really surprises me. I wonder if a non-final patch revision got checked in.

Comment 2

[Kai Engert [:KaiE:]]

(In reply to comment #1)
> Oops. Looks this got in untested. Which really surprises me. I wonder if a
> non-final patch revision got checked in.

No, this indeed went in untested. We had only tested that the "new" scenario works (no names in cert). :-/

Comment 3

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b5pre) Gecko/2008031801 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

Confirming with SeaMonkey:
{{
Secure Connection Failed
72.14.223.191 uses an invalid security certificate.
The certificate is not valid for any server names.
(Error code: ssl_error_bad_cert_domain)
}}

Comment 4

[Damon Sicore (:damons)]

Should fix, but won't block the release.  wanted1.9.0.x+.  Will take a patch for 1.9.

Comment 5

[Robert Relyea]

Comment on attachment 310376 [details] [diff] [review]
Patch v1

r+

Comment 6

[Damon Sicore (:damons)]

Comment on attachment 310376 [details] [diff] [review]
Patch v1

Seems like we should test this.  Approval pending test.

Comment 7

[Kai Engert [:KaiE:]]

Damon, I recently gave a long explanation in another bug, as to why it's difficult to test the wording on the error page at this point. Adding a test would rely on external sites, which can change their behavior at any time.

I propose not to delay a helpful fix, only because we don't have automated testing.

I propose that at this point we manually test the fix after it landed using an hourly.


Renomating for approval in the hope my arguments are convincing.


Not approving it now will mean that we ship with incorrect error messages, because I can't get the test done soon.

Comment 8

[Kai Engert [:KaiE:]]

Comment on attachment 310376 [details] [diff] [review]
Patch v1

I really think this obvious correctness fix should go in.
There is no risk at all.
This is only about error pages, there is no risk to make it worse, but it will really improve our reporting.

Can you please reconsider?

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 310376 [details] [diff] [review]
Patch v1

a=beltzner, though I agree that it shouldn't block.

Can we add a litmus test? Or would that require we set up a server in a way that sends the mismatched information?

Comment 10

[Kai Engert [:KaiE:]]

checked in, marking fixed

Comment 11

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008041102 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

{{
Secure Connection Failed
72.14.223.191 uses an invalid security certificate.
The certificate is only valid for the following names:
  blogger.com , *.blogger.com  
(Error code: ssl_error_bad_cert_domain)
}}

V.Fixed