Elio Maldonado Reporter
	Description • 9 years ago

As reported by Kamil on Fedora:

 Kamil Dudka 2014-06-04 06:04:31 EDT

Description of problem:
NSS re-uses a session cache entry despite the server name does not match, which causes SNI host name to differ from the actual host name.  Consequently, certain servers (as e.g. github.com) respond by 400 to such requests.

Version-Release number of selected component (if applicable):
nss-3.16.1-2.fc21

Steps to Reproduce:
1. curl -fL https://raw.github.com/teeedubb/teeedubb-xbmc-repo/master/addons.xml.md5

Actual results:
curl: (22) The requested URL returned error: 400 Bad Request

Expected results:
97c2bb53b388ad03329e273951407511

Additional info:
See bug 1098711 for details. 

NOTE: that bug is now closed 
https://bugzilla.redhat.com/show_bug.cgi?id=1098711

	Elio Maldonado Reporter
	Comment 1 • 9 years ago

additional info from: 

Nathan Salter 2014-09-30 04:00:15 EDT

I can confirm that this is also happening with libcurl 7.19.7 using NSS 3.15.3. 

When switching connection between two different hosts which share the same certificate (and probably IP), it will reuse the TLS handshake from the previous request which causes many server applications (such as Apache) to return a 400 error.

Interestingly enough it will continue to use the incorrect handshake even when the server returns non-200 responses. Tested using openssl fixes the issue with definitely implies that there's a problem with the NSS lib.

Example (PHP) code to replicate the issue: http://pastebin.com/K7JhCLsz

	Elio Maldonado Reporter
	Comment 2 • 9 years ago

See Kamil's patch at https://bugzilla.redhat.com/attachment.cgi?id=902122&action=diff
which I'll shortly submit it suitably adapted to the current sources.

	Ryan Sleevi
	Comment 3 • 9 years ago

(In reply to Elio Maldonado from comment #2)
> See Kamil's patch at
> https://bugzilla.redhat.com/attachment.cgi?id=902122&action=diff
> which I'll shortly submit it suitably adapted to the current sources.

I think it would be very preferable that you don't submit it (unless you just meant attaching it).

I don't believe that patch results in desirable behaviour, so it may be worth discussing more. It is also worth noting that the same root behaviour is expressly supported in HTTP/2, so any issues being caused likely transcend just this case.

To me it seems that these servers are misconfigured, and would rather see those fixed.

	Elio Maldonado Reporter
	Comment 4 • 9 years ago

(In reply to Ryan Sleevi from comment #3)
> (In reply to Elio Maldonado from comment #2)
> > See Kamil's patch at
> > https://bugzilla.redhat.com/attachment.cgi?id=902122&action=diff
> > which I'll shortly submit it suitably adapted to the current sources.
> 
> I think it would be very preferable that you don't submit it (unless you
> just meant attaching it).
> 
That's what meant, attach for review here, we never push to git without it, added Kamil and others to the CC list. Thanks for the prompt response.

	Elio Maldonado Reporter
	Comment 5 • 9 years ago

Kamil's patch adapted to current state of the sources. Let's start with Bob as the initial reviewer. This of course needs an additional reviewer that's not from Red Hat.

	Ryan Sleevi
	Comment 6 • 9 years ago

I'd be curious why neither Firefox nor Chromium, both which use(d) this code, did not ever encounter a server with such an issue.

In HTTP/2, this is detailed somewhat in Section 9.1.1 (see https://tools.ietf.org/html/rfc7540#section-9.1.1 ), but that's specifically in the context of HTTP/2 connection reuse.

I'm quite skeptical of the root cause analysis, as it may have been some second-order issue. But, for example, a server that ensures that the SNI presented in the TLS handshake matches the HTTP 'Host' header is already in for a bad time, and that's the only way I can conceive of this issue arising as presented.

	Ryan Sleevi
	Comment 7 • 9 years ago

(In reply to Ryan Sleevi from comment #6)
> I'd be curious why neither Firefox nor Chromium, both which use(d) this
> code, did not ever encounter a server with such an issue.

Answer: Both Firefox and Chromium use SSL_SetSockPeerID, which causes the 'proxy (peerID) matches' check to fail.

https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/ssl_client_socket_nss.cc&l=2936
https://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsNSSIOLayer.cpp?mark=2544-2549&offset=300#2544

So despite setting values for the urlSvrName (via SSL_SetURL), the SSL_SetSockPeerID preempts and prevents sessions from being pooled across hostnames.

	Kamil Dudka
	Comment 8 • 9 years ago

Thank you for digging this!  Are you suggesting to use SSL_SetSockPeerID() as a workaround for libcurl?

	Ryan Sleevi
	Comment 9 • 9 years ago

Yes, that will work around the issue for libcurl, although it's a reasonable request to consider changing the NSS behaviour, since it's clear it's not tested by either Firefox or Chrome and thus may cause issues (despite it being a perfectly valid behaviour)

	Kamil Dudka
	Comment 10 • 9 years ago

I have applied the following patch on curl to work around the issue:

https://github.com/bagder/curl/commit/958d2ffb

	Daniel Stenberg [:bagder]
	Comment 11 • 9 years ago

I officially love it when my worlds collide and I can read about curl fixes in bugzilla. Thanks for your work there Kamil!

	Kai Engert (:KaiE:)
	Comment 12 • 8 years ago

https://hg.mozilla.org/projects/nss/rev/bf8a54155531