153380 - SSL_ENABLE_TLS comments in ssl.h is wrong

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Mark Smith [:mcs]]

I noticed this in ssl.h:

#define SSL_ENABLE_TLS    13 /* enable TLS (off by default) */

But TLS is enabled by default now.

Also, it would be great to include comments in ssl.h for each option that says
whether it is ON or OFF by default. Some include that information and some do not.

Comment 1

[Wan-Teh Chang]

Fixed on the tip, in rev. 13 of ssl.h.

I found that the "factory" default settings of the various
SSL options are not documented in the SSL Reference
(http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslfnc.html).
It is not clear whether we should document the factory
default settings (which means they won't change, at least
not until the next major version upgrade) or we should
state that the factory default settings are implementation
defined and users need to call SSL_OptionGetDefault to get
them.  Nelson, what do you think?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

We should document the factory default settings (which means they 
won't change, at least not until the next major version upgrade).
That is and has been the policy for the default settings of all ssl
options for a long time.  

TLS was off by default in all the 2.x releases (since it was implemented), 
and is on by default beginning in NSS 3.0, which was a major version.
The comment is wrong, and should be fixed.

Comment 3

[Wan-Teh Chang]

Attachment: Proposed comments in ssl.h

How does this look, Nelson and Mark?

Comment 4

[Mark Smith [:mcs]]

Comment on attachment 88698 [details]
Proposed comments in ssl.h

Looks OK to me, but now I want to know what the difference is between 2 and
other values that might be used for SSL_REQUIRE_CERTIFICATE.

Comment 5

[Wan-Teh Chang]

The values that might be used for SSL_REQUIRE_CERTIFICATE are
the topic of bug 135261.  We plan to add a new value (3) and
add symbolic constants for these values.

Until the patch for bug 135261 is checked in, I have to use the
(undocumented) value 2 in the comments in ssl.h.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

The patch for 135261 is now checked in on the trunk.

Comment 7

[Wan-Teh Chang]

Attachment: Proposed comments in ssl.h, v2

Replaced 2 by the symbolic constant SSL_REQUIRE_FIRST_HANDSHAKE.

Nelson, could you add some comments explaining the four possible
values for the SSL_REQUIRE_FIRST_HANDSHAKE option?

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

I agree with the change you've proposed to the comments in ssl.h.

The differences between some of the 4 values for SSL_REQUIRE_FIRST_HANDSHAKE 
are very subtle.  I don't think I could describe them adequately in one or 
two lines of text per option.  Perhaps it is best to add a reference to
the SSL "reference manual" web page on mozilla.org, and clarify the 
definitions of those 4 values in that page.

Comment 9

[Wan-Teh Chang]

Nelson, that makes sense.  Then I'm going to mark
this bug fixed.