net: add ChaCha20+Poly1305 support to libssl.

net/third_party/nss/ssl/ssl3con.c

Index: net/third_party/nss/ssl/ssl3con.c
diff --git a/net/third_party/nss/ssl/ssl3con.c b/net/third_party/nss/ssl/ssl3con.c
index 8be517cde67c2bc388be107e322ac3f4e765fd32..74e9a5657a1a97e9e8f8644c4701d6a38d3ab5ad 100644
--- a/net/third_party/nss/ssl/ssl3con.c
+++ b/net/third_party/nss/ssl/ssl3con.c
@@ -40,6 +40,20 @@
 #define CKM_NSS_TLS_MASTER_KEY_DERIVE_DH_SHA256 (CKM_NSS + 24)
 #endif
 
+/* This is a bodge to allow this code to be compiled against older NSS
+ * headers. */
+#ifndef CKM_NSS_CHACHA20_POLY1305
+#define CKM_NSS_CHACHA20_POLY1305               (CKM_NSS + 25)
+
+typedef struct CK_AEAD_PARAMS {
+  CK_BYTE_PTR  pIv;  /* This is the nonce. */
+  CK_ULONG     ulIvLen;
+  CK_BYTE_PTR  pAAD;
+  CK_ULONG     ulAADLen;
+} CK_AEAD_PARAMS;

[wtc, 2013/09/13 17:29:14]

This differs from CK_GCM_PARAMS only in the ulTagBits field. Do you think the
a ulTagBits field could be useful?

[agl, 2013/09/13 20:51:45]

I don't plan on truncating the MAC, but others might. Even so, it would seem odd
to use CK_GCM_PARAMS with other algorithms. Would you prefer that the code
support ulTagBits in order to keep the structs the same? If so, you would like
to use CK_GCM_PARAMS for ChaCha, or typedef CK_GCM_PARAMS CK_AEAD_PARAMS?

[wtc, 2013/09/13 21:32:26]

For the struct to be named CK_AEAD_PARAMS, it should be
general enough to support all AEAD algorithms. This is why
I compared CK_AEAD_PARAMS with CK_GCM_PARAMS and noticed
that it is missing ulTagBit.

I just checked CK_CCM_PARAMS and found it also has a
ulDataLen field (the other fields seem equivalent to the
fields of CK_GCM_PARAMS even though the names are different).

1517 /* CK_CCM_PARAMS is new for version 2.30 */
1518 typedef struct CK_CCM_PARAMS {
1519   CK_ULONG     ulDataLen;
1520   CK_BYTE_PTR  pNonce;
1521   CK_ULONG     ulNonceLen;
1522   CK_BYTE_PTR  pAAD;
1523   CK_ULONG     ulAADLen;
1524   CK_ULONG     ulMACLen;
1525 } CK_CCM_PARAMS;

So, I would prefer that we either have a CK_AEAD_PARAMS
that is general, or just have a CK_CHACHA20_POLY1305_PARAMS.

[agl, 2013/09/16 22:19:07]

I've added the ulTagBit member.

+
+#endif
+
 #include <stdio.h>
 #ifdef NSS_ENABLE_ZLIB
 #include "zlib.h"
@@ -100,6 +114,8 @@ static SECStatus ssl3_AESGCMBypass(ssl3KeyMaterial *keys, PRBool doDecrypt,
 static ssl3CipherSuiteCfg cipherSuites[ssl_V3_SUITES_IMPLEMENTED] = {
    /*      cipher_suite                         policy      enabled is_present*/
 #ifdef NSS_ENABLE_ECC
+ { TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, SSL_NOT_ALLOWED, PR_TRUE, PR_FALSE},
+ { TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,   SSL_NOT_ALLOWED, PR_TRUE, PR_FALSE},

[wtc, 2013/09/13 17:29:14]

The |enabled| setting for the two CHACHA20_POLY1305 ciphers should be PR_FALSE
because these are ECC ciphers.

The CHACHA20_POLY1305 ciphers are listed before AES_128_GCM ciphers. I assume
this is because CHACHA20_POLY1305 uses 256-bit keys.

[agl, 2013/09/13 20:51:45]

Done.
Initially it's because I wanted to test but I do think that the ChaCha ciphers
are safer than AES_GCM because not all systems have AES-NI. With optimisation
enabled, they run at about 50% the speed of AES-GCM with AES-NI.

In the future I would like the AES_GCM ciphers to be listed first in cases where
the system has AES-NI. (Perhaps AES256 as, with AES-NI, that's not a major
performance issue.) Servers would need to be able to select different ciphers
depending on the client's cipher preference without always respecting the client
cipher preference, so it's a little complex. For now however, ChaCha is a safe
choice as a first-preference cipher.

  { TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,SSL_NOT_ALLOWED, PR_FALSE,PR_FALSE},
  { TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,  SSL_NOT_ALLOWED, PR_FALSE,PR_FALSE},
 #endif /* NSS_ENABLE_ECC */
@@ -273,6 +289,7 @@ static const ssl3BulkCipherDef bulk_cipher_defs[] = {
     {cipher_camellia_256, calg_camellia,    32,32, type_block, 16,16, 0, 0},
     {cipher_seed,         calg_seed,        16,16, type_block, 16,16, 0, 0},
     {cipher_aes_128_gcm,  calg_aes_gcm,     16,16, type_aead,   4, 0,16, 8},
+    {cipher_c20p1305,     calg_c20p1305,    32,32, type_aead,   0, 0,16, 0},
     {cipher_missing,      calg_null,         0, 0, type_stream, 0, 0, 0, 0},
 };
 
@@ -399,6 +416,8 @@ static const ssl3CipherSuiteDef cipher_suite_defs[] =
     {TLS_RSA_WITH_AES_128_GCM_SHA256, cipher_aes_128_gcm, mac_aead, kea_rsa},
     {TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, cipher_aes_128_gcm, mac_aead, kea_ecdhe_rsa},
     {TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, cipher_aes_128_gcm, mac_aead, kea_ecdhe_ecdsa},
+    {TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, cipher_c20p1305, mac_null, kea_ecdhe_ecdsa},
+    {TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, cipher_c20p1305, mac_null, kea_ecdhe_rsa},

[wtc, 2013/09/13 17:29:14]

Use mac_aead instead of mac_null.

Nit: list ECDHE_RSA before ECDHE_ECDSA, matching the {0xcc, 0x13}, {0xcc, 0x14}
order.

[agl, 2013/09/13 20:51:45]

Done.

 
 #ifdef NSS_ENABLE_ECC
     {TLS_ECDH_ECDSA_WITH_NULL_SHA,        cipher_null, mac_sha, kea_ecdh_ecdsa},
@@ -464,6 +483,7 @@ static const SSLCipher2Mech alg2Mech[] = {
     { calg_camellia , CKM_CAMELLIA_CBC			},
     { calg_seed     , CKM_SEED_CBC			},
     { calg_aes_gcm  , CKM_AES_GCM			},
+    { calg_c20p1305 , CKM_NSS_CHACHA20_POLY1305		},
 /*  { calg_init     , (CK_MECHANISM_TYPE)0x7fffffffL    }  */
 };
 
@@ -2020,6 +2040,45 @@ ssl3_AESGCMBypass(ssl3KeyMaterial *keys,
 }
 #endif
 
+static SECStatus
+ssl3_ChaCha20Poly1305(
+	ssl3KeyMaterial *keys,
+	PRBool doDecrypt,
+	unsigned char *out,
+	int *outlen,
+	int maxout,
+	const unsigned char *in,
+	int inlen,
+	const unsigned char *additionalData,
+	int additionalDataLen)
+{
+    SECItem            param;
+    SECStatus          rv = SECFailure;
+    static const int   tagSize = 16;

[wtc, 2013/09/13 17:29:14]

Delete tagSize. It's not used.

[agl, 2013/09/13 20:51:45]

Done.

+    unsigned int       uOutLen;
+    CK_AEAD_PARAMS     aeadParams;
+
+    memset(&param, 0, sizeof(param));
+    param.len = sizeof(CK_AEAD_PARAMS);

[wtc, 2013/09/13 17:29:14]

Nit: use sizeof(aeadParams).

Nit: SECItem has just three fields. In NSS we usually just assign them without
using a memset to zero the struct first:

  param.type = siBuffer;
  param.data = (unsigned char *) &aeadParams;
  param.len = sizeof(aeadParams);

[agl, 2013/09/13 20:51:45]

Done.

+    param.data = (unsigned char *) &aeadParams;
+    memset(&aeadParams, 0, sizeof(CK_AEAD_PARAMS));
+    aeadParams.pIv = (unsigned char *) additionalData;
+    aeadParams.ulIvLen = 8;
+    aeadParams.pAAD = (unsigned char *) additionalData;
+    aeadParams.ulAADLen = additionalDataLen;
+
+    if (doDecrypt) {
+	rv = pk11_decrypt(keys->write_key, CKM_NSS_CHACHA20_POLY1305, &param,
+			  out, &uOutLen, maxout, in, inlen);
+    } else {
+	rv = pk11_encrypt(keys->write_key, CKM_NSS_CHACHA20_POLY1305, &param,
+			  out, &uOutLen, maxout, in, inlen);
+    }
+    *outlen = (int) uOutLen;
+
+    return rv;
+}
+
 /* Initialize encryption and MAC contexts for pending spec.
  * Master Secret already is derived.
  * Caller holds Spec write lock.
@@ -2053,13 +2112,17 @@ ssl3_InitPendingContextsPKCS11(sslSocket *ss)
     pwSpec->client.write_mac_context = NULL;
     pwSpec->server.write_mac_context = NULL;
 
-    if (calg == calg_aes_gcm) {
+    if (calg == calg_aes_gcm || calg == calg_c20p1305) {

[wtc, 2013/09/13 17:29:14]

We should have a more systematic way to set the pwSpec->aead function pointer
if AEAD ciphers proliferate.

 	pwSpec->encode = NULL;
 	pwSpec->decode = NULL;
 	pwSpec->destroy = NULL;
 	pwSpec->encodeContext = NULL;
 	pwSpec->decodeContext = NULL;
-	pwSpec->aead = ssl3_AESGCM;
+	if (calg == calg_aes_gcm) {
+	    pwSpec->aead = ssl3_AESGCM;
+	} else {
+	    pwSpec->aead = ssl3_ChaCha20Poly1305;
+	}
 	return SECSuccess;
     }