874622 - Undefined behavior when mixing new and OLD SSL version range APIs

Status: RESOLVED INVALID

(NSS :: Libraries, enhancement)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ This bug was initially created as a clone of Bug #733647 +++
+++ This bug was initially created as a clone of Bug #565047 +++

(In reply to Alex Xu from Bug #733647 comment #11)
[snip]
> However, the fallback is not implemented properly; there is undefined
> behavior in NSS due to the simultaneous use of SSL_VersionRangeSetDefault
> and SSL_OptionSet(SSL_ENABLE_TLS).

Alex, could you please explain what you mean by this? We tried to make things sensible when you mix the two APIs to avoid undefined behavior.

Comment 1

[Alex Xu]

Erm... I used undefined behavior not in the C standard sense, but in a vague "bad things might happen" sense.

Moreover, it would appear that since I last read the code, my memory has faded. Here is the relevant portion I was attempting to recall:

nss/lib/ssl/ssl.h:

** Using the new version range API in conjunction with the older
** SSL_OptionSet-based API for controlling the enabled protocol versions may
** cause unexpected results. Going forward, we guarantee only the following:
**
** SSL_OptionGet(SSL_ENABLE_TLS) will return PR_TRUE if *ANY* versions of TLS
** are enabled.
**
** SSL_OptionSet(SSL_ENABLE_TLS, PR_FALSE) will disable *ALL* versions of TLS,
** including TLS 1.0 and later.
**
** The above two properties provide compatibility for applications that use
** SSL_OptionSet to implement the insecure fallback from TLS 1.x to SSL 3.0.
**
** SSL_OptionSet(SSL_ENABLE_TLS, PR_TRUE) will enable TLS 1.0, and may also
** enable some later versions of TLS, if it is necessary to do so in order to
** keep the set of enabled versions contiguous. For example, if TLS 1.2 is
** enabled, then after SSL_OptionSet(SSL_ENABLE_TLS, PR_TRUE), TLS 1.0,
** TLS 1.1, and TLS 1.2 will be enabled, and the call will have no effect on
** whether SSL 3.0 is enabled. If no later versions of TLS are enabled at the
** time SSL_OptionSet(SSL_ENABLE_TLS, PR_TRUE) is called, then no later
** versions of TLS will be enabled by the call.
**
** SSL_OptionSet(SSL_ENABLE_SSL3, PR_FALSE) will disable SSL 3.0, and will not
** change the set of TLS versions that are enabled.
**
** SSL_OptionSet(SSL_ENABLE_SSL3, PR_TRUE) will enable SSL 3.0, and may also
** enable some versions of TLS if TLS 1.1 or later is enabled at the time of
** the call, the same way SSL_OptionSet(SSL_ENABLE_TLS, PR_TRUE) works, in
** order to keep the set of enabled versions contiguous.

So. If I'm right (probably not), the current implementation is fine.

However: the fallback code will disable TLS, both 1.0 and 1.1 if any intolerance is detected which is likely unintended behavior.

It is also possible, IIRC, that the test for false intolerance may only enable TLS 1.0 or some other similar unintended behavior.

It would, however, be sensible to implement bug 839310 comment 2 (re: yngve and assuming TLS tolerance if TLS Extension tolerant) properly.

tl;dr I remembered wrong. False alarm, everybody. My apologies.

Comment 2

[Wan-Teh Chang]

Thank you for the reply.  Marked the bug invalid.