527240 - Expose Support for SSL & TLS Renegotiation settings in JSS

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect, P1)

Description

[glen beasley]

Expose the NSS SSL/TLS configurations options to enable the mode of renegotiation that the peer must use, and enable/disable RequireSafeNegotiation

Comment 1

[glen beasley]

Attachment: patch v1

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.

Comment 2

[glen beasley]

Attachment: test output for patch

Comment 3

[Wan-Teh Chang]

Comment on attachment 410985 [details] [diff] [review]
patch v1

I suggest not exposing enableRequireSafeNegotiation until
that option has been implemented in NSS.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 5

[glen beasley]

Attachment: patch v2 as checked in

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

Comment 6

[Daniel Veditz [:dveditz]]

Should this bug be RESOLVED FIXED now, or is there something left to do?

Comment 7

[Wan-Teh Chang]

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.

Comment 8

[glen beasley]

(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

Comment 9

[glen beasley]

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.

Comment 10

[Wan-Teh Chang]

Attachment: Reorder the constants and reserve one value

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.

Comment 11

[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.

Comment 12

[Wan-Teh Chang]

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.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

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 14

[glen beasley]

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

Comment 15

[glen beasley]

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