If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

SSL_ENABLE_TLS comments in ssl.h is wrong



16 years ago
15 years ago


(Reporter: mcs, Assigned: Wan-Teh Chang)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



16 years ago
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

16 years ago
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
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?
Priority: -- → P1
Target Milestone: --- → 3.6
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

16 years ago
Created attachment 88698 [details]
Proposed comments in ssl.h

How does this look, Nelson and Mark?

Comment 4

16 years ago
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

16 years ago
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.
The patch for 135261 is now checked in on the trunk.
Depends on: 135261

Comment 7

15 years ago
Created attachment 99740 [details] [diff] [review]
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?
Attachment #88698 - Attachment is obsolete: true
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

15 years ago
Nelson, that makes sense.  Then I'm going to mark
this bug fixed.
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.