Closed Bug 1148766 Opened 9 years ago Closed 9 years ago

Secure connection failed (sec_error_bad_der) due to certs with SAN dNSName entries incorrectly containing IP addresses

Categories

(Core :: Security: PSM, defect)

Product:
Core
Component:
Security: PSM
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
firefox37 + wontfix
firefox38 + wontfix
firefox39 - wontfix
firefox40 - wontfix
firefox-esr31 --- unaffected
firefox-esr38 39+ wontfix

People

(Reporter: jdm, Assigned: keeler)

References

Details

(Keywords: regression, site-compat)

Attachments

(2 files, 2 obsolete files)

patch v3
11.99 KB, patch
briansmith
: feedback-
Details | Diff | Splinter Review
certdump-mobil.dhl.de.txt
27.94 KB, text/plain
Details 
I can view https://en.environicsanalytics.ca/prizm5_lookup.aspx in Chrome without any problems, but in Firefox I see

"An error occurred during a connection to en.environicsanalytics.ca. security library: improperly formatted DER-encoded message. (Error code: sec_error_bad_der) "
NSS' pp tool has this to say about the end-entity cert:
> Name: Certificate Subject Alt Name
> DNS name: "www.environicsanalytics.ca"
> DNS name: "172.24.200.4"
> IP Address: 172.24.200.4
> DNS name: "www.environicsanalytics.com"
> DNS name: "38.99.163.50"
> IP Address: 38.99.163.50
> DNS name: "alyx"
> DNS name: "crm"
> DNS name: "edev"
> DNS name: "etest"
> DNS name: "edemo"
> DNS name: "eserver"
> DNS name: "w5"
> DNS name: "ea.environicsanalytics.ca"
> DNS name: "162.209.49.252"
> IP Address: 162.209.49.252
> DNS name: "en.environicsanalytics.ca"

IIUC, |DNS name: "172.24.200.4"| etc are the problematic entries in this incorrectly generated cert.

Moving to PSM for now in case I'm wrong, to allow someone more knowledgeable to correct me.
Component: Networking → Security: PSM
OS: Mac OS X → All
Hardware: x86 → All
ni? David on whether Comment 1 is correct.
Flags: needinfo?(dkeeler)
Yes - you're correct. I think in cases where we can't decode naming information, it might make sense to treat that like a domain name mismatch. However, we would want to make sure that the UI is consistent with this (e.g. we should probably fix bug 1024781 before doing this).
Flags: needinfo?(dkeeler)
I think the current behavior is good enough unless/until we see that too many failures are happening due to it.

Otherwise, if we make some change to accept such certificates, we need to be aware of the interaction between this and name constraints. In particular, IP address constraints would have to be applied to IP addresses in dNSName SAN entries.
So a person point me this bug because when trying to access this page https://app.siscad.cadivi.gob.ve/ I get the same error on nightly and on Firefox 37 I can't add the security exception.
(In reply to Leonard Camacho [:lcamacho] from comment #5)
> So a person point me this bug because when trying to access this page
> https://app.siscad.cadivi.gob.ve/ I get the same error on nightly and on
> Firefox 37 I can't add the security exception.

pp -a -t certificate -i <Path to cert of app.siscad.cadivi.gob.ve in PEM format>
> Name: Certificate Subject Alt Name
> Other Name: "RIF-V-15379005"
>     OID: OID.2.16.862.2.3
> IP Address: 190.202.84.148
> DNS name: "200.44.32.10"
> DNS name: "200.11.248.11"
> Other Name: "servidorOthers"
>     OID: OID.2.16.862.2.4
> Other Name: "http://ar.fii.gob.ve"
>     OID: OID.2.16.862.2.5
> Other Name: "http://www.fii.gob.ve"
>     OID: OID.2.16.862.2.6

Indeed, looks like the same issue.
Summary: Secure connection failed (sec_error_bad_der) → Secure connection failed (sec_error_bad_der) due to certs with SAN dNSName entries incorrectly containing IP addresses
i'm seeing similar issues on Nightly 40.0a1 (2015-04-07)
i have multiple devices that (server IMM modules) that are no longer accessible as of the latest update.
"An error occurred during a connection to host-imm. security library: improperly formatted DER-encoded message. (Error code: sec_error_bad_der)"
when checking the certificate generated by the IMM, i see following:

> openssl x509 -in ibm.pem -text -noout
> Certificate:
>     Data:
>         Version: 3 (0x2)
>         Serial Number: 203 (0xcb)
>     Signature Algorithm: sha1WithRSAEncryption
>         Issuer: CN=CA for 5AE62099-1A2C-42D6-8B3E-997643655555, 12-12-07 03:43:55, O=Generated by IBM Firmware, C=US, ST=TX, L=Austin
>         Validity
>             Not Before: Jan  1 00:01:00 1950 GMT
>             Not After : Dec 31 23:59:59 2048 GMT
>         Subject: CN=host-imm.management.local, O=Generated by IBM Firmware, C=US, ST=TX, L=Austin
>         Subject Public Key Info:
>             Public Key Algorithm: rsaEncryption
>                 Public-Key: (2048 bit)
>                 Modulus:
>                     .
>                     .
>                     .
>                 Exponent: 65537 (0x10001)
>         X509v3 extensions:
>             X509v3 Basic Constraints:
>                 CA:FALSE
>             X509v3 Key Usage: critical
>                 Digital Signature, Key Encipherment
>             X509v3 Extended Key Usage:
>                 TLS Web Client Authentication, TLS Web Server Authentication
>             X509v3 Subject Alternative Name:
>                 IP Address:FE80:0:0:0:3640:CCFF:FECF:BBBB, DNS:fe80:0000:0000:0000:3640:CCFF:FECF:BBBB, IP Address:FE80:0:0:0:3640:CCFF:FECF:BBBA, DNS:fe80:0000:0000:0000:3640:CCFF:FECF:BBBA, IP Address:10.2.2.13, DNS:10.2.2.13, IP Address:192.19.19.21, DNS:192.19.19.21, DNS:host-imm, DNS:host-imm.management.local
>     Signature Algorithm: sha1WithRSAEncryption
>          .
>          .
>          .

It looks like it is being caused by same problem where IP addresses are listed in the DNS fields. 

Is there a way to whitelist sites with these types Subject Alternative Name entries either globally (always allow) or manually (add each site separately)?  there is no "add exception" button on the "Secure Connection Failed" error page that would allow overwrite.
[Tracking Requested - why for this release]: Compatibility regression. It may be one that we don't want to fix, but release drivers and PMs should be aware of it.
status-firefox37: --- → affected
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox40: --- → affected
status-firefox-esr31: --- → unaffected
tracking-firefox37: --- → ?
tracking-firefox39: --- → ?
tracking-firefox40: --- → ?
Keywords: regression, site-compat
I don't think this is going to result in a point release for 37 so wontfixing for 37. 
Brian, how far back does this affect Firefox?   Why would we not want to fix it, and who would decide? Brian would that be you or maybe David?
tracking-firefox37: ? → +
tracking-firefox38: ? → +
tracking-firefox39: ? → +
tracking-firefox40: ? → +
Flags: needinfo?(dkeeler)
Flags: needinfo?(brian)
Hello, thank you for your work. 
I have a comment just in case, this might be related (or duplicated) on BUG 1138273.  

We fixed this problem in our secured website by reissuing our certificate moving our secured IP to the last position in our SANs list. By doing it we stop receiving this message.
(In reply to Liz Henry (:lizzard) from comment #9)
> Brian, how far back does this affect Firefox?

Probably new in Firefox 37.

> Why would we not want to fix it

* The certs are non-compliant with the specification that defines how certificates work. Firefox is complying with the spec. This is more about whether we want to implement a workaround to help non-compliant certificates keep working. The more such workarounds we implement, the more likely it is we do something that makes Firefox less safe.

* Not many sites seem to be affected by this. Most websites don't have any need for IP addresses in their certificates.

* There are simple workarounds that the site owner can use to fix the problem.

> and who would decide? Brian would that be you or maybe David?

Ultimately, David. 

Based on Aureliano Pato Gozzi's comment and other comments I've read, it seems we strictly check the syntax of every dNSName entry until we find a matching one, and then avoid checking the rest. Thus, the order of the invalid dNSName entries relative to other entries matters. It seems wrong to me that the order matters. If it is important that we reject invalid dNSName entries, then we should check the syntax of all of them. Conversely, if it is safe to have invalid dNSName entries at the end, then it should be OK to have invalid dNSName entries everywhere.
Flags: needinfo?(brian)
Brian, would it be sensible to just skip malformed DNSName SAN entries for the purposes of CheckCertHostname but still fail with ERROR_BAD_DER for name constraints? I think that would address the immediate compatibility concerns without compromising security too much (particularly since we currently effectively skip malformed entries if they appear after a match).
Flags: needinfo?(dkeeler) → needinfo?(brian)
(In reply to David Keeler [:keeler] (use needinfo?) from comment #13)
> Brian, would it be sensible to just skip malformed DNSName SAN entries for
> the purposes of CheckCertHostname but still fail with ERROR_BAD_DER for name
> constraints? I think that would address the immediate compatibility concerns
> without compromising security too much (particularly since we currently
> effectively skip malformed entries if they appear after a match).

I agree that is probably the best course of action. [1] indicates that Windows doesn't support iPAddress subjectAltName and Microsoft recommends (not just on that page, but elsewhere) that dNSName be used for IP addresses instead.

Ideally, we'd return the SEC_ERROR_CERT_NOT_IN_NAME_SPACE error instead of SEC_ERROR_BAD_DER during name constraint checking, to help people understand why things went wrong when they go wrong, but that can be done in a follow-up bug.

[1] https://social.technet.microsoft.com/Forums/ie/en-US/0f6e6195-b98a-40ba-9f85-5686b5ad2f3f/ip-address-as-subjectaltname-does-not-work-with-ie8-but-works-with-firefox?forum=ieitprocurrentver
Flags: needinfo?(brian)
Should we take this up with Microsoft? This seems to suck, rather...

Gerv
(In reply to Gervase Markham [:gerv] from comment #15)
> Should we take this up with Microsoft? This seems to suck, rather...
> 
> Gerv

That would be appreciated. Do you have contacts there?
Attached patch patch (obsolete) — — Splinter Review 
What do you think of this approach? I'm opting to defer handling reporting name-constraints-specific errors for now.
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #8591060 - Flags: review?(brian)
Comment on attachment 8591060 [details] [diff] [review]
patch

Review of attachment 8591060 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/pkix/lib/pkixnames.cpp
@@ +364,5 @@
>                                             match);
>        if (rv != Success) {
> +        // Skip over invalid DNS name entries when attempting to match a DNS
> +        // name. Some real-world certificates have IP addresses in DNS name
> +        // entries. We should be able to safely ignore them when doing name

s/We should be able to safely/It is safe to/

Should we do this in MatchPresentedDNSIDWithReferenceDNSID instead? It seems safer to limit the scope to that function.

It's worth calling out in the comment that we will reject the invalid encoding during name constraint matching and that we don't allow invalid dNSName entries in name constraints.
Maybe my problem is related? I'm running a TLS server on my MacBook Pro for local development and am getting the error "Secure Connection Failed - An error occurred during a connection to localhost. security library: improperly formatted DER-encoded message. (Error code: sec_error_bad_der)" whenever I try to access my https://localhost sites. I've never seen this error until I upgraded to Firefox 37. My sites work fine under TLS in Firefox 36 and earlier as well as Chrome. If the certficates have worked fine before, why do they no longer work for Firefox 37 now?

For my own purposes on my local development machine, the CA is self-signed, and was used to sign the server certificate. (There's also a client certificate for added security, but that doesn't seem to be the problem.)

It would be useful to know what exactly is the problem so it can be troubleshooted.
Interestingly, I don't get this error when I type in an IP address on my local network (10.0.1.7), or the IP address of the router that can then packet route to my local machine. It seems to be a problem only using localhost. Again, this seems only to be a problem with Firefox 37 (and 37.0.1). Any ideas how to fix? I can post the openssl x509 text data of the server and CA pems if that would be useful.

Just trying to figure out how to fix this and learn.
Hi Jason, it seems like this could be related, yes. I'll have a look if you post the certificate you're using.
Ok, here you go.

openssl x509 -text -in serverCert.pem -noout produces this:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 17355664335659931253 (0xf0dbb4c24fed1675)
    Signature Algorithm: sha512WithRSAEncryption
        Issuer: O=CMacCA/emailAddress=jasoncillo@comcast.net, L=Henrico, ST=Virginia, C=US, CN=localhost root CA
        Validity
            Not Before: May 14 04:29:51 2014 GMT
            Not After : May 14 04:29:51 2015 GMT
        Subject: C=US, ST=Virginia, O=CMacCA, CN=localhost
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:b7:60:7f:d9:c4:f7:c2:fb:f3:d0:ba:8d:8a:af:
                    65:36:04:fa:ce:b2:09:02:c7:ce:30:87:26:39:ed:
                    8d:a1:d8:f6:27:2e:e5:6f:71:7b:7b:ac:87:7a:79:
                    55:7e:a2:1f:e2:cf:de:e6:d7:1c:62:8c:f8:69:2b:
                    f5:98:ec:23:8e:6d:75:6a:36:03:7d:c2:4f:3c:6c:
                    6d:ec:45:c6:1d:30:8d:46:d0:74:11:3e:2c:57:89:
                    89:27:d9:e0:3e:b6:66:64:20:14:28:01:03:6a:77:
                    bb:19:e8:6c:37:b2:53:57:3d:8c:6c:6f:22:8e:7c:
                    5b:8b:5e:2b:92:50:fe:4d:36:b1:4a:ba:28:d5:3f:
                    ab:43:fe:e1:0d:9e:ec:6b:fb:59:9b:8d:aa:0a:84:
                    ec:6c:50:7a:ee:7c:1b:ae:df:28:08:61:83:f5:09:
                    47:f6:4d:fe:d7:3c:98:d4:3f:65:16:53:33:26:ff:
                    d2:df:7b:bc:f8:d6:0e:be:19:47:67:c8:56:d7:66:
                    1c:55:aa:07:3d:8c:49:3e:f4:8a:09:af:8a:65:d1:
                    66:4a:bf:37:61:53:92:5f:4c:04:fe:34:ba:1b:aa:
                    e6:f4:64:85:c8:1a:ce:ae:02:2f:48:94:9a:e8:2f:
                    66:86:31:ac:62:92:1d:42:be:d4:68:ab:a9:16:e4:
                    55:bd
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Alternative Name: 
                DNS:76.123.11.144, DNS:173.15.206.101
    Signature Algorithm: sha512WithRSAEncryption
         5b:25:8d:72:36:b0:56:72:b2:02:1d:ec:a3:bf:7d:80:05:bb:
         1f:b0:6a:49:cc:93:6b:aa:1d:9d:65:f7:78:eb:83:35:a4:05:
         18:53:28:0e:d5:71:6d:28:e1:5e:d1:5c:ac:ab:d2:b0:a1:cb:
         76:1d:65:79:8b:d4:a3:cc:0c:6a:27:d0:7c:60:3c:04:1b:ff:
         63:83:c5:7b:e7:5e:76:cf:e2:24:43:a7:a8:7b:f0:2b:ee:a5:
         be:bf:1d:3b:95:21:b0:5a:82:de:d2:55:d2:12:50:3c:29:f8:
         35:bc:31:74:0c:19:f7:06:ff:4c:6b:b9:80:5f:66:e4:24:79:
         e7:7e:0b:10:dd:70:0c:7a:0d:b2:71:ed:75:95:60:f2:6b:b0:
         95:95:0d:de:2f:69:cf:40:6f:fc:6a:4d:81:61:20:d7:53:af:
         27:18:16:54:9d:74:3b:3c:7a:b6:e8:25:37:0f:ee:30:d1:58:
         79:ce:86:e9:19:50:eb:3b:8d:4c:f8:51:42:4e:14:a8:03:c7:
         59:0e:33:1e:39:a1:ee:38:a5:87:82:43:f3:b1:f5:41:9f:f3:
         01:75:08:45:a7:86:35:6a:59:3d:91:63:4b:ef:99:62:64:c3:
         60:11:60:de:de:ac:d0:5c:b6:92:91:a3:67:55:c3:78:ed:e7:
         39:19:7a:92:69:a7:6c:1e:6e:5c:ab:35:fb:7d:ba:8e:4c:8e:
         b3:60:d7:90:7b:ea:68:85:3a:ba:7f:f3:1a:dd:f2:da:ad:23:
         8f:1e:65:77:65:1f:31:48:c5:47:c8:08:f9:a2:96:6f:46:8c:
         e7:76:68:07:2a:49:bb:ab:52:ea:ab:89:09:0e:cb:d7:df:1d:
         f4:9c:e4:38:9f:69:dd:c9:7d:92:36:74:a4:0c:da:8c:d9:20:
         46:8e:08:c5:14:c9:c0:66:1b:ad:96:68:2f:28:45:22:95:74:
         1d:c6:0f:b4:dc:d1:b4:6d:31:f1:8d:aa:89:bf:7f:85:3b:a8:
         7c:ad:ef:92:a4:a0:35:e3:5d:16:5e:65:76:ca:75:07:32:ec:
         c2:8a:78:d1:0b:ed:01:d1:cd:e4:66:ac:2f:24:a3:eb:c7:7f:
         1d:a2:9c:d9:eb:52:11:0e:33:a9:f2:9e:35:66:ab:a3:5f:38:
         04:24:0b:45:96:01:0b:84:18:90:2f:2d:c8:4f:6c:13:f8:f8:
         36:e1:77:50:5b:a1:f3:e3:52:a4:5e:df:15:33:4a:55:78:09:
         e0:e8:0e:c9:f4:bf:fc:87:a6:5a:de:a6:b7:b0:29:ec:0f:47:
         a9:33:2f:0a:cf:3f:f9:65:b2:a9:c3:44:af:86:6b:88:5a:70:
         f9:a0:a8:8a:14:19:66:a8


openssl x509 -text -in cacert.pem -noout produces this:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 14752242707957963272 (0xccba7e95082e5a08)
    Signature Algorithm: sha512WithRSAEncryption
        Issuer: O=CMacCA/emailAddress=jasoncillo@comcast.net, L=Henrico, ST=Virginia, C=US, CN=localhost root CA
        Validity
            Not Before: May 14 03:15:49 2014 GMT
            Not After : May 11 03:15:49 2024 GMT
        Subject: O=CMacCA/emailAddress=jasoncillo@comcast.net, L=Henrico, ST=Virginia, C=US, CN=localhost root CA
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (4096 bit)
                Modulus:
                    00:dd:0d:ad:fb:e3:fd:46:f9:8b:6f:18:90:93:5f:
                    09:27:e7:e9:f1:20:ea:ca:b5:8a:ef:c4:92:31:40:
                    27:e4:45:f5:a5:47:94:8f:8f:35:7e:94:59:7d:73:
                    55:45:8d:cf:9f:49:fe:8b:ac:49:ed:00:48:34:20:
                    bb:9e:d6:00:ed:a1:81:8c:07:eb:d6:27:7b:7f:34:
                    10:5d:2a:e2:06:81:4b:f7:89:0d:7c:d8:ad:c3:cc:
                    85:84:84:6b:11:d5:9f:77:f9:e9:e5:ab:ba:25:e6:
                    3e:c4:aa:c2:be:11:3a:f2:df:8e:00:26:f2:44:44:
                    09:f6:85:a0:8a:97:0a:c3:c6:76:0c:4b:aa:bb:70:
                    cb:61:fd:43:c9:3d:21:60:fa:b1:b6:33:27:45:c8:
                    b6:37:93:ce:fc:18:e7:6d:b0:e0:1a:b2:b6:f6:10:
                    7f:fe:52:3a:33:e3:a3:ed:7a:1f:da:4f:a9:99:7e:
                    4f:65:25:a9:91:1c:e7:20:82:e3:8b:8f:1d:57:12:
                    55:60:22:00:b3:6c:b5:e4:2a:20:7d:3a:0a:7d:c3:
                    7a:3f:71:ab:1e:b2:bf:81:84:6d:a5:39:15:4c:f9:
                    f3:98:cf:97:83:c6:e2:33:29:70:14:61:6f:dc:0d:
                    84:4e:e3:d1:72:78:b3:48:c8:f1:b1:f2:32:db:91:
                    ce:21:71:57:fe:63:0f:2b:96:6f:c6:bb:bf:20:b7:
                    9c:42:2f:da:38:da:4d:b2:67:3f:3c:ca:88:73:53:
                    ce:1e:ca:5c:13:bb:23:fb:d9:9f:82:72:b4:6f:a3:
                    46:ca:ae:1c:9d:9f:e2:31:74:fe:dd:ad:15:c7:47:
                    ff:cd:d0:7f:c3:a0:a7:21:2d:2c:59:f7:27:62:fd:
                    b6:63:01:6d:ab:46:a6:dc:ed:ad:97:d1:f3:6e:95:
                    b0:c2:f2:59:25:55:2b:36:8b:ab:59:7a:74:df:cb:
                    9b:82:92:0b:3f:bf:a6:97:d1:6e:ac:e2:65:22:27:
                    cb:23:da:9d:66:92:9a:73:2c:8a:56:3c:82:f9:fa:
                    fb:8b:3c:79:af:2c:86:7f:97:24:dc:77:ca:9a:9c:
                    a2:c0:4f:fe:3e:4f:d3:09:5a:76:c5:45:51:d5:3e:
                    a5:15:e0:ab:da:4b:56:3a:83:b4:48:35:5a:2b:9f:
                    65:ee:ed:9a:07:aa:2b:c9:b7:29:a1:8d:5a:0d:5f:
                    4d:6c:fc:57:f6:e0:45:41:03:cf:fd:e5:6f:22:66:
                    a7:02:e2:ee:65:37:d5:dc:07:35:cd:2b:e7:30:45:
                    04:1f:3d:e3:b0:03:13:6e:0d:e4:8e:52:bb:23:f4:
                    73:e8:8c:bc:17:56:1b:6b:c9:aa:6c:61:22:8a:3c:
                    df:ab:61
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Basic Constraints: 
                CA:TRUE
            X509v3 Subject Key Identifier: 
                E5:5A:DA:84:65:A8:55:A5:9D:9C:F7:D7:A5:F9:2D:DB:CE:E4:70:5C
            X509v3 Authority Key Identifier: 
                keyid:E5:5A:DA:84:65:A8:55:A5:9D:9C:F7:D7:A5:F9:2D:DB:CE:E4:70:5C
                DirName:/O=CMacCA/emailAddress=jasoncillo@comcast.net/L=Henrico/ST=Virginia/C=US/CN=localhost root CA
                serial:CC:BA:7E:95:08:2E:5A:08

    Signature Algorithm: sha512WithRSAEncryption
         57:e4:dd:ae:39:b5:fa:b4:9c:2a:cc:2b:8b:3e:4a:d5:14:45:
         7f:a5:29:9d:8c:85:f0:31:d0:79:c9:7d:56:77:f3:8c:ae:a1:
         86:ff:7b:80:a9:74:14:bc:b4:eb:b8:6e:a2:66:30:fb:64:ce:
         60:5d:87:7e:17:62:71:95:7f:42:d8:d5:f3:8b:32:3f:4f:14:
         7a:ff:10:42:a1:d1:3b:3d:75:fb:46:b1:29:fb:ec:7f:f4:24:
         f6:db:a8:95:4d:da:b4:0a:29:1d:cf:68:d2:da:4a:04:c8:09:
         c9:3d:ed:96:4b:fd:de:7b:38:69:de:3b:98:02:36:1a:6d:69:
         0c:50:23:7c:64:ff:10:79:89:59:48:80:2a:7e:cd:70:5d:cc:
         4e:6f:78:bc:1a:19:b6:2a:48:3f:e2:e3:f1:10:21:0c:b1:b6:
         5c:43:8f:b2:cd:7e:c5:3e:95:24:16:de:ed:45:78:0e:92:f8:
         31:29:6c:87:79:3f:ea:b9:60:96:6e:a8:84:72:c1:15:5c:d2:
         db:08:80:bf:76:3b:df:a7:ab:71:82:67:15:25:0b:05:13:01:
         0f:b3:90:bb:ae:e1:63:aa:9f:24:dc:1f:9f:5c:75:54:5c:cf:
         38:6b:76:78:9b:20:5d:2b:65:88:cc:bd:bb:85:a2:55:b7:af:
         21:39:24:64:ea:d0:0b:28:84:80:05:5f:f5:2d:19:73:49:dc:
         67:54:d2:0f:2e:17:91:7d:50:a9:9b:af:41:a4:d3:67:b3:6b:
         6c:c5:68:3a:e7:61:67:6d:9f:6a:95:22:14:49:9a:4b:ac:15:
         77:1c:73:c7:59:fd:3a:48:6b:f0:8d:08:0a:1e:3e:6c:5c:81:
         ee:57:95:77:7f:20:5f:af:b5:fd:6b:21:7b:57:ae:40:12:52:
         a9:fd:f4:e3:9a:9f:9e:15:b0:54:61:e8:03:5a:b8:a6:dc:3b:
         b5:b0:63:e5:46:cb:b7:8e:ce:bf:d3:91:a7:aa:7e:6b:32:23:
         f1:d5:b2:bb:01:88:1c:2f:e9:ec:35:b6:61:4f:62:a7:13:62:
         36:09:02:ec:b4:c6:fe:03:ae:c3:ca:40:f2:d4:0a:91:1a:99:
         ba:ae:60:11:4c:19:cf:a8:d4:e5:b7:d3:8e:c1:ba:ce:f7:9b:
         2f:f5:a8:59:b5:c5:09:b0:4b:57:7b:31:7d:48:28:7b:67:38:
         c3:79:79:00:a5:25:b7:f8:70:22:40:74:a4:e6:17:f0:89:5a:
         ae:5f:0a:cd:6f:46:99:d9:8a:5c:0d:dc:3f:20:2f:7f:92:b5:
         24:73:93:a5:9f:1b:ac:23:6d:01:8c:c2:8b:39:6f:3d:8c:7c:
         a5:36:09:82:68:f4:7f:ab

Again, this works in earlier Firefox and Chrome, but not in FF37+ for some reason.
Comment on attachment 8591060 [details] [diff] [review]
patch

Review of attachment 8591060 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/pkix/test/gtest/pkixnames_tests.cpp
@@ +1347,5 @@
>                                             IPAddress(ipv4_addr_bytes),
>             Success),
>    // Duplicate DNSName.
>    WITH_SAN("a", RDN(CN("foo")), DNSName("a") + DNSName("a"), Success),
> +  // After an invalid DNSName (which is skipped during name checking).

Please reference this bug here.

@@ +1441,5 @@
>    WITH_SAN("example.org", RDN(CN("foo")),
>             IPAddress(example_com) + DNSName("example.org"), Success),
>  
> +  // We also skip over malformed DNSName because of real-world certificates
> +  // with IP addresses stuffed into DNSName SANs.

Reference the bug number here too, please.

@@ +1645,5 @@
>    Input hostnameInput;
>    ASSERT_EQ(Success, hostnameInput.Init(param.referenceDNSID.data(),
>                                          param.referenceDNSID.length()));
>    Result expectedResult
> +    = param.expectedResult == Success && param.expectedMatches

Better have parentheses around expressions here to avoid any doubt about the precedence of |&&| and |?:|.
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #18)
> Should we do this in MatchPresentedDNSIDWithReferenceDNSID instead?
Flags: needinfo?(dkeeler)
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #24)
> (In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment
> #18)
> > Should we do this in MatchPresentedDNSIDWithReferenceDNSID instead?

Sorry - I meant to respond sooner. I tried reworking the patch that way, but then there isn't a sense of "these two DNS IDs don't match" vs. "one of those wasn't even a valid DNS ID" for that function. Also, since other functions call MatchPresentedDNSIDWithReferenceDNSID, I thought it made more sense to limit this skipping behavior to just where we're looping through the SAN when doing name matching.

I also thought it would be best to limit it to things that looked like IP addresses, since that's the incompatibility we're seeing and I don't think we should be more liberal than necessary.

In any case, I have a better patch that I'll post when it's ready.
Flags: needinfo?(dkeeler)
Attachment #8591060 - Attachment is obsolete: true
Attachment #8591060 - Flags: review?(brian)
(In reply to Jason Cillo from comment #22)
>         X509v3 extensions:
>             X509v3 Subject Alternative Name: 
>                 DNS:76.123.11.144, DNS:173.15.206.101

Yes, this is the issue you're encountering. Since IPv4 addresses aren't valid DNS names, Firefox currently assumes the certificate is malformed and rejects it. If you want to use IP addresses, you should be able to specify IP address entries in the subject alternative name extension (they have a different tag from DNS name entries).
Cool. Took some work, but I changed the certificate so in the SAN field, the IP addresses are incorporated using the IP tag instead of the DNS tag, and now it works fine — i.e., my using "https://localhost" as the address in FF37+ now doesn't generate the above error.

Weird error to report, though, if entering "localhost" doesn't work for this reason, isn't it? Should that be a fatal error?
Attached patch patch v2 (obsolete) — — Splinter Review 
I think this is a better approach.
Attachment #8596169 - Flags: review?(brian)
Comment on attachment 8596169 [details] [diff] [review]
patch v2

Review of attachment 8596169 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/pkix/lib/pkixnames.cpp
@@ +373,5 @@
> +        // as strings in dNSName entries. These are not valid, but for
> +        // compatibility we ignore them when attempting to match a reference
> +        // DNS name. See bug 1148766. Note that we will reject the invalid
> +        // encoding during name constraint matching and that we don't allow
> +        // invalid dNSName entries in name constraints.

Hmmm - I guess I forgot to add a test specifically for this last part.
Too late for 37 & 38. 
If we want to fix this in 38 esr, please let me know.
status-firefox37: affected → wontfix
status-firefox38: affected → wontfix
I emailed my contact at MS today to ask about their support for sAN:IP.

Gerv
My contact tells me: "This issue is fixed in Windows 10."

Gerv
(In reply to Sylvestre Ledru [:sylvestre] -- PTO May 9=>16 from comment #30)
> Too late for 37 & 38. 
> If we want to fix this in 38 esr, please let me know.

Fixing this for ESR would be appreciated.
Gerv, (In reply to Gervase Markham [:gerv] from comment #15)
> This seems to suck, rather...

David, Gerv: Do you still want to add this workaround? It seems like if we can get away without it for two release cycles, we can probably get away with it forever. And, not implementing the workaround would discourage people from depending on it, which would mean that eventually we could remove support for malformed dNSName entries at the *end* of the SAN list too.
Flags: needinfo?(dkeeler)
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #34)
> David, Gerv: Do you still want to add this workaround? It seems like if we
> can get away without it for two release cycles, we can probably get away
> with it forever. And, not implementing the workaround would discourage
> people from depending on it, which would mean that eventually we could
> remove support for malformed dNSName entries at the *end* of the SAN list
> too.

AIUI, currently the only thing that works in Windows < 10 is rejected by Firefox, and the thing that works in Firefox isn't recognised by Windows < 10. This seems to be an unreasonable state of affairs; I suspect we haven't heard much about it because binding certs to IP addresses is relatively rare.

So I'd say that we should still change this so we ignore such errors. That means people have to put both the wrong thing (to work in Windows) and the right thing (to work in Firefox) in their certs, and so the right thing will always be present. Which I think is the best we can achieve.

Is that analysis reasonable?

Gerv
Gerv,

I think your approach is very reasonable as we are having issues with some of our certificate customers. 

Thanks, Bruce.
(In reply to Gervase Markham [:gerv] from comment #35)
> AIUI, currently the only thing that works in Windows < 10 is rejected by
> Firefox, and the thing that works in Firefox isn't recognised by Windows <
> 10. This seems to be an unreasonable state of affairs; I suspect we haven't
> heard much about it because binding certs to IP addresses is relatively rare.
> 
> So I'd say that we should still change this so we ignore such errors. That
> means people have to put both the wrong thing (to work in Windows) and the
> right thing (to work in Firefox) in their certs, and so the right thing will
> always be present. Which I think is the best we can achieve.
> 
> Is that analysis reasonable?
> 
> Gerv

It seems to me that if all of the valid entries appear before all of the invalid entries, both Windows and Firefox will be satisfied without us having to add the workaround (which, as Brian said, would discourage people from depending on it).
That said, there's at least one confirmed duplicate and a few potential duplicates of this bug, so I don't think it's rare enough to ignore.
Flags: needinfo?(dkeeler)
(In reply to David Keeler [:keeler] (use needinfo?) from comment #37)
> It seems to me that if all of the valid entries appear before all of the
> invalid entries, both Windows and Firefox will be satisfied without us
> having to add the workaround (which, as Brian said, would discourage people
> from depending on it).

I'm not sure what you are saying here. As I understand it from comment #0, we currently barf on the invalid entries. So if there are any present at all, we can't connect. Is that right? If so, I'm not sure how that fits with what you just said about "Firefox being satisfied".

If I'm right, then all I'm saying is, change our behaviour to "not barf". We don't have to _recognise_ the invalid entries, just ignore them. I agree that recognising them would be bad; people might then create certificates with _just_ the bad entries, meaning we'd be entrenching this problem. We want people to create certificates with _both_. We achieve that by ignoring the bad ones, and accepting the good ones. Windows < 10 ignores the good and accepts the bad, and Windows 10 (probably) accepts either. 

Gerv
Comment 33 requests for this to be fixed in ESR 38.

Does anyone have an URL of a public site that's still broken,
as a test case?

If this bug prevents users from accessing web sites (or device management interfaces), and without offering an override, then this should be fixed on ESR 38.
status-firefox-esr38: --- → affected
tracking-firefox-esr38: --- → ?
An impacted certificate can be found at this site, https://ums.lpu.in/. I'm not sure if this can be used for testing.

Bruce.
I'm getting this error right now at https://golang.org. Odd that I haven't seen it before. I browse the package docs there just about every day with Firefox.

Chrome renders it fine, and SSLlabs gaves it a B[1].

[1] https://www.ssllabs.com/ssltest/analyze.html?d=golang.org
Tracking. I agree with Kai, we should be able to override this error.
David, can you help ? Thanks
tracking-firefox-esr38: ? → 39+
Flags: needinfo?(dkeeler)
Attached patch patch v3 — — Splinter Review 
This is an updated patch with the tests I didn't add the first time. I also made some changes to make error reporting more consistent with respect to encountering malformed input in the subject common name vs. the subject alternative names extension.

Brian, are you available to review this? If not, I can find someone else. Thanks.
Attachment #8596169 - Attachment is obsolete: true
Attachment #8596169 - Flags: review?(brian)
Flags: needinfo?(dkeeler)
Attachment #8611438 - Flags: review?(brian)
(In reply to cmars from comment #41)
> I'm getting this error right now at https://golang.org. Odd that I haven't
> seen it before. I browse the package docs there just about every day with
> Firefox.

That is:
https://bugzilla.mozilla.org/show_bug.cgi?id=1169149
Also affected is the mobile version of the parcel tracking website of the german postal service DHL.

I'm attaching a dump of the server's certificate.

I get the error when accessing https://mobil.dhl.de
(In reply to Kai Engert (:kaie) from comment #45)
> Created attachment 8613002 [details]
> certdump-mobil.dhl.de.txt

An intermediate (apparently) from Globalsign is involved, which contains an unusual name constraints extension (at least I haven't seen that before):

Issuer: "CN=Trusted Root CA G2,O=GlobalSign nv-sa,OU=Trusted Root,C=BE"
Subject: "CN=DPDHL TLS CA I3,O=Deutsche Post,L=Bonn,ST=Nordrhein-Westfalen,C=DE"

Name: Certificate Name Constraints
IP Address: 00:00:00:00:00:00:00:00
IP Address: 00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
Any progress? David is this something you will want to uplift to 39?
Flags: needinfo?(dkeeler)
This was made much less urgent by bug 1170303. I think we should untrack this bug and instead uplift that one as far as possible.
Flags: needinfo?(dkeeler)
OK. Will do. Leaving this open and still marked "affected" though.
tracking-firefox39: + → -
tracking-firefox40: + → -
:keeler: now we have bug 1170303, is it possible to make a cert which works everywhere by putting the IP address in twice, or will Firefox still complain about it?

Gerv
Flags: needinfo?(dkeeler)
If the duplicate IP addresses encoded as DNS names appear last, it should work as expected. That is, if there is a matching DNS name or (real) IP address, Firefox will find it and accept the certificate. If no matches are found, Firefox will display the "domain name mismatch" error page and allow the user to opt to continue.
Flags: needinfo?(dkeeler)
Comment on attachment 8611438 [details] [diff] [review]
patch v3

I think that since we've had so few complaints about the behavior, it's unlikely that this is negatively affecting many sites.

Google (Ryan Sleevi in particular) seems to be trying to be more strict regarding this, so we should avoid doing anything more to accommodate this Ryan Sleevi pointed out the "correct" way to work around Microsoft's problem: Encode the IP address(es) in CN fields of the subject field, and encode them as iPAddresses in the subjectAltName.

More generally, I'd like to avoid making the code more complicated without a good reason for doing so. In fact, I'd like to start *removing* such complications.
Attachment #8611438 - Flags: review?(brian) → feedback-
yeah, because most people tell others, "try chrome as it's working for me". I don't see any reason to fail without letting user override. Very few people have an idea they can report bugs and most cannot file bugs even if they had the idea. Reason might be computer illiteracy or language barrier. 

It's not about making such certs work without complain. It's about giving users the choice to continue even if certificate has issues. We don't live in ideal world. I just would like to have fewer situations where I'm forced to use chrome or other browsers.
See Also: → 1200577
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #53)
> More generally, I'd like to avoid making the code more complicated without a
> good reason for doing so. In fact, I'd like to start *removing* such
> complications.

Agreed. I'm happy to WONTFIX this.

(In reply to Alexander from comment #54)
> yeah, because most people tell others, "try chrome as it's working for me".
> I don't see any reason to fail without letting user override. Very few
> people have an idea they can report bugs and most cannot file bugs even if
> they had the idea. Reason might be computer illiteracy or language barrier. 
> 
> It's not about making such certs work without complain. It's about giving
> users the choice to continue even if certificate has issues. We don't live
> in ideal world. I just would like to have fewer situations where I'm forced
> to use chrome or other browsers.

Due to bug 1170303, as for Firefox 39 there should be an option to add an override. If you're finding this isn't the case, please file a new bug.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Restrict Comments: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: