Hi Kaspar,

On Saturday 17 of October 2009 08:41:02 Kaspar Brand wrote:
> Günter asked me (off-list) to comment on this patch... so here we go.

thanks for review!

> I haven't looked at the proposed code changes in much detail, but have
> some additional observations/thoughts.

I've just rewritten it anyway.

> Seems like you tripped over yet another category of broken TLS servers:
> those which freak out when they encounter TLS extensions. Both
> www.orange.sk ("Server: Oracle-Application-Server-10g/10.1.3.1.0
> Oracle-HTTP-Server") and tcs.mysap.com (no Server header, running some
> SAP stuff, I guess) aren't really "TLS intolerant", they do speak TLS
> 1.0, actually.

The "TLS intolerant" terminology has been formerly taken from xulrunner.

> However, when encountering a ClientHello with TLS extensions - be that
> SNI or SessionTicket, e.g. -, these servers will immediately return a
> fatal TLS alert, either "illegal_parameter" or "unexpected_message"
> (i.e., the ones you're catching in Curl_nss_connect). Probably needless
> to say that this behavior does not comply with the TLS 1.0 spec -
> already RFC 2246 had a "Forward compatibility note" which allows
> additional data in the ClientHello (and noted that this data "must
> otherwise be ignored" - certainly not being replied to with a fatal
> alert...).

I agree, the servers are broken. But users need to download the content and
not adhering the specification is acceptable for them in that case. I can
reference two Fedora bugs:

https://bugzilla.redhat.com/525496
https://bugzilla.redhat.com/527771

From the two bugs above I gathered the error numbers to precedent patch.

> This doesn't a priori preclude libcurl from implementing a workaround,
> of course course (depending on how frequently such server
> implementations are seen in the wild). Some points to consider:

I have no statistics, but the user's question is always the same: "I can
download https://XXX with Firefox, why it is not possible with curl?"

> - Is downgrading to SSL 3.0 really the best solution? If there's a way
> to only suppress TLS extensions, that would probably be preferrable.

Well, I don't know NSS internals. The only knowledge base for tweaking SSL
I know about is the following documentation:

http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html#1086543

If you find a better solution, I am happy to use it. From my observation
either disabling TLS or enabling V2_HELLO was successful for those two URLs.
But I wanted to avoid reinventing a wheel, thus took the working solution
from xulrunner as-is.

> - The fix for these particular server implementations should make it
> clear that it's dealing with "broken TLS servers" (or maybe
> "TLS-extension intolerant servers"). As mentioned above, those servers
> will "happily" negotiate TLS 1.0 if no extensions are sent in the
> ClientHello.

Fair enough. I've just renamed the variables and verbose messages to
use "broken TLS severs".

> - Mozilla's PSM code has 15 NSS error codes which possibly indicate a
> TLS intolerance - you might want to add some of them to your list. See
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/ssl/sr
>c/nsNSSIOLayer.cpp&rev=1.165&mark=1594-1608#1578

Thanks for the hint! I took the whole list from xulrunner. I am aware I have
no simple test case for each of the error codes. But if there is a bug in the
list, the bug affects also Firefox. It makes me calm a bit.

> - libcurl versions compiled against OpenSSL or GnuTLS will most likely
> suffer from the same problem, so maybe an implementation with fallback
> to "extension-less" TLS (or even SSL 3.0) would better go into
> lib/sslgen.c, not into lib/nss.c only?

The variables in lib/urldata.h can be shared by others implementation, the
hook for reconnect after SSL error as well. Unfortunately the SSL error code
resolving and then SSL option tweaking is per-library specific. That code
has to stay within lib/nss.c.

> - The "ssl_connect_retry" state variable, how exactly does it behave in
> a long-living process (such as a daemon, e.g.)? If a particular URL has
> been marked as "intolerant", would it stay so forever? I'm asking this
> because PSM has some issues with this specific scenario, see e.g.
> https://bugzilla.mozilla.org/show_bug.cgi?id=239381 (and the bugs
> mentioned in comment 12, too). Some of them have recently been addressed.

IMO this bug does not affect libcurl, but good point anyway. It makes sense to
cache the "broken TLS server" detection result among different connections on
the same handle. I've added a new state variable to struct UrlState to
achieve this.

Let's go for the second iteration :-)

Kamil

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html