Bug 1385203 - Use sslBuffer for encoding more widely, r=ekr

This removes ssl3_Append[Number]ToItem and switches more places to using
buffers.  The advantage with that, aside from a consistent interface, is that
encoding using sslBuffer is length checked.  A new field is added to the struct
so that it can be used for encoding directly onto the stack rather than relying
on reallocation when the space limit is reached.

New macros are added so that the internals of sslBuffer aren't accessed
directly.  Not all instances of these accesses have been fixed.

Differential Revision: https://nss-review.dev.mozaws.net/D390

--HG--
branch : NSS_TLS13_DRAFT19_BRANCH
extra : rebase_source : 60595ab46e30d80dc8bd13a341053edd5cd907b6
extra : amend_source : 25edd67ac2d8bda91a4f927c8671cabd257ccba7

gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -758,12 +758,14 @@ TEST_F(TlsConnectTest, SendSessionTicketMassiveToken) {
   Connect();
   // It should be safe to set length with a NULL token because the length should
   // be checked before reading token.
-  EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0xffff))
-      << "no special tickets in TLS 1.2";
+  EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0x1ffff))
+      << "this is clearly too big";
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 
-  EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), NULL, 0x1ffff))
-      << "no special tickets in TLS 1.2";
+  static const uint8_t big_token[0xffff] = {1};
+  EXPECT_EQ(SECFailure, SSL_SendSessionTicket(server_->ssl_fd(), big_token,
+                                              sizeof(big_token)))
+      << "this is too big, but that's not immediately obvious";
   EXPECT_EQ(SEC_ERROR_INVALID_ARGS, PORT_GetError());
 }
 

lib/ssl/selfencrypt.c

@@ -121,12 +121,11 @@ ssl_SelfEncryptProtectInt(
     PRUint8 *out, unsigned int *outLen, unsigned int maxOutLen)
 {
     unsigned int len;
+    unsigned int lenOffset;
     unsigned char iv[AES_BLOCK_SIZE];
     SECItem ivItem = { siBuffer, iv, sizeof(iv) };
-    unsigned char mac[SHA256_LENGTH]; /* SHA-256 */
-    unsigned int macLen;
-    SECItem outItem = { siBuffer, out, maxOutLen };
-    SECItem lengthBytesItem;
+    /* Write directly to out. */
+    sslBuffer buf = SSL_BUFFER_FIXED(out, maxOutLen);
     SECStatus rv;
 
     /* Generate a random IV */
@@ -137,52 +136,54 @@ ssl_SelfEncryptProtectInt(
     }
 
     /* Add header. */
-    rv = ssl3_AppendToItem(&outItem, keyName, SELF_ENCRYPT_KEY_NAME_LEN);
+    rv = sslBuffer_Append(&buf, keyName, SELF_ENCRYPT_KEY_NAME_LEN);
     if (rv != SECSuccess) {
         return SECFailure;
     }
-    rv = ssl3_AppendToItem(&outItem, iv, sizeof(iv));
+    rv = sslBuffer_Append(&buf, iv, sizeof(iv));
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    /* Skip forward by two so we can encode the ciphertext in place. */
-    lengthBytesItem = outItem;
-    rv = ssl3_AppendNumberToItem(&outItem, 0, 2);
+    /* Leave space for the length of the ciphertext. */
+    rv = sslBuffer_Skip(&buf, 2, &lenOffset);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
+    /* Encode the ciphertext in place. */
     rv = PK11_Encrypt(encKey, CKM_AES_CBC_PAD, &ivItem,
-                      outItem.data, &len, outItem.len, in, inLen);
+                      SSL_BUFFER_NEXT(&buf), &len,
+                      SSL_BUFFER_SPACE(&buf), in, inLen);
+    if (rv != SECSuccess) {
+        return SECFailure;
+    }
+    rv = sslBuffer_Skip(&buf, len, NULL);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    outItem.data += len;
-    outItem.len -= len;
-
-    /* Now encode the ciphertext length. */
-    rv = ssl3_AppendNumberToItem(&lengthBytesItem, len, 2);
+    rv = sslBuffer_InsertLength(&buf, lenOffset, 2);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    /* MAC the entire output buffer and append the MAC to the end. */
+    /* MAC the entire output buffer into the output. */
+    PORT_Assert(buf.space - buf.len >= SHA256_LENGTH);
     rv = ssl_MacBuffer(macKey, CKM_SHA256_HMAC,
-                       out, outItem.data - out,
-                       mac, &macLen, sizeof(mac));
+                       SSL_BUFFER_BASE(&buf), /* input */
+                       SSL_BUFFER_LEN(&buf),
+                       SSL_BUFFER_NEXT(&buf), &len, /* output */
+                       SHA256_LENGTH);
     if (rv != SECSuccess) {
         return SECFailure;
     }
-    PORT_Assert(macLen == sizeof(mac));
-
-    rv = ssl3_AppendToItem(&outItem, mac, macLen);
+    rv = sslBuffer_Skip(&buf, len, NULL);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
-    *outLen = outItem.data - out;
+    *outLen = SSL_BUFFER_LEN(&buf);
     return SECSuccess;
 }
 
@@ -270,16 +271,14 @@ ssl_SelfEncryptUnprotectInt(
 #endif
 
 /* Predict the size of the encrypted data, including padding */
-SECStatus
-ssl_SelfEncryptGetProtectedSize(unsigned int inLen, unsigned int *outLen)
+unsigned int
+ssl_SelfEncryptGetProtectedSize(unsigned int inLen)
 {
-    *outLen = SELF_ENCRYPT_KEY_NAME_LEN +
+    return SELF_ENCRYPT_KEY_NAME_LEN +
            AES_BLOCK_SIZE +
            2 +
            ((inLen / AES_BLOCK_SIZE) + 1) * AES_BLOCK_SIZE + /* Padded */
            SHA256_LENGTH;
-
-    return SECSuccess;
 }
 
 SECStatus

lib/ssl/selfencrypt.h

@@ -13,8 +13,7 @@
 
 typedef struct sslSocketStr sslSocket;
 
-SECStatus ssl_SelfEncryptGetProtectedSize(unsigned int inLen,
-                                          unsigned int *outLen);
+unsigned int ssl_SelfEncryptGetProtectedSize(unsigned int inLen);
 SECStatus ssl_SelfEncryptProtect(
     sslSocket *ss, const PRUint8 *in, unsigned int inLen,
     PRUint8 *out, unsigned int *outLen, unsigned int maxOutLen);