Bug 403563: checked in Nagendra Modadugu's patch v3

- moved TLS1ExtensionData to ss
- shortened member names
- removed if (length > 0) goto alert_loser from ssl3_HandleServerHello in
  ssl3con.c so we ignore stuff after the end of the server hello.
Modified Files:
 Tag: NSS_RFC4507BIS_BRANCH
	ssl3con.c ssl3ext.c sslimpl.h

3 files changed, 46 insertions, 57 deletions

diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c
index 887dab716..ef533bfb6 100644
--- a/security/nss/lib/ssl/ssl3con.c
+++ b/security/nss/lib/ssl/ssl3con.c
@@ -3507,7 +3507,7 @@ ssl3_SendClientHello(sslSocket *ss)
     /* We might be starting a session renegotiation in which case we should
      * clear previous state.
      */
-    PORT_Memset(&ss->ssl3.extension_data, 0, sizeof(TLS1ExtensionData));
+    PORT_Memset(&ss->xtnData, 0, sizeof(TLS1ExtensionData));
 
     SSL_TRC(30,("%d: SSL3[%d]: reset handshake hashes",
 	    SSL_GETPID(), ss->fd ));
@@ -4662,8 +4662,6 @@ ssl3_HandleServerHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
 	    goto alert_loser;
 	rv = ssl3_HandleHelloExtensions(ss, &extensions.data, &extensions.len);
 	if (rv != SECSuccess) goto alert_loser;
-    } else if (length > 0) {
-	goto alert_loser;
     }
 #endif
 
@@ -5629,8 +5627,8 @@ ssl3_HandleClientHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
     /* We might be starting a session renegotiation in which case we should
      * clear previous state.
      */
-    PORT_Memset(&ss->ssl3.extension_data, 0, sizeof(TLS1ExtensionData));
-    ss->ssl3.stateless_resume = PR_FALSE;
+    PORT_Memset(&ss->xtnData, 0, sizeof(TLS1ExtensionData));
+    ss->statelessResume = PR_FALSE;
 
     /* OpenSSL 0.9.8g sends TLS extensions even when negotiating SSL3,
      * so we simply ignore any trailing bytes if the negotiated
@@ -5658,7 +5656,7 @@ ssl3_HandleClientHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
      * ticket extension, but sent an empty ticket.
      */
     if (!ssl3_ExtensionNegotiated(ss, session_ticket_xtn) ||
-	ss->ssl3.extension_data.empty_session_ticket) {
+	ss->xtnData.emptySessionTicket) {
 	if (sidBytes.len > 0 && !ss->opt.noCache) {
 	    SSL_TRC(7, ("%d: SSL3[%d]: server, lookup client session-id for 0x%08x%08x%08x%08x",
 			SSL_GETPID(), ss->fd, ss->sec.ci.peer.pr_s6_addr32[0],
@@ -5673,7 +5671,7 @@ ssl3_HandleClientHello(sslSocket *ss, SSL3Opaque *b, PRUint32 length)
 		goto loser;
 	    }
 	}
-    } else if (ss->ssl3.stateless_resume) {
+    } else if (ss->statelessResume) {
 	/* Fill in the client's session ID if doing a stateless resume.
 	 * (When doing stateless resumes, server echos client's SessionID.)
 	 */
@@ -5926,7 +5924,7 @@ compression_found:
 	 * XXX make sure compression still matches
 	 */
 	SSL_AtomicIncrementLong(& ssl3stats.hch_sid_cache_hits );
-	if (ss->ssl3.stateless_resume)
+	if (ss->statelessResume)
 	    SSL_AtomicIncrementLong(&ssl3stats.hch_sid_stateless_resumes);
 
 	ss->ssl3.hs.isResuming = PR_TRUE;
@@ -6235,7 +6233,7 @@ ssl3_SendServerHello(sslSocket *ss)
     sid = ss->sec.ci.sid;
 
     extensions_len = ssl3_CallHelloExtensionSenders(ss, PR_FALSE, maxBytes,
-	&ss->ssl3.extension_data.serverExtensionSenders[0]);
+	&ss->xtnData.senders[0]);
     if (extensions_len > 0)
     	extensions_len += 2; /* Add sizeof total extension length */
 
@@ -6287,7 +6285,7 @@ ssl3_SendServerHello(sslSocket *ss)
 	if (rv != SECSuccess) 
 	    return rv;	/* err set by ssl3_SetupPendingCipherSpec */
 	sent_len = ssl3_CallHelloExtensionSenders(ss, PR_TRUE, extensions_len,
-	    &ss->ssl3.extension_data.serverExtensionSenders[0]);
+	    &ss->xtnData.senders[0]);
         PORT_Assert(sent_len == extensions_len);
 	if (sent_len != extensions_len) {
 	    if (sent_len >= 0)
@@ -8413,7 +8411,7 @@ ssl3_InitState(sslSocket *ss)
 #endif
     ssl_ReleaseSpecWriteLock(ss);
 
-    PORT_Memset(&ss->ssl3.extension_data, 0, sizeof(TLS1ExtensionData));
+    PORT_Memset(&ss->xtnData, 0, sizeof(TLS1ExtensionData));
 
     rv = ssl3_NewHandshakeHashes(ss);
     if (rv == SECSuccess) {
diff --git a/security/nss/lib/ssl/ssl3ext.c b/security/nss/lib/ssl/ssl3ext.c
index c202c4daa..bf50383e1 100644
--- a/security/nss/lib/ssl/ssl3ext.c
+++ b/security/nss/lib/ssl/ssl3ext.c
@@ -253,16 +253,16 @@ arrayContainsExtension(PRUint16 *array, PRUint32 array_len, PRUint16 ex_type)
 
 PRBool
 ssl3_ExtensionNegotiated(sslSocket *ss, PRUint16 ex_type) {
-    TLS1ExtensionData *extension_data = &ss->ssl3.extension_data;
-    return arrayContainsExtension(extension_data->negotiatedExtensions,
-	extension_data->numNegotiatedExtensions, ex_type);
+    TLS1ExtensionData *xtnData = &ss->xtnData;
+    return arrayContainsExtension(xtnData->negotiated,
+	xtnData->numNegotiated, ex_type);
 }
 
 PRBool
 ssl3_ClientExtensionAdvertised(sslSocket *ss, PRUint16 ex_type) {
-    TLS1ExtensionData *extension_data = &ss->ssl3.extension_data;
-    return arrayContainsExtension(extension_data->advertisedClientExtensions,
-	extension_data->numAdvertisedClientExtensions, ex_type);
+    TLS1ExtensionData *xtnData = &ss->xtnData;
+    return arrayContainsExtension(xtnData->advertised,
+	xtnData->numAdvertised, ex_type);
 }
 
 /* Format an SNI extension, using the name from the socket's URL,
@@ -303,10 +303,8 @@ ssl3_SendServerNameExt(
 	rv = ssl3_AppendHandshakeVariable(ss, (unsigned char *)ss->url, len, 2);
 	if (rv != SECSuccess) return -1;
 	if (!ss->sec.isServer) {
-	    TLS1ExtensionData *ex_data = &ss->ssl3.extension_data;
-	    ex_data->advertisedClientExtensions[
-		    ex_data->numAdvertisedClientExtensions++] =
-		server_name_xtn;
+	    TLS1ExtensionData *xtnData = &ss->xtnData;
+	    xtnData->advertised[xtnData->numAdvertised++] = server_name_xtn;
 	}
     }
     return len + 9;
@@ -351,14 +349,14 @@ ssl3_SendSessionTicketExt(
 	sid = ss->sec.ci.sid;
 	session_ticket = &sid->u.ssl3.session_ticket;
 	if (session_ticket->ticket.data) {
-	    if (ss->ssl3.extension_data.ticket_timestamp_verified) {
+	    if (ss->xtnData.ticketTimestampVerified) {
 		extension_length += session_ticket->ticket.len;
 	    } else if (!append &&
 		(session_ticket->ticket_lifetime_hint == 0 ||
 		(session_ticket->ticket_lifetime_hint +
 		    session_ticket->received_timestamp > ssl_Time()))) {
 		extension_length += session_ticket->ticket.len;
-		ss->ssl3.extension_data.ticket_timestamp_verified = PR_TRUE;
+		ss->xtnData.ticketTimestampVerified = PR_TRUE;
 	    }
 	}
     }
@@ -370,10 +368,10 @@ ssl3_SendSessionTicketExt(
         if (rv != SECSuccess)
 	    goto loser;
 	if (session_ticket && session_ticket->ticket.data &&
-	    ss->ssl3.extension_data.ticket_timestamp_verified) {
+	    ss->xtnData.ticketTimestampVerified) {
 	    rv = ssl3_AppendHandshakeVariable(ss, session_ticket->ticket.data,
 		session_ticket->ticket.len, 2);
-	    ss->ssl3.extension_data.ticket_timestamp_verified = PR_FALSE;
+	    ss->xtnData.ticketTimestampVerified = PR_FALSE;
 	} else {
 	    rv = ssl3_AppendHandshakeNumber(ss, 0, 2);
 	}
@@ -381,10 +379,8 @@ ssl3_SendSessionTicketExt(
 	    goto loser;
 
 	if (!ss->sec.isServer) {
-	    TLS1ExtensionData *ex_data = &ss->ssl3.extension_data;
-	    ex_data->advertisedClientExtensions[
-		    ex_data->numAdvertisedClientExtensions++] =
-		session_ticket_xtn;
+	    TLS1ExtensionData *xtnData = &ss->xtnData;
+	    xtnData->advertised[xtnData->numAdvertised++] = session_ticket_xtn;
 	}
     } else if (maxBytes < extension_length) {
 	PORT_Assert(0);
@@ -393,7 +389,7 @@ ssl3_SendSessionTicketExt(
     return extension_length;
 
  loser:
-    ss->ssl3.extension_data.ticket_timestamp_verified = PR_FALSE;
+    ss->xtnData.ticketTimestampVerified = PR_FALSE;
     return -1;
 }
 
@@ -712,14 +708,11 @@ SECStatus
 ssl3_ClientHandleSessionTicketExt(sslSocket *ss, PRUint16 ex_type,
     SECItem *data)
 {
-    TLS1ExtensionData *ex_data;
     if (data->len != 0)
 	return SECFailure;
 
     /* Keep track of negotiated extensions. */
-    ex_data = &ss->ssl3.extension_data;
-    ex_data->negotiatedExtensions[ex_data->numNegotiatedExtensions++] =
-	ex_type;
+    ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
     return SECSuccess;
 }
 
@@ -738,15 +731,14 @@ ssl3_ServerHandleSessionTicketExt(sslSocket *ss, PRUint16 ex_type,
 	return SECSuccess;
 
     /* Keep track of negotiated extensions. */
-    ss->ssl3.extension_data.negotiatedExtensions[
-	    ss->ssl3.extension_data.numNegotiatedExtensions++] = ex_type;
+    ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
 
     /* Parse the received ticket sent in by the client.  We are
      * lenient about some parse errors, falling back to a fullshake
      * instead of terminating the current connection.
      */
     if (data->len == 0) {
-	ss->ssl3.extension_data.empty_session_ticket = PR_TRUE;
+	ss->xtnData.emptySessionTicket = PR_TRUE;
     } else {
 	int                    i;
 	SECItem                extension_data;
@@ -1036,7 +1028,7 @@ ssl3_ServerHandleSessionTicketExt(sslSocket *ss, PRUint16 ex_type,
 		    goto loser;
 		}
 	    }
-	    ss->ssl3.stateless_resume = PR_TRUE;
+	    ss->statelessResume = PR_TRUE;
 	    ss->sec.ci.sid = sid;
 	}
     }
@@ -1122,7 +1114,7 @@ ssl3_HandleHelloExtensions(sslSocket *ss,
 	SECStatus rv;
 	PRInt32   extension_type;
 	SECItem   extension_data;
-	TLS1ExtensionData *ex_data = &ss->ssl3.extension_data;
+	TLS1ExtensionData *xtnData = &ss->xtnData;
 	const ssl3HelloExtensionHandler * handler;
 
 	/* Get the extension's type field */
@@ -1143,8 +1135,8 @@ ssl3_HandleHelloExtensions(sslSocket *ss,
 	    return SECFailure;
 
 	/* Check whether an extension has been sent multiple times. */
-	if (arrayContainsExtension(ex_data->negotiatedExtensions,
-		ex_data->numNegotiatedExtensions, extension_type))
+	if (arrayContainsExtension(xtnData->negotiated,
+		xtnData->numNegotiated, extension_type))
 	    return SECFailure;
 
 	/* find extension_type in table of Client Hello Extension Handlers */
@@ -1169,8 +1161,7 @@ ssl3_RegisterServerHelloExtensionSender(sslSocket *ss, PRUint16 ex_type,
 				        ssl3HelloExtensionSenderFunc cb)
 {
     int i;
-    ssl3HelloExtensionSender *sender =
-        &ss->ssl3.extension_data.serverExtensionSenders[0];
+    ssl3HelloExtensionSender *sender = &ss->xtnData.senders[0];
 
     for (i = 0; i < MAX_EXTENSION_SENDERS; ++i, ++sender) {
         if (!sender->ex_sender) {
diff --git a/security/nss/lib/ssl/sslimpl.h b/security/nss/lib/ssl/sslimpl.h
index 19eab2ee9..f9f2c33e1 100644
--- a/security/nss/lib/ssl/sslimpl.h
+++ b/security/nss/lib/ssl/sslimpl.h
@@ -710,16 +710,16 @@ typedef struct SessionTicketDataStr      SessionTicketData;
 
 struct TLS1ExtensionDataStr {
     /* registered callbacks that send server hello extensions */
-    ssl3HelloExtensionSender serverExtensionSenders[MAX_EXTENSION_SENDERS];
+    ssl3HelloExtensionSender senders[MAX_EXTENSION_SENDERS];
     /* Keep track of the extensions that are negotiated. */
-    PRUint16 numAdvertisedClientExtensions;
-    PRUint16 numNegotiatedExtensions;
-    PRUint16 advertisedClientExtensions[MAX_EXTENSION_SENDERS];
-    PRUint16 negotiatedExtensions[MAX_EXTENSION_SENDERS];
+    PRUint16 numAdvertised;
+    PRUint16 numNegotiated;
+    PRUint16 advertised[MAX_EXTENSION_SENDERS];
+    PRUint16 negotiated[MAX_EXTENSION_SENDERS];
 
     /* SessionTicket Extension related data. */
-    PRBool ticket_timestamp_verified;
-    PRBool empty_session_ticket;
+    PRBool ticketTimestampVerified;
+    PRBool emptySessionTicket;
 };
 
 /*
@@ -797,13 +797,6 @@ struct ssl3StateStr {
     PRBool               initialized;
     SSL3HandshakeState   hs;
     ssl3CipherSpec       specs[2];	/* one is current, one is pending. */
-
-    /*
-     * TLS1 Extension related data.
-     */
-    /* True when the current session is a stateless resume. */
-    PRBool               stateless_resume;
-    TLS1ExtensionData    extension_data;
 };
 
 typedef struct {
@@ -1088,6 +1081,13 @@ const unsigned char *  preferredCipher;
 
     /* SSL3 state info.  Formerly was a pointer */
     ssl3State        ssl3;
+
+    /*
+     * TLS1 Extension related data.
+     */
+    /* True when the current session is a stateless resume. */
+    PRBool               statelessResume;
+    TLS1ExtensionData    xtnData;
 };