Fix a buffer length bug and nits in the next protocol negotiation (NPN)

functions.

R=agl@chromium.org
BUG=116617
TEST=none

Review URL: http://codereview.chromium.org/9663034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126239 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 98 insertions, 10 deletions

diff --git a/net/third_party/nss/README.chromium b/net/third_party/nss/README.chromium
index 80ffb89..b6435f2 100644
--- a/net/third_party/nss/README.chromium
+++ b/net/third_party/nss/README.chromium
@@ -69,6 +69,11 @@ Patches:
     https://bugzilla.mozilla.org/show_bug.cgi?id=728919
     patches/clang-sslcon.patch
 
+  * Fix a buffer length bug and miscellaneous nits in the next protocol 
+    negotiation (NPN) functions.
+    https://bugzilla.mozilla.org/show_bug.cgi?id=734534
+    patches/nextprotocleanup.patch
+
 Apply the patches to NSS by running the patches/applypatches.sh script.  Read
 the comments at the top of patches/applypatches.sh for instructions.
 
diff --git a/net/third_party/nss/patches/applypatches.sh b/net/third_party/nss/patches/applypatches.sh
index a895782..48cbe52 100755
--- a/net/third_party/nss/patches/applypatches.sh
+++ b/net/third_party/nss/patches/applypatches.sh
@@ -36,3 +36,5 @@ patch -p5 < $patches_dir/clang-sslcon.patch
 patch -p6 < $patches_dir/restartclientauth.patch
 
 patch -p6 < $patches_dir/encryptedclientcerts.patch
+
+patch -p5 < $patches_dir/nextprotocleanup.patch
diff --git a/net/third_party/nss/patches/nextprotocleanup.patch b/net/third_party/nss/patches/nextprotocleanup.patch
new file mode 100644
index 0000000..046b937
--- /dev/null
+++ b/net/third_party/nss/patches/nextprotocleanup.patch
@@ -0,0 +1,83 @@
+Index: mozilla/security/nss/lib/ssl/ssl3ext.c
+===================================================================
+RCS file: /cvsroot/mozilla/security/nss/lib/ssl/ssl3ext.c,v
+retrieving revision 1.21
+diff -u -p -r1.21 ssl3ext.c
+--- mozilla/security/nss/lib/ssl/ssl3ext.c	15 Feb 2012 21:52:08 -0000	1.21
++++ mozilla/security/nss/lib/ssl/ssl3ext.c	10 Mar 2012 00:01:26 -0000
+@@ -592,10 +592,7 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSoc
+     unsigned char resultBuffer[255];
+     SECItem result = { siBuffer, resultBuffer, 0 };
+ 
+-    if (ss->firstHsDone) {
+-	PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID);
+-	return SECFailure;
+-    }
++    PORT_Assert(!ss->firstHsDone);
+ 
+     rv = ssl3_ValidateNextProtoNego(data->data, data->len);
+     if (rv != SECSuccess)
+@@ -607,6 +604,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSoc
+      */
+     PORT_Assert(ss->nextProtoCallback != NULL);
+     if (!ss->nextProtoCallback) {
++	/* XXX Use a better error code. This is an application error, not an
++	 * NSS bug. */
+ 	PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
+ 	return SECFailure;
+     }
+@@ -617,7 +616,7 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSoc
+ 	return rv;
+     /* If the callback wrote more than allowed to |result| it has corrupted our
+      * stack. */
+-    if (result.len > sizeof result) {
++    if (result.len > sizeof resultBuffer) {
+ 	PORT_SetError(SEC_ERROR_OUTPUT_LEN);
+ 	return SECFailure;
+     }
+Index: mozilla/security/nss/lib/ssl/sslsock.c
+===================================================================
+RCS file: /cvsroot/mozilla/security/nss/lib/ssl/sslsock.c,v
+retrieving revision 1.82
+diff -u -p -r1.82 sslsock.c
+--- mozilla/security/nss/lib/ssl/sslsock.c	15 Feb 2012 21:52:08 -0000	1.82
++++ mozilla/security/nss/lib/ssl/sslsock.c	10 Mar 2012 00:01:26 -0000
+@@ -1303,7 +1303,7 @@ SSL_SetNextProtoCallback(PRFileDesc *fd,
+     return SECSuccess;
+ }
+ 
+-/* NextProtoStandardCallback is set as an NPN callback for the case when
++/* ssl_NextProtoNegoCallback is set as an NPN callback for the case when
+  * SSL_SetNextProtoNego is used.
+  */
+ static SECStatus
+@@ -1349,12 +1349,12 @@ pick_first:
+     result = ss->opt.nextProtoNego.data;
+ 
+ found:
+-    *protoOutLen = result[0];
+     if (protoMaxLen < result[0]) {
+ 	PORT_SetError(SEC_ERROR_OUTPUT_LEN);
+ 	return SECFailure;
+     }
+     memcpy(protoOut, result + 1, result[0]);
++    *protoOutLen = result[0];
+     return SECSuccess;
+ }
+ 
+@@ -1408,13 +1408,12 @@ SSL_GetNextProto(PRFileDesc *fd, SSLNext
+ 
+     if (ss->ssl3.nextProtoState != SSL_NEXT_PROTO_NO_SUPPORT &&
+ 	ss->ssl3.nextProto.data) {
+-	*bufLen = ss->ssl3.nextProto.len;
+-	if (*bufLen > bufLenMax) {
++	if (ss->ssl3.nextProto.len > bufLenMax) {
+ 	    PORT_SetError(SEC_ERROR_OUTPUT_LEN);
+-	    *bufLen = 0;
+ 	    return SECFailure;
+ 	}
+ 	PORT_Memcpy(buf, ss->ssl3.nextProto.data, ss->ssl3.nextProto.len);
++	*bufLen = ss->ssl3.nextProto.len;
+     } else {
+ 	*bufLen = 0;
+     }
diff --git a/net/third_party/nss/ssl/ssl3ext.c b/net/third_party/nss/ssl/ssl3ext.c
index 412e799..ef015fa 100644
--- a/net/third_party/nss/ssl/ssl3ext.c
+++ b/net/third_party/nss/ssl/ssl3ext.c
@@ -606,10 +606,7 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type,
     unsigned char resultBuffer[255];
     SECItem result = { siBuffer, resultBuffer, 0 };
 
-    if (ss->firstHsDone) {
-	PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID);
-	return SECFailure;
-    }
+    PORT_Assert(!ss->firstHsDone);
 
     rv = ssl3_ValidateNextProtoNego(data->data, data->len);
     if (rv != SECSuccess)
@@ -621,6 +618,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type,
      */
     PORT_Assert(ss->nextProtoCallback != NULL);
     if (!ss->nextProtoCallback) {
+	/* XXX Use a better error code. This is an application error, not an
+	 * NSS bug. */
 	PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
 	return SECFailure;
     }
@@ -631,7 +630,7 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type,
 	return rv;
     /* If the callback wrote more than allowed to |result| it has corrupted our
      * stack. */
-    if (result.len > sizeof result) {
+    if (result.len > sizeof resultBuffer) {
 	PORT_SetError(SEC_ERROR_OUTPUT_LEN);
 	return SECFailure;
     }
diff --git a/net/third_party/nss/ssl/sslsock.c b/net/third_party/nss/ssl/sslsock.c
index c183566..9812549 100644
--- a/net/third_party/nss/ssl/sslsock.c
+++ b/net/third_party/nss/ssl/sslsock.c
@@ -1344,7 +1344,7 @@ SSL_SetNextProtoCallback(PRFileDesc *fd, SSLNextProtoCallback callback,
     return SECSuccess;
 }
 
-/* NextProtoStandardCallback is set as an NPN callback for the case when
+/* ssl_NextProtoNegoCallback is set as an NPN callback for the case when
  * SSL_SetNextProtoNego is used.
  */
 static SECStatus
@@ -1390,12 +1390,12 @@ pick_first:
     result = ss->opt.nextProtoNego.data;
 
 found:
-    *protoOutLen = result[0];
     if (protoMaxLen < result[0]) {
 	PORT_SetError(SEC_ERROR_OUTPUT_LEN);
 	return SECFailure;
     }
     memcpy(protoOut, result + 1, result[0]);
+    *protoOutLen = result[0];
     return SECSuccess;
 }
 
@@ -1449,13 +1449,12 @@ SSL_GetNextProto(PRFileDesc *fd, SSLNextProtoState *state, unsigned char *buf,
 
     if (ss->ssl3.nextProtoState != SSL_NEXT_PROTO_NO_SUPPORT &&
 	ss->ssl3.nextProto.data) {
-	*bufLen = ss->ssl3.nextProto.len;
-	if (*bufLen > bufLenMax) {
+	if (ss->ssl3.nextProto.len > bufLenMax) {
 	    PORT_SetError(SEC_ERROR_OUTPUT_LEN);
-	    *bufLen = 0;
 	    return SECFailure;
 	}
 	PORT_Memcpy(buf, ss->ssl3.nextProto.data, ss->ssl3.nextProto.len);
+	*bufLen = ss->ssl3.nextProto.len;
     } else {
 	*bufLen = 0;
     }