Support TLS Channel IDs in NSS

net/third_party/nss/ssl/ssl3con.c

 /* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /*
  * 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
@@ skipping 68 matching lines @@
 static PK11SymKey *ssl3_GenerateRSAPMS(sslSocket *ss, ssl3CipherSpec *spec,
                                        PK11SlotInfo * serverKeySlot);
 static SECStatus ssl3_DeriveMasterSecret(sslSocket *ss, PK11SymKey *pms);
 static SECStatus ssl3_DeriveConnectionKeysPKCS11(sslSocket *ss);
 static SECStatus ssl3_HandshakeFailure(      sslSocket *ss);
 static SECStatus ssl3_InitState(             sslSocket *ss);
 static SECStatus ssl3_SendCertificate(       sslSocket *ss);
 static SECStatus ssl3_SendEmptyCertificate(  sslSocket *ss);
 static SECStatus ssl3_SendCertificateRequest(sslSocket *ss);
 static SECStatus ssl3_SendNextProto(         sslSocket *ss);
+static SECStatus ssl3_SendEncryptedExtensions(sslSocket *ss);
 static SECStatus ssl3_SendFinished(          sslSocket *ss, PRInt32 flags);
 static SECStatus ssl3_SendServerHello(       sslSocket *ss);
 static SECStatus ssl3_SendServerHelloDone(   sslSocket *ss);
 static SECStatus ssl3_SendServerKeyExchange( sslSocket *ss);
 static SECStatus ssl3_NewHandshakeHashes(    sslSocket *ss);
 static SECStatus ssl3_UpdateHandshakeHashes( sslSocket *ss,
                                              const unsigned char *b,
                                              unsigned int l);
 static SECStatus ssl3_FlushHandshakeMessages(sslSocket *ss, PRInt32 flags);
 
@@ skipping 6132 matching lines @@
     }
 
     /* XXX: If the server's certificate hasn't been authenticated by this
      * point, then we may be leaking this NPN message to an attacker.
      */
     if (!ss->firstHsDone) {
         rv = ssl3_SendNextProto(ss);
         if (rv != SECSuccess) {
             goto loser; /* err code was set. */
         }
+        rv = ssl3_SendEncryptedExtensions(ss);
+        if (rv != SECSuccess) {
+            goto loser; /* err code was set. */
+        }
     }
 
     rv = ssl3_SendFinished(ss, 0);
     if (rv != SECSuccess) {
         goto loser;     /* err code was set. */
     }
 
     ssl_ReleaseXmitBufLock(ss);         /*******************************/
 
     if (ssl3_ExtensionNegotiated(ss, ssl_session_ticket_xtn))
@@ skipping 2563 matching lines @@
         inData.data  = (unsigned char *) val;
         inData.len   = valLen;
         outData.data = out;
         outData.len  = outLen;
         rv = TLS_PRF(&spec->msItem, label, &inData, &outData, isFIPS);
         PORT_Assert(rv != SECSuccess || outData.len == outLen);
     }
     return rv;
 }
 
 /* called from ssl3_HandleServerHelloDone

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

This comment is incomplete -- ssl3_SendNextProto is called
from two places:
- ssl3_SendClientSecondRound (was ssl3_HandleServerHelloDone
  before Brian Smith's refactoring)
- ssl3_HandleFinished

If you agree, I will fix this comment upstream.

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

Yes, that's correct. I'll avoid changing it here to prevent a merge conflict
form upstream in the future.

  */
 static SECStatus
 ssl3_SendNextProto(sslSocket *ss)
 {
     SECStatus rv;
     int padding_len;
     static const unsigned char padding[32] = {0};
 
     if (ss->ssl3.nextProto.len == 0)
         return SECSuccess;
@@ skipping 13 matching lines @@
     if (rv != SECSuccess) {
         return rv;      /* error code set by AppendHandshake */
     }
     rv = ssl3_AppendHandshakeVariable(ss, padding, padding_len, 1);
     if (rv != SECSuccess) {
         return rv;      /* error code set by AppendHandshake */
     }
     return rv;
 }
 
 /* called from ssl3_HandleServerHelloDone

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

This comment should be updated in the same way.

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

Done.

+ */
+static SECStatus
+ssl3_SendEncryptedExtensions(sslSocket *ss)

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

Ideally this function should be structured so that it calls
encrypted extension senders.  Right now it is hardcoded to
send the Channel ID extension only.  This is acceptable as
a first implementation.

+{
+    SECStatus rv = SECFailure;
+    SECItem *spki = NULL;
+    SSL3Hashes hashes;
+    const unsigned char *pub_bytes;
+    static const char CHANNEL_ID_MAGIC[] = "TLS Channel ID signature";
+    unsigned char signed_data[sizeof(CHANNEL_ID_MAGIC) + sizeof(SSL3Hashes)];

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

IMPORTANT: By using sizeof(CHANNEL_ID_MAGIC), the terminating null
byte in the "TLS Channel ID signature" string literal will
be included.  Just wanted to confirm this is what you wanted.

If not, you need to subtract one from sizeof(CHANNEL_ID_MAGIC)
to get the string length.

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

Yes, I intended to include the NUL byte. It's not terribly important but the
thinking was that we want signature context strings to be prefix free: i.e. we
don't want context strings "abc" and "abcdef" both to be in use because the
latter is valid for the former.

+    unsigned char digest[SHA256_LENGTH];
+    SECItem digest_item;
+    unsigned char signature[64];
+    SECItem signature_item;
+
+    if (!ssl3_ExtensionNegotiated(ss, ssl_channel_id_xtn))
+        return SECSuccess;

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

Move these two lines after the lock assertions:

    PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss));
    PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));

    if (!ssl3_ExtensionNegotiated(ss, ssl_channel_id_xtn))
        return SECSuccess;

    PORT_Assert(ss->ssl3.channelID);

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

Done.

+
+    PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss));
+    PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
+    PORT_Assert(ss->ssl3.channelID);
+
+    /* ChannelIDs are always 128 bytes long: 64 bytes of P-256 public key and 64
+     * bytes of ECDSA signature. */
+    static const int CHANNEL_ID_PUBLIC_KEY_LENGTH = 64;
+    static const int CHANNEL_ID_LENGTH = 128;

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

Move these two lines up to the variable declaration section.
(NSS is still being compiled by pre-C99 compilers.)

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

Done.

+
+    if (SECKEY_GetPrivateKeyType(ss->ssl3.channelID) != ecKey ||
+        PK11_SignatureLen(ss->ssl3.channelID) != sizeof(signature)) {
+        PORT_SetError(SSL_ERROR_INVALID_CHANNEL_ID_KEY);
+        return SECFailure;
+    }
+
+    rv = ssl3_ComputeHandshakeHashes(ss, ss->ssl3.cwSpec, &hashes, PR_TRUE);

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

Pass 0 as the last argument.  It is only used by SSLv3.

I suggest holding the spec read lock across this
ssl3_ComputeHandshakeHashes call.

Are you sure we should use cwSpec rather than pwSpec?

See the equivalent code in ssl3_SendCertificateVerify:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/ssl3...

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

Done.
Done.
Unlike when sending the CertificateVerify, this message is sent under
encryption. Therefore I believe that the pending spec has become the current
spec at this point.

+    if (rv != SECSuccess)
+        goto loser;
+
+    rv = ssl3_AppendHandshakeHeader(ss, encrypted_extensions,
+                                    2 + 2 + CHANNEL_ID_LENGTH);
+    if (rv != SECSuccess)
+        return rv;      /* error code set by AppendHandshakeHeader */
+    rv = ssl3_AppendHandshakeNumber(ss, ssl_channel_id_xtn, 2);
+    if (rv != SECSuccess)
+        return rv;      /* error code set by AppendHandshake */
+    rv = ssl3_AppendHandshakeNumber(ss, CHANNEL_ID_LENGTH, 2);
+    if (rv != SECSuccess)
+        return rv;      /* error code set by AppendHandshake */
+
+    spki = SECKEY_EncodeDERSubjectPublicKeyInfo(ss->ssl3.channelIDPub);
+
+    /* This is the ASN.1 prefix for a P-256 public key. Specifically it's:
+     * SEQUENCE
+     *   SEQUENCE
+     *     OID id-ecPublicKey
+     *     OID prime256v1
+     *   BIT STRING, length 66, 0 trailing bits: 0x04
+     *
+     * The 0x04 in the BIT STRING is the prefix for an uncompressed, X9.62
+     * public key. Following that are the two field elements as 32-byte,
+     * big-endian numbers, as required by the Channel ID. */
+    static const unsigned char P256_SPKI_PREFIX[] = {

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

Move this up to the variable declaration section.

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

Done.

+        0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86,
+        0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a,
+        0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03,
+        0x42, 0x00, 0x04
+    };
+
+    if (spki->len != sizeof(P256_SPKI_PREFIX) + CHANNEL_ID_PUBLIC_KEY_LENGTH ||
+        memcmp(spki->data, P256_SPKI_PREFIX, sizeof(P256_SPKI_PREFIX) != 0)) {
+        PORT_SetError(SSL_ERROR_INVALID_CHANNEL_ID_KEY);
+        return SECFailure;

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

Replace
        return SECFailure;
by
        rv = SECFailure;
        goto loser;

otherwise |spki| is leaked.

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

Good catch, thanks!

+    }
+
+    pub_bytes = spki->data + sizeof(P256_SPKI_PREFIX);
+
+    memcpy(signed_data, CHANNEL_ID_MAGIC, sizeof(CHANNEL_ID_MAGIC));
+    memcpy(signed_data + sizeof(CHANNEL_ID_MAGIC), &hashes, sizeof(hashes));
+
+    rv = SHA256_HashBuf(digest, signed_data, sizeof(signed_data));

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

Please use the PK11_ function instead:

  rv = PK11_HashBuf(SEC_OID_SHA256, digest, signed_data, sizeof(signed_data));

SHA256_HashBuf can only be used when we are in bypassPKCS11
mode.

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

Done.

+    if (rv != SECSuccess)
+        goto loser;
+
+    digest_item.data = (void*) &digest;

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

BUG: I believe this should be
    digest_item.data = digest;

Similarly line 8949 below should be
    signature_item.data = signature;

Perhaps for an array |digest|, &digest is the same as
&digest[0]?

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

Yes, that's the case. However it's surprising so I've removed the &.

+    digest_item.len = sizeof(digest);
+
+    signature_item.data = (void*) &signature;
+    signature_item.len = sizeof(signature);
+
+    rv = PK11_Sign(ss->ssl3.channelID, &signature_item, &digest_item);
+    if (rv != SECSuccess)
+        goto loser;
+
+    rv = ssl3_AppendHandshake(ss, pub_bytes, CHANNEL_ID_PUBLIC_KEY_LENGTH);
+    rv = ssl3_AppendHandshake(ss, signature, sizeof(signature));
+
+    rv = SECSuccess;

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

BUG: remove this line because it suppresses the failure
of the ssl3_AppendHandshake call.

Add error checking:
    rv = ssl3_AppendHandshake(ss, pub_bytes, CHANNEL_ID_PUBLIC_KEY_LENGTH);
    if (rv != SECSuccess)
        goto loser;
    rv = ssl3_AppendHandshake(ss, signature, sizeof(signature));

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

Thanks!

+
+loser:
+    if (spki)
+        SECITEM_FreeItem(spki, PR_TRUE);
+
+    return rv;
+}
+
+/* called from ssl3_HandleServerHelloDone
  *             ssl3_HandleClientHello
  *             ssl3_HandleFinished
  */
 static SECStatus
 ssl3_SendFinished(sslSocket *ss, PRInt32 flags)
 {
     ssl3CipherSpec *cwSpec;
     PRBool          isTLS;
     PRBool          isServer = ss->sec.isServer;
     SECStatus       rv;
@@ skipping 234 matching lines @@
         */
         if (ss->writerThread == PR_GetCurrentThread()) {
             flags = ssl_SEND_FLAG_FORCE_INTO_BUFFER;
         }
 
         if (!isServer && !ss->firstHsDone) {
             rv = ssl3_SendNextProto(ss);
             if (rv != SECSuccess) {
                 goto xmit_loser; /* err code was set. */
             }
+            rv = ssl3_SendEncryptedExtensions(ss);

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

This means Channel ID must not be used in renegotiation,
right?

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

It did, but that is no longer correct. I've moved the two calls outside of if
(!ss->firstHsDone) so that they are sent on renegotiation.

+            if (rv != SECSuccess) {
+                goto xmit_loser; /* err code was set. */
+            }
         }
 
         if (IS_DTLS(ss)) {
             flags |= ssl_SEND_FLAG_NO_RETRANSMIT;
         }
 
         rv = ssl3_SendFinished(ss, flags);
         if (rv != SECSuccess) {
             goto xmit_loser;    /* err is set. */
         }
@@ skipping 1285 matching lines @@
         SECITEM_FreeItem(&ss->ssl3.hs.cert_status, PR_FALSE);
     }
 
     /* free the SSL3Buffer (msg_body) */
     PORT_Free(ss->ssl3.hs.msg_body.buf);
 
     /* free up the CipherSpecs */
     ssl3_DestroyCipherSpec(&ss->ssl3.specs[0], PR_TRUE/*freeSrvName*/);
     ssl3_DestroyCipherSpec(&ss->ssl3.specs[1], PR_TRUE/*freeSrvName*/);
 
+    if (ss->ssl3.channelID)
+        SECKEY_DestroyPrivateKey(ss->ssl3.channelID);
+    if (ss->ssl3.channelIDPub)
+        SECKEY_DestroyPublicKey(ss->ssl3.channelIDPub);

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

I suggest adding these right after line 10499 (where we
destroy ss->ssl3.clientCertChain) because this is similar
to client certificates.

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

Done.

+
     /* Destroy the DTLS data */
     if (IS_DTLS(ss)) {
         if (ss->ssl3.hs.lastMessageFlight) {
             dtls_FreeHandshakeMessages(ss->ssl3.hs.lastMessageFlight);
             PORT_Free(ss->ssl3.hs.lastMessageFlight);
         }
         if (ss->ssl3.hs.recvdFragments.buf) {
             PORT_Free(ss->ssl3.hs.recvdFragments.buf);
         }
     }
 
     ss->ssl3.initialized = PR_FALSE;
 
     SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE);
 }
 
 /* End of ssl3con.c */