Support TLS Channel IDs in NSS

net/third_party/nss/ssl/ssl3ext.c

 /*
  * SSL3 Protocol
  *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
@@ skipping 62 matching lines @@
     PK11SymKey **aes_key, PK11SymKey **mac_key);
 static SECStatus ssl3_GetSessionTicketKeys(const unsigned char **aes_key,
     PRUint32 *aes_key_length, const unsigned char **mac_key,
     PRUint32 *mac_key_length);
 static PRInt32 ssl3_SendRenegotiationInfoXtn(sslSocket * ss,
     PRBool append, PRUint32 maxBytes);
 static SECStatus ssl3_HandleRenegotiationInfoXtn(sslSocket *ss, 
     PRUint16 ex_type, SECItem *data);
 static SECStatus ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss,
                         PRUint16 ex_type, SECItem *data);
+static SECStatus ssl3_ClientHandleChannelIDXtn(sslSocket *ss,
+                        PRUint16 ex_type, SECItem *data);
 static SECStatus ssl3_ServerHandleNextProtoNegoXtn(sslSocket *ss,
                         PRUint16 ex_type, SECItem *data);
 static PRInt32 ssl3_ClientSendNextProtoNegoXtn(sslSocket *ss, PRBool append,
                                                PRUint32 maxBytes);
+static PRInt32 ssl3_ClientSendChannelIDXtn(sslSocket *ss, PRBool append,
+                                          PRUint32 maxBytes);

[wtc, 2012/05/24 23:10:03]

We should add a server side implementation that at least
echoes the extension and verifies the signature in the
EncryptedExtensions message.  This can be done later.

 
 /*
  * Write bytes.  Using this function means the SECItem structure
  * cannot be freed.  The caller is expected to call this function
  * on a shallow copy of the structure.
  */
 static SECStatus
 ssl3_AppendToItem(SECItem *item, const unsigned char *buf, PRUint32 bytes)
 {
     if (bytes > item->len)
@@ skipping 149 matching lines @@
 };
 
 /* These two tables are used by the client, to handle server hello
  * extensions. */
 static const ssl3HelloExtensionHandler serverHelloHandlersTLS[] = {
     { ssl_server_name_xtn,        &ssl3_HandleServerNameXtn },
     /* TODO: add a handler for ssl_ec_point_formats_xtn */
     { ssl_session_ticket_xtn,     &ssl3_ClientHandleSessionTicketXtn },
     { ssl_renegotiation_info_xtn, &ssl3_HandleRenegotiationInfoXtn },
     { ssl_next_proto_nego_xtn,    &ssl3_ClientHandleNextProtoNegoXtn },
+    { ssl_channel_id_xtn,          &ssl3_ClientHandleChannelIDXtn },
     { ssl_cert_status_xtn,        &ssl3_ClientHandleStatusRequestXtn },
     { -1, NULL }
 };
 
 static const ssl3HelloExtensionHandler serverHelloHandlersSSL3[] = {
     { ssl_renegotiation_info_xtn, &ssl3_HandleRenegotiationInfoXtn },
     { -1, NULL }
 };
 
 /* Tables of functions to format TLS hello extensions, one function per
  * extension.
  * These static tables are for the formatting of client hello extensions.
  * The server's table of hello senders is dynamic, in the socket struct,
  * and sender functions are registered there.
  */
 static const 
 ssl3HelloExtensionSender clientHelloSendersTLS[SSL_MAX_EXTENSIONS] = {
     { ssl_server_name_xtn,        &ssl3_SendServerNameXtn        },
     { ssl_renegotiation_info_xtn, &ssl3_SendRenegotiationInfoXtn },
 #ifdef NSS_ENABLE_ECC
     { ssl_elliptic_curves_xtn,    &ssl3_SendSupportedCurvesXtn },
     { ssl_ec_point_formats_xtn,   &ssl3_SendSupportedPointFormatsXtn },
 #endif
     { ssl_session_ticket_xtn,     &ssl3_SendSessionTicketXtn },
     { ssl_next_proto_nego_xtn,    &ssl3_ClientSendNextProtoNegoXtn },
+    { ssl_channel_id_xtn,          &ssl3_ClientSendChannelIDXtn },
     { ssl_cert_status_xtn,        &ssl3_ClientSendStatusRequestXtn }
     /* any extra entries will appear as { 0, NULL }    */
 };
 
 static const 
 ssl3HelloExtensionSender clientHelloSendersSSL3[SSL_MAX_EXTENSIONS] = {
     { ssl_renegotiation_info_xtn, &ssl3_SendRenegotiationInfoXtn }
     /* any extra entries will appear as { 0, NULL }    */
 };
 
@@ skipping 337 matching lines @@
         PORT_SetError(SEC_ERROR_OUTPUT_LEN);
         return SECFailure;
     }
 
     ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
 
     SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE);
     return SECITEM_CopyItem(NULL, &ss->ssl3.nextProto, &result);
 }
 
+static SECStatus
+ssl3_ClientHandleChannelIDXtn(sslSocket *ss, PRUint16 ex_type,

[wtc, 2012/05/24 23:10:03]

Please move this next to the ssl3_ClientSendChannelIDXtn
function.

[agl, 2012/05/30 15:28:33]

Done.

+                             SECItem *data)
+{
+    if (!ss->ssl3.channelID)
+        return 0;

[wtc, 2012/05/24 23:10:03]

BUG: we should return either SECSuccess or SECFailure here.
It seems that we should return SECFailure (and set an
error code) if ss->ssl3.channelID is NULL.

Actually, this cannot happen, so you can remove this check
(or replace it with a PORT_Assert).

If ss->ssl3.channelID is NULL, we won't send the extension.

If the server sends an unsolicited extension, NSS will
detect that and won't call the client extension handler.

This is why ss->ssl3.channelID cannot be NULL here.

[agl, 2012/05/30 15:28:33]

Done.

+
+    if (data->len) {
+        PORT_SetError(SSL_ERROR_BAD_CHANNEL_ID_DATA);
+        return SECFailure;
+    }
+    ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
+    return SECSuccess;
+}
+
 static PRInt32
 ssl3_ClientSendNextProtoNegoXtn(sslSocket * ss, PRBool append,
                                 PRUint32 maxBytes)
 {
     PRInt32 extension_length;
 
     /* Renegotiations do not send this extension. */
     if (!ss->nextProtoCallback || ss->firstHsDone) {
         return 0;
     }
@@ skipping 13 matching lines @@
     } else if (maxBytes < extension_length) {
         return 0;
     }
 
     return extension_length;
 
 loser:
     return -1;
 }
 
+static PRInt32
+ssl3_ClientSendChannelIDXtn(sslSocket * ss, PRBool append,
+                            PRUint32 maxBytes)
+{
+    PRInt32 extension_length = 4;
+
+    if (!ss->ssl3.channelID)

[wtc, 2012/05/24 23:10:03]

BUG: this should be
    /* Renegotiations do not send this extension. */
    if (!ss->ssl3.channelID || ss->firstHsDone)
        return 0;

[agl, 2012/05/30 15:28:33]

As noted in ssl3con.c, the semantics have changed so that this is sent on
renegotiation. So the bug was that the code *wasn't* sending it on renegotiation
in ssl3con, rather than here.

+        return 0;
+
+    if (append && maxBytes >= extension_length) {
+        SECStatus rv;
+        rv = ssl3_AppendHandshakeNumber(ss, ssl_channel_id_xtn, 2);
+        if (rv != SECSuccess)
+            goto loser;
+        rv = ssl3_AppendHandshakeNumber(ss, 0, 2);
+        if (rv != SECSuccess)
+            goto loser;
+        ss->xtnData.advertised[ss->xtnData.numAdvertised++] =
+                ssl_channel_id_xtn;
+    } else if (maxBytes < extension_length) {
+        return 0;
+    }
+
+    return extension_length;

[wtc, 2012/05/24 23:10:03]

Nit: I find this logic difficult to understand.  I know
you copied the logic from other functions.  I think this
is equivalent and clearer:

    if (maxBytes < extension_length) {
        PORT_Assert(0);
        return 0;
    }
    if (append) {
        SECStatus rv;
        rv = ssl3_AppendHandshakeNumber(ss, ssl_channel_id_xtn, 2);
        ...
    }
    return extension_length;

What do you think?

[agl, 2012/05/30 15:28:33]

Agreed. Done.

+
+loser:
+    return -1;
+}
+
 SECStatus
 ssl3_ClientHandleStatusRequestXtn(sslSocket *ss, PRUint16 ex_type,
                                   SECItem *data)
 {
     /* If we didn't request this extension, then the server may not echo it. */
     if (!ss->opt.enableOCSPStapling)
         return SECFailure;
 
     /* The echoed extension must be empty. */
     if (data->len != 0)
@@ skipping 1042 matching lines @@
     ss->peerRequestedProtection = 1;
     ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
     if (ss->sec.isServer) {
         /* prepare to send back the appropriate response */
         rv = ssl3_RegisterServerHelloExtensionSender(ss, ex_type,
                                              ssl3_SendRenegotiationInfoXtn);
     }
     return rv;
 }