Closed Bug 527240 Opened 15 years ago Closed 15 years ago

Expose Support for SSL & TLS Renegotiation settings in JSS

Categories

(JSS Graveyard :: Library, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glenbeasley, Assigned: glenbeasley)

References

Details

(Whiteboard: [sg:high])

Attachments

(3 files, 1 obsolete file)

Expose the NSS SSL/TLS configurations options to enable the mode of renegotiation that the peer must use, and enable/disable RequireSafeNegotiation
Attached patch patch v1 (obsolete) — Splinter Review
this patch expose the configuration option for SSL_ENABLE_RENEGOTIATION and 
SSL_REQUIRE_SAFE_NEGOTIATION. 

SSL_RENEGOTIATE_REQUIRES_XTN is not fully implemented in NSS. The patch provide no exception handling for setting an un-implemented option. 

This patch also updates JSS versions of SSLerrs.h and SECerrs.h.

The basic testing performed was simply to check if the options are set correctly.
Attachment #410985 - Flags: review?(nelson)
Attached file test output for patch
Comment on attachment 410985 [details] [diff] [review]
patch v1

I suggest not exposing enableRequireSafeNegotiation until
that option has been implemented in NSS.
Comment on attachment 410985 [details] [diff] [review]
patch v1

I think I concur with Wan-Teh, when he wrote:
> I suggest not exposing enableRequireSafeNegotiation until
> that option has been implemented in NSS.
But otherwise, r=nelson
Attachment #410985 - Flags: review?(nelson) → review+
thanks for the review nelson. the patch checked in only provides support 
for enableRenegotiation which can be set to the following modes

*  @param mode One of:
*      SSLSocket.SSL_RENEGOTIATE_NEVER - Never renegotiate at all.
*
*      SSLSocket.SSL_RENEGOTIATE_UNRESTRICTED - Renegotiate without
*      restriction, whether or not the peer's client hello bears the
*      renegotiation info extension (like we always did in the past).
*
*      SSLSocket.SSL_RENEGOTIATE_REQUIRES_XTN - NOT YET IMPLEMENTED


Checking in org/mozilla/jss/ssl/SSLServerSocket.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.java,v  <--  SSLServerSocket.java
new revision: 1.26; previous revision: 1.25
done
Checking in org/mozilla/jss/ssl/SSLSocket.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLSocket.java,v  <--  SSLSocket.java
new revision: 1.31; previous revision: 1.30
done
Checking in org/mozilla/jss/ssl/SocketBase.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SocketBase.java,v  <--  SocketBase.java
new revision: 1.17; previous revision: 1.16
done
Checking in org/mozilla/jss/ssl/common.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/common.c,v  <--  common.c
new revision: 1.30; previous revision: 1.29
done
Checking in org/mozilla/jss/util/SECerrs.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/SECerrs.h,v  <--  SECerrs.h
new revision: 1.5; previous revision: 1.4
done
Checking in org/mozilla/jss/util/SSLerrs.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/SSLerrs.h,v  <--  SSLerrs.h
new revision: 1.5; previous revision: 1.4
done
Attachment #410985 - Attachment is obsolete: true
Target Milestone: --- → 4.3.1
Should this bug be RESOLVED FIXED now, or is there something left to do?
Whiteboard: [sg:high]
Comment on attachment 411076 [details] [diff] [review]
patch v2 as checked in

>+    static final int SSL_RENEGOTIATE_NEVER = 23;
>+    static final int SSL_RENEGOTIATE_UNRESTRICTED = 24;
>+    static final int SSL_RENEGOTIATE_REQUIRES_XTN = 25;

Glen, is it possible to define these three constants as 1, 2, and 3
to match their values in NSS?  These three constants are in their
own space, so they can have the same values as the other constants.
(In reply to comment #7)
> (From update of attachment 411076 [details] [diff] [review])
> >+    static final int SSL_RENEGOTIATE_NEVER = 23;
> >+    static final int SSL_RENEGOTIATE_UNRESTRICTED = 24;
> >+    static final int SSL_RENEGOTIATE_REQUIRES_XTN = 25;
> 
> Glen, is it possible to define these three constants as 1, 2, and 3
> to match their values in NSS?  These three constants are in their
> own space, so they can have the same values as the other constants.

The above constants defined in SocketBase.java represent their enum value in the "JSSL_enums" table defined in common.c. This is safer than copying the values of the NSS C constants, which are subject to change, into Java code. 

http://mxr.mozilla.org/security/source/security/jss/org/mozilla/jss/ssl/common.c#368
now that bug 526689 is open, this bug as well can be public. 

I am unable remove the Security-Sensitive Core Bug flag. 

This bug can also be closed once bug 526689 is closed. I will open a 
new bug to expose enableRequireSafeNegotiation when it is fully implemented 
in NSS.
Glen:

>+    static final int SSL_RENEGOTIATE_NEVER = 23;
>+    static final int SSL_RENEGOTIATE_UNRESTRICTED = 24;
>+    static final int SSL_RENEGOTIATE_REQUIRES_XTN = 25;
>+    static final int SSL_ENABLE_RENEGOTIATION = 26;

I'd like to suggest two changes to these constants:

1. List SSL_ENABLE_RENEGOTIATION first.  This is just a cosmetic
change.

2. Add a reserved value at the end.  This allows you to do
a simple range check for the validity of a SSL_RENEGOTIATE_xxx
value if NSS ever adds a fourth SSL_RENEGOTIATE_xxx value.
NSS uses two bits to represent SSL_RENEGOTIATE_xxx values,
so there could be up to 4 values.

The corresponding entry in JSSL_enums[] in common.c should
have 3 (the unused SSL_RENEGOTIATE_xxx value):
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/ssl.h&rev=1.30&mark=173,176,179#171

We can release the reserved value when NSS has implemented
the renegotiation info TLS extension and no new
SSL_RENEGOTIATE_xxx value is added.
Attachment #411687 - Flags: review?(glen.beasley)
Comment on attachment 411687 [details] [diff] [review]
Reorder the constants and reserve one value


org.mozilla.jss.ssl.SocketBase.java is not public;  The class cannot be accessed from classes outside org.mozilla.jss.ssl package
SSLSocket and SSLServerSocket use private instances of SocketBase

the JSS jni library and JSS jar need to match each release and therefore a JSS developer if
needed can choose to modify the existing order of the JSSL_enums table in common.c
and SocketBase.java. 

while I will r+ this patch because it is cleaner;  I only want to do so if NSS 3.12.5 is forced to respin thus allowing
JSS 4.3.1 to respin. 

It appears that bug 517615 may cause NSS 3.12.5 to respin.
Glen: we have to respin NSS 3.12.5 because the version string
in mozilla/security/nss/lib/nss/nss.h still says 3.12.5.0 Beta.
Could you check in my patch for me?  Thanks.
We will have to respin NSS 3.12.5 because a patch for a SECOID bug is 
causing new crashes.  We must back that out and respin. :(
Comment on attachment 411687 [details] [diff] [review]
Reorder the constants and reserve one value

Checking in SocketBase.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SocketBase.java,v  <--  SocketBase.java
new revision: 1.18; previous revision: 1.17
done
Checking in common.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/common.c,v  <--  common.c
new revision: 1.31; previous revision: 1.30
done
Attachment #411687 - Flags: review?(glen.beasley) → review+
Group: core-security
This bug exposed the NSS 3.12.5 functionality to JSS 4.3.1 to enableRenegotiation which can be set to the following modes

*  @param mode One of:
*      SSLSocket.SSL_RENEGOTIATE_NEVER - Never renegotiate at all.
*
*      SSLSocket.SSL_RENEGOTIATE_UNRESTRICTED - Renegotiate without
*      restriction, whether or not the peer's client hello bears the
*      renegotiation info extension (like we always did in the past).
*
*      SSLSocket.SSL_RENEGOTIATE_REQUIRES_XTN - NOT YET IMPLEMENTED


Bug 530575 will expose support to enable/disable RequireSafeNegotiation
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: