From a9aee608c601e6aa240a99db1daa5cb29ee14680 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Tue, 1 Aug 2017 21:21:38 +1000 Subject: [PATCH] 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 | 10 +- lib/ssl/selfencrypt.c | 61 ++- lib/ssl/selfencrypt.h | 3 +- lib/ssl/ssl3con.c | 539 ++++++++++---------- lib/ssl/ssl3ecc.c | 20 +- lib/ssl/ssl3exthandle.c | 164 ++---- lib/ssl/sslencode.c | 196 +++++-- lib/ssl/sslencode.h | 46 +- lib/ssl/sslimpl.h | 14 +- lib/ssl/tls13con.c | 53 +- lib/ssl/tls13con.h | 3 +- lib/ssl/tls13exthandle.c | 33 +- lib/ssl/tls13hashstate.c | 32 +- lib/ssl/tls13hkdf.c | 50 +- 14 files changed, 656 insertions(+), 568 deletions(-) diff --git a/gtests/ssl_gtest/ssl_resumption_unittest.cc b/gtests/ssl_gtest/ssl_resumption_unittest.cc index 2f14b1cea6..f780e3b235 100644 --- a/gtests/ssl_gtest/ssl_resumption_unittest.cc +++ b/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()); } diff --git a/lib/ssl/selfencrypt.c b/lib/ssl/selfencrypt.c index d00ddf7d9a..a48bd0acab 100644 --- a/lib/ssl/selfencrypt.c +++ b/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 + - AES_BLOCK_SIZE + - 2 + - ((inLen / AES_BLOCK_SIZE) + 1) * AES_BLOCK_SIZE + /* Padded */ - SHA256_LENGTH; - - return SECSuccess; + return SELF_ENCRYPT_KEY_NAME_LEN + + AES_BLOCK_SIZE + + 2 + + ((inLen / AES_BLOCK_SIZE) + 1) * AES_BLOCK_SIZE + /* Padded */ + SHA256_LENGTH; } SECStatus diff --git a/lib/ssl/selfencrypt.h b/lib/ssl/selfencrypt.h index d65d3e5b27..1ca13b06ba 100644 --- a/lib/ssl/selfencrypt.h +++ b/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); diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 5ef7cf02c2..e19ae38ac1 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -70,9 +70,6 @@ static CK_MECHANISM_TYPE ssl3_GetHashMechanismByHashType(SSLHashType hashType); static CK_MECHANISM_TYPE ssl3_GetMgfMechanismByHashType(SSLHashType hash); PRBool ssl_IsRsaPssSignatureScheme(SSLSignatureScheme scheme); -#define MAX_SEND_BUF_LENGTH 32000 /* watch for 16-bit integer overflow */ -#define MIN_SEND_BUF_LENGTH 4000 - /* This list of SSL3 cipher suites is sorted in descending order of * precedence (desirability). It only includes cipher suites we implement. * This table is modified by SSL3_SetPolicy(). The ordering of cipher suites @@ -1431,69 +1428,76 @@ static SECStatus ssl3_ComputeDHKeyHash(sslSocket *ss, SSLHashType hashAlg, SSL3Hashes *hashes, SECItem dh_p, SECItem dh_g, SECItem dh_Ys, PRBool padY) { - PRUint8 *hashBuf; - PRUint8 *pBuf; - SECStatus rv = SECSuccess; - unsigned int bufLen, yLen; - PRUint8 buf[2 * SSL3_RANDOM_LENGTH + 2 + 4096 / 8 + 2 + 4096 / 8]; + sslBuffer buf = SSL_BUFFER_EMPTY; + SECStatus rv; + unsigned int yLen; + unsigned int i; PORT_Assert(dh_p.data); PORT_Assert(dh_g.data); PORT_Assert(dh_Ys.data); - yLen = padY ? dh_p.len : dh_Ys.len; - bufLen = 2 * SSL3_RANDOM_LENGTH + - 2 + dh_p.len + - 2 + dh_g.len + - 2 + yLen; - if (bufLen <= sizeof buf) { - hashBuf = buf; - } else { - hashBuf = PORT_Alloc(bufLen); - if (!hashBuf) { - return SECFailure; - } + rv = sslBuffer_Append(&buf, &ss->ssl3.hs.client_random, SSL3_RANDOM_LENGTH); + if (rv != SECSuccess) { + goto loser; } - - memcpy(hashBuf, &ss->ssl3.hs.client_random, SSL3_RANDOM_LENGTH); - pBuf = hashBuf + SSL3_RANDOM_LENGTH; - memcpy(pBuf, &ss->ssl3.hs.server_random, SSL3_RANDOM_LENGTH); - pBuf += SSL3_RANDOM_LENGTH; - pBuf = ssl_EncodeUintX(dh_p.len, 2, pBuf); - memcpy(pBuf, dh_p.data, dh_p.len); - pBuf += dh_p.len; - pBuf = ssl_EncodeUintX(dh_g.len, 2, pBuf); - memcpy(pBuf, dh_g.data, dh_g.len); - pBuf += dh_g.len; - pBuf = ssl_EncodeUintX(yLen, 2, pBuf); - if (padY && dh_p.len > dh_Ys.len) { - memset(pBuf, 0, dh_p.len - dh_Ys.len); - pBuf += dh_p.len - dh_Ys.len; + rv = sslBuffer_Append(&buf, &ss->ssl3.hs.server_random, SSL3_RANDOM_LENGTH); + if (rv != SECSuccess) { + goto loser; + } + /* p */ + rv = sslBuffer_AppendVariable(&buf, dh_p.data, dh_p.len, 2); + if (rv != SECSuccess) { + goto loser; + } + /* g */ + rv = sslBuffer_AppendVariable(&buf, dh_g.data, dh_g.len, 2); + if (rv != SECSuccess) { + goto loser; + } + /* y - complicated by padding */ + yLen = padY ? dh_p.len : dh_Ys.len; + rv = sslBuffer_AppendNumber(&buf, yLen, 2); + if (rv != SECSuccess) { + goto loser; } /* If we're padding Y, dh_Ys can't be longer than dh_p. */ PORT_Assert(!padY || dh_p.len >= dh_Ys.len); - memcpy(pBuf, dh_Ys.data, dh_Ys.len); - pBuf += dh_Ys.len; - PORT_Assert((unsigned int)(pBuf - hashBuf) == bufLen); + for (i = dh_Ys.len; i < yLen; ++i) { + rv = sslBuffer_AppendNumber(&buf, 0, 1); + if (rv != SECSuccess) { + goto loser; + } + } + rv = sslBuffer_Append(&buf, dh_Ys.data, dh_Ys.len); + if (rv != SECSuccess) { + goto loser; + } - rv = ssl3_ComputeCommonKeyHash(hashAlg, hashBuf, bufLen, hashes); + rv = ssl3_ComputeCommonKeyHash(hashAlg, SSL_BUFFER_BASE(&buf), + SSL_BUFFER_LEN(&buf), hashes); + if (rv != SECSuccess) { + goto loser; + } - PRINT_BUF(95, (NULL, "DHkey hash: ", hashBuf, bufLen)); - if (rv == SECSuccess) { - if (hashAlg == ssl_hash_none) { - PRINT_BUF(95, (NULL, "DHkey hash: MD5 result", - hashes->u.s.md5, MD5_LENGTH)); - PRINT_BUF(95, (NULL, "DHkey hash: SHA1 result", - hashes->u.s.sha, SHA1_LENGTH)); - } else { - PRINT_BUF(95, (NULL, "DHkey hash: result", - hashes->u.raw, hashes->len)); - } + PRINT_BUF(95, (NULL, "DHkey hash: ", SSL_BUFFER_BASE(&buf), + SSL_BUFFER_LEN(&buf))); + if (hashAlg == ssl_hash_none) { + PRINT_BUF(95, (NULL, "DHkey hash: MD5 result", + hashes->u.s.md5, MD5_LENGTH)); + PRINT_BUF(95, (NULL, "DHkey hash: SHA1 result", + hashes->u.s.sha, SHA1_LENGTH)); + } else { + PRINT_BUF(95, (NULL, "DHkey hash: result", + hashes->u.raw, hashes->len)); } - if (hashBuf != buf && hashBuf != NULL) - PORT_Free(hashBuf); - return rv; + sslBuffer_Clear(&buf); + return SECSuccess; + +loser: + sslBuffer_Clear(&buf); + return SECFailure; } /* Called twice, only from ssl3_DestroyCipherSpec (immediately below). */ @@ -2544,24 +2548,71 @@ ssl3_CompressMACEncryptRecord(ssl3CipherSpec *cwSpec, return SECSuccess; } +/* Note: though this can report failure, it shouldn't. */ +SECStatus +ssl_InsertRecordHeader(const sslSocket *ss, ssl3CipherSpec *cwSpec, + PRBool capRecordVersion, SSL3ContentType type, + unsigned int len, sslBuffer *wrBuf) +{ + SSL3ProtocolVersion version = cwSpec->version; + PRBool isTLS13 = (PRBool)(version >= SSL_LIBRARY_VERSION_TLS_1_3); + SECStatus rv; + +#ifndef UNSAFE_FUZZER_MODE + if (isTLS13 && cwSpec->cipher_def->calg != ssl_calg_null) { + rv = sslBuffer_AppendNumber(wrBuf, content_application_data, 1); + } else +#endif + { + rv = sslBuffer_AppendNumber(wrBuf, type, 1); + } + if (rv != SECSuccess) { + return SECFailure; + } + + if (IS_DTLS(ss)) { + version = isTLS13 ? SSL_LIBRARY_VERSION_TLS_1_1 : version; + version = dtls_TLSVersionToDTLSVersion(version); + + rv = sslBuffer_AppendNumber(wrBuf, version, 2); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_AppendNumber(wrBuf, cwSpec->write_seq_num, 8); + if (rv != SECSuccess) { + return SECFailure; + } + } else { + if (capRecordVersion || isTLS13) { + version = PR_MIN(SSL_LIBRARY_VERSION_TLS_1_0, version); + } + rv = sslBuffer_AppendNumber(wrBuf, version, 2); + if (rv != SECSuccess) { + return SECFailure; + } + } + rv = sslBuffer_AppendNumber(wrBuf, len, 2); + if (rv != SECSuccess) { + return SECFailure; + } + + return SECSuccess; +} + SECStatus ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, PRBool capRecordVersion, SSL3ContentType type, const PRUint8 *pIn, PRUint32 contentLen, sslBuffer *wrBuf) { const ssl3BulkCipherDef *cipher_def = cwSpec->cipher_def; - PRUint16 headerLen; - sslBuffer protBuf; - SSL3ProtocolVersion version = cwSpec->version; + unsigned int headerLen = IS_DTLS(ss) ? DTLS_RECORD_HEADER_LENGTH + : SSL3_RECORD_HEADER_LENGTH; + sslBuffer protBuf = SSL_BUFFER_FIXED(SSL_BUFFER_BASE(wrBuf) + headerLen, + SSL_BUFFER_SPACE(wrBuf) - headerLen); PRBool isTLS13; - PRUint8 *ptr = wrBuf->buf; SECStatus rv; - headerLen = IS_DTLS(ss) ? DTLS_RECORD_HEADER_LENGTH : SSL3_RECORD_HEADER_LENGTH; - protBuf.buf = wrBuf->buf + headerLen; - protBuf.len = 0; - protBuf.space = wrBuf->space - headerLen; - + PORT_Assert(SSL_BUFFER_LEN(wrBuf) == 0); PORT_Assert(cipher_def->max_records <= RECORD_SEQ_MAX); if ((cwSpec->write_seq_num & RECORD_SEQ_MAX) >= cipher_def->max_records) { SSL_TRC(3, ("%d: SSL[-]: write sequence number at limit 0x%0llx", @@ -2573,8 +2624,16 @@ ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, isTLS13 = (PRBool)(cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_3); #ifdef UNSAFE_FUZZER_MODE - rv = Null_Cipher(NULL, protBuf.buf, (int *)&protBuf.len, protBuf.space, - pIn, contentLen); + { + int len; + rv = Null_Cipher(NULL, SSL_BUFFER_BASE(&protBuf), &len, + SSL_BUFFER_SPACE(&protBuf), pIn, contentLen); + if (rv != SECSuccess) { + return SECFailure; /* error was set */ + } + rv = sslBuffer_Skip(&protBuf, len, NULL); + PORT_Assert(rv == SECSuccess); /* Can't fail. */ + } #else if (isTLS13) { rv = tls13_ProtectRecord(ss, cwSpec, type, pIn, contentLen, &protBuf); @@ -2589,32 +2648,113 @@ ssl_ProtectRecord(sslSocket *ss, ssl3CipherSpec *cwSpec, } PORT_Assert(protBuf.len <= MAX_FRAGMENT_LENGTH + (isTLS13 ? 256 : 1024)); - wrBuf->len = protBuf.len + headerLen; -#ifndef UNSAFE_FUZZER_MODE - if (isTLS13 && cipher_def->calg != ssl_calg_null) { - *ptr++ = content_application_data; - } else -#endif - { - *ptr++ = type; + rv = ssl_InsertRecordHeader(ss, cwSpec, capRecordVersion, type, + SSL_BUFFER_LEN(&protBuf), wrBuf); + if (rv != SECSuccess) { + return SECFailure; } - if (IS_DTLS(ss)) { - version = isTLS13 ? SSL_LIBRARY_VERSION_TLS_1_1 : version; - version = dtls_TLSVersionToDTLSVersion(version); + PORT_Assert(SSL_BUFFER_LEN(wrBuf) == headerLen); + rv = sslBuffer_Skip(wrBuf, SSL_BUFFER_LEN(&protBuf), NULL); + if (rv != SECSuccess) { + PORT_Assert(0); /* Can't fail. */ + return SECFailure; + } + ++cwSpec->write_seq_num; + + return SECSuccess; +} + +SECStatus +ssl_ProtectRecordMaybeSplit(sslSocket *ss, ssl3CipherSpec *cwSpec, + SSL3ContentType type, PRBool capRecordVersion, + const PRUint8 *pIn, unsigned int nIn, + unsigned int *written) +{ + sslBuffer *wrBuf = &ss->sec.writeBuf; + unsigned int contentLen = PR_MIN(nIn, MAX_FRAGMENT_LENGTH); + unsigned int spaceNeeded; + unsigned int numRecords; + SECStatus rv; - ptr = ssl_EncodeUintX(version, 2, ptr); - ptr = ssl_EncodeUintX(cwSpec->write_seq_num, 8, ptr); + if (nIn > 1 && ss->opt.cbcRandomIV && + ss->ssl3.cwSpec->version < SSL_LIBRARY_VERSION_TLS_1_1 && + type == content_application_data && + ss->ssl3.cwSpec->cipher_def->type == type_block /* CBC mode */) { + /* We will split the first byte of the record into its own record, + * as explained in the documentation for SSL_CBC_RANDOM_IV in ssl.h + */ + numRecords = 2; } else { - if (capRecordVersion || isTLS13) { - version = PR_MIN(SSL_LIBRARY_VERSION_TLS_1_0, version); + numRecords = 1; + } + + spaceNeeded = contentLen + (numRecords * SSL3_BUFFER_FUDGE); + if (ss->ssl3.cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_1 && + ss->ssl3.cwSpec->cipher_def->type == type_block) { + spaceNeeded += ss->ssl3.cwSpec->cipher_def->iv_size; + } + if (spaceNeeded > SSL_BUFFER_SPACE(wrBuf)) { + rv = sslBuffer_Grow(wrBuf, spaceNeeded); + if (rv != SECSuccess) { + SSL_DBG(("%d: SSL3[%d]: expand write buffer to %d bytes", + SSL_GETPID(), ss->fd, spaceNeeded)); + return SECFailure; } - ptr = ssl_EncodeUintX(version, 2, ptr); } - (void)ssl_EncodeUintX(protBuf.len, 2, ptr); - ++cwSpec->write_seq_num; + if (numRecords == 2) { + rv = ssl_ProtectRecord(ss, ss->ssl3.cwSpec, capRecordVersion, type, + pIn, 1, wrBuf); + if (rv != SECSuccess) { + return SECFailure; + } + + PRINT_BUF(50, (ss, "send (encrypted) record data [1/2]:", + SSL_BUFFER_BASE(wrBuf), SSL_BUFFER_LEN(wrBuf))); + + { + sslBuffer secondRecord = SSL_BUFFER_FIXED(SSL_BUFFER_NEXT(wrBuf), + SSL_BUFFER_SPACE(wrBuf)); + + rv = ssl_ProtectRecord(ss, ss->ssl3.cwSpec, capRecordVersion, type, + pIn + 1, contentLen - 1, &secondRecord); + if (rv != SECSuccess) { + return SECFailure; + } + PRINT_BUF(50, (ss, "send (encrypted) record data [2/2]:", + SSL_BUFFER_BASE(&secondRecord), + SSL_BUFFER_LEN(&secondRecord))); + rv = sslBuffer_Skip(wrBuf, SSL_BUFFER_LEN(&secondRecord), NULL); + if (rv != SECSuccess) { + return SECFailure; + } + } + } else { + ssl3CipherSpec *spec; + + if (cwSpec) { + /* cwSpec can only be set for retransmissions of DTLS handshake + * messages. */ + PORT_Assert(IS_DTLS(ss) && + (type == content_handshake || + type == content_change_cipher_spec)); + spec = cwSpec; + } else { + spec = ss->ssl3.cwSpec; + } + + rv = ssl_ProtectRecord(ss, spec, !IS_DTLS(ss) && capRecordVersion, + type, pIn, contentLen, wrBuf); + if (rv != SECSuccess) { + return SECFailure; + } + PRINT_BUF(50, (ss, "send (encrypted) record data:", + SSL_BUFFER_BASE(wrBuf), SSL_BUFFER_LEN(wrBuf))); + } + + *written = contentLen; return SECSuccess; } @@ -2661,7 +2801,6 @@ ssl3_SendRecord(sslSocket *ss, SECStatus rv; PRInt32 totalSent = 0; PRBool capRecordVersion; - ssl3CipherSpec *spec; SSL_TRC(3, ("%d: SSL3[%d] SendRecord type: %s nIn=%d", SSL_GETPID(), ss->fd, ssl3_DecodeContentType(type), @@ -2703,84 +2842,15 @@ ssl3_SendRecord(sslSocket *ss, } while (nIn > 0) { - PRUint32 contentLen = PR_MIN(nIn, MAX_FRAGMENT_LENGTH); - unsigned int spaceNeeded; - unsigned int numRecords; + unsigned int contentLen = 0; ssl_GetSpecReadLock(ss); /********************************/ - - if (nIn > 1 && ss->opt.cbcRandomIV && - ss->ssl3.cwSpec->version < SSL_LIBRARY_VERSION_TLS_1_1 && - type == content_application_data && - ss->ssl3.cwSpec->cipher_def->type == type_block /* CBC mode */) { - /* We will split the first byte of the record into its own record, - * as explained in the documentation for SSL_CBC_RANDOM_IV in ssl.h - */ - numRecords = 2; - } else { - numRecords = 1; - } - - spaceNeeded = contentLen + (numRecords * SSL3_BUFFER_FUDGE); - if (ss->ssl3.cwSpec->version >= SSL_LIBRARY_VERSION_TLS_1_1 && - ss->ssl3.cwSpec->cipher_def->type == type_block) { - spaceNeeded += ss->ssl3.cwSpec->cipher_def->iv_size; - } - if (spaceNeeded > wrBuf->space) { - rv = sslBuffer_Grow(wrBuf, spaceNeeded); - if (rv != SECSuccess) { - SSL_DBG(("%d: SSL3[%d]: SendRecord, tried to get %d bytes", - SSL_GETPID(), ss->fd, spaceNeeded)); - goto spec_locked_loser; /* sslBuffer_Grow set error code. */ - } - } - - if (numRecords == 2) { - sslBuffer secondRecord; - rv = ssl_ProtectRecord(ss, ss->ssl3.cwSpec, capRecordVersion, type, - pIn, 1, wrBuf); - if (rv != SECSuccess) - goto spec_locked_loser; - - PRINT_BUF(50, (ss, "send (encrypted) record data [1/2]:", - wrBuf->buf, wrBuf->len)); - - secondRecord.buf = wrBuf->buf + wrBuf->len; - secondRecord.len = 0; - secondRecord.space = wrBuf->space - wrBuf->len; - - rv = ssl_ProtectRecord(ss, ss->ssl3.cwSpec, capRecordVersion, type, - pIn + 1, contentLen - 1, &secondRecord); - if (rv == SECSuccess) { - PRINT_BUF(50, (ss, "send (encrypted) record data [2/2]:", - secondRecord.buf, secondRecord.len)); - wrBuf->len += secondRecord.len; - } - } else { - if (cwSpec) { - /* cwSpec can only be set for retransmissions of DTLS handshake - * messages. */ - PORT_Assert(IS_DTLS(ss) && - (type == content_handshake || - type == content_change_cipher_spec)); - spec = cwSpec; - } else { - spec = ss->ssl3.cwSpec; - } - - rv = ssl_ProtectRecord(ss, spec, !IS_DTLS(ss) && capRecordVersion, - type, pIn, contentLen, wrBuf); - if (rv == SECSuccess) { - PRINT_BUF(50, (ss, "send (encrypted) record data:", - wrBuf->buf, wrBuf->len)); - } - } - - spec_locked_loser: + rv = ssl_ProtectRecordMaybeSplit(ss, cwSpec, type, capRecordVersion, + pIn, nIn, &contentLen); ssl_ReleaseSpecReadLock(ss); /************************************/ - - if (rv != SECSuccess) + if (rv != SECSuccess) { return SECFailure; + } pIn += contentLen; nIn -= contentLen; @@ -2793,12 +2863,12 @@ ssl3_SendRecord(sslSocket *ss, if ((ss->pendingBuf.len > 0) || (flags & ssl_SEND_FLAG_FORCE_INTO_BUFFER)) { - rv = ssl_SaveWriteData(ss, wrBuf->buf, wrBuf->len); + rv = ssl_SaveWriteData(ss, SSL_BUFFER_BASE(wrBuf), + SSL_BUFFER_LEN(wrBuf)); if (rv != SECSuccess) { /* presumably a memory error, SEC_ERROR_NO_MEMORY */ return SECFailure; } - wrBuf->len = 0; /* All cipher text is saved away. */ if (!(flags & ssl_SEND_FLAG_FORCE_INTO_BUFFER)) { PRInt32 sent; @@ -2812,10 +2882,13 @@ ssl3_SendRecord(sslSocket *ss, flags |= ssl_SEND_FLAG_FORCE_INTO_BUFFER; } } - } else if (wrBuf->len > 0) { + } else { PRInt32 sent; + + PORT_Assert(SSL_BUFFER_LEN(wrBuf) > 0); ss->handshakeBegun = 1; - sent = ssl_DefSend(ss, wrBuf->buf, wrBuf->len, + sent = ssl_DefSend(ss, SSL_BUFFER_BASE(wrBuf), + SSL_BUFFER_LEN(wrBuf), flags & ~ssl_SEND_FLAG_MASK); if (sent < 0) { if (PR_GetError() != PR_WOULD_BLOCK_ERROR) { @@ -2825,8 +2898,7 @@ ssl3_SendRecord(sslSocket *ss, /* we got PR_WOULD_BLOCK_ERROR, which means none was sent. */ sent = 0; } - wrBuf->len -= sent; - if (wrBuf->len) { + if (SSL_BUFFER_LEN(wrBuf) > sent) { if (IS_DTLS(ss)) { /* DTLS just says no in this case. No buffering */ PR_SetError(PR_WOULD_BLOCK_ERROR, 0); @@ -2835,13 +2907,15 @@ ssl3_SendRecord(sslSocket *ss, /* now take all the remaining unsent new ciphertext and * append it to the buffer of previously unsent ciphertext. */ - rv = ssl_SaveWriteData(ss, wrBuf->buf + sent, wrBuf->len); + rv = ssl_SaveWriteData(ss, SSL_BUFFER_BASE(wrBuf) + sent, + SSL_BUFFER_LEN(wrBuf) - sent); if (rv != SECSuccess) { /* presumably a memory error, SEC_ERROR_NO_MEMORY */ return SECFailure; } } } + wrBuf->len = 0; totalSent += contentLen; } return totalSent; @@ -4147,96 +4221,6 @@ ssl3_UpdateHandshakeHashes(sslSocket *ss, const unsigned char *b, unsigned int l return rv; } -/************************************************************************** - * Append Handshake functions. - * All these functions set appropriate error codes. - * Most rely on ssl3_AppendHandshake to set the error code. - **************************************************************************/ -SECStatus -ssl3_AppendHandshake(sslSocket *ss, const void *void_src, unsigned int bytes) -{ - unsigned char *src = (unsigned char *)void_src; - int room = ss->sec.ci.sendBuf.space - ss->sec.ci.sendBuf.len; - SECStatus rv; - - PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); /* protects sendBuf. */ - - if (!bytes) - return SECSuccess; - if (ss->sec.ci.sendBuf.space < MAX_SEND_BUF_LENGTH && room < bytes) { - rv = sslBuffer_Grow(&ss->sec.ci.sendBuf, PR_MAX(MIN_SEND_BUF_LENGTH, - PR_MIN(MAX_SEND_BUF_LENGTH, ss->sec.ci.sendBuf.len + bytes))); - if (rv != SECSuccess) - return rv; /* sslBuffer_Grow has set a memory error code. */ - room = ss->sec.ci.sendBuf.space - ss->sec.ci.sendBuf.len; - } - - PRINT_BUF(60, (ss, "Append to Handshake", (unsigned char *)void_src, bytes)); - rv = ssl3_UpdateHandshakeHashes(ss, src, bytes); - if (rv != SECSuccess) - return rv; /* error code set by ssl3_UpdateHandshakeHashes */ - - while (bytes > room) { - if (room > 0) - PORT_Memcpy(ss->sec.ci.sendBuf.buf + ss->sec.ci.sendBuf.len, src, - room); - ss->sec.ci.sendBuf.len += room; - rv = ssl3_FlushHandshake(ss, ssl_SEND_FLAG_FORCE_INTO_BUFFER); - if (rv != SECSuccess) { - return rv; /* error code set by ssl3_FlushHandshake */ - } - bytes -= room; - src += room; - room = ss->sec.ci.sendBuf.space; - PORT_Assert(ss->sec.ci.sendBuf.len == 0); - } - PORT_Memcpy(ss->sec.ci.sendBuf.buf + ss->sec.ci.sendBuf.len, src, bytes); - ss->sec.ci.sendBuf.len += bytes; - return SECSuccess; -} - -SECStatus -ssl3_AppendHandshakeNumber(sslSocket *ss, PRUint64 num, unsigned int lenSize) -{ - PRUint8 b[sizeof(num)]; - SSL_TRC(60, ("%d: number:", SSL_GETPID())); - (void)ssl_EncodeUintX(num, lenSize, b); - return ssl3_AppendHandshake(ss, b, lenSize); -} - -SECStatus -ssl3_AppendHandshakeVariable( - sslSocket *ss, const PRUint8 *src, unsigned int bytes, unsigned int lenSize) -{ - SECStatus rv; - - PORT_Assert((bytes < (1 << 8) && lenSize == 1) || - (bytes < (1L << 16) && lenSize == 2) || - (bytes < (1L << 24) && lenSize == 3)); - - SSL_TRC(60, ("%d: append variable:", SSL_GETPID())); - rv = ssl3_AppendHandshakeNumber(ss, bytes, lenSize); - if (rv != SECSuccess) { - return rv; /* error code set by AppendHandshake, if applicable. */ - } - SSL_TRC(60, ("data:")); - rv = ssl3_AppendHandshake(ss, src, bytes); - return rv; /* error code set by AppendHandshake, if applicable. */ -} - -SECStatus -ssl3_AppendBufferToHandshake(sslSocket *ss, sslBuffer *buf) -{ - return ssl3_AppendHandshake(ss, buf->buf, buf->len); -} - -SECStatus -ssl3_AppendBufferToHandshakeVariable(sslSocket *ss, sslBuffer *buf, - unsigned int lenSize) -{ - return ssl3_AppendHandshakeVariable(ss, buf->buf, buf->len, lenSize); -} - SECStatus ssl3_AppendHandshakeHeader(sslSocket *ss, SSLHandshakeType t, PRUint32 length) { @@ -4951,7 +4935,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) PRBool isTLS = PR_FALSE; PRBool requestingResume = PR_FALSE, fallbackSCSV = PR_FALSE; PRBool unlockNeeded = PR_FALSE; - sslBuffer extensionBuf = { NULL, 0, 0 }; + sslBuffer extensionBuf = SSL_BUFFER_EMPTY; unsigned numCompressionMethods; PRUint16 version; PRInt32 flags; @@ -6108,7 +6092,8 @@ ssl3_SendDHClientKeyExchange(sslSocket *ss, SECKEYPublicKey *svrPubKey) }; sslEphemeralKeyPair *keyPair = NULL; SECKEYPublicKey *pubKey; - sslBuffer dhBuf = { NULL, 0, 0 }; + PRUint8 dhData[1026]; /* Enough for the 8192-bit group. */ + sslBuffer dhBuf = SSL_BUFFER(dhData); PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss)); @@ -9271,7 +9256,7 @@ ssl3_SendServerHello(sslSocket *ss) { sslSessionID *sid; SECStatus rv; - sslBuffer extensionBuf = { NULL, 0, 0 }; + sslBuffer extensionBuf = SSL_BUFFER_EMPTY; PRUint32 length; SSL3ProtocolVersion version; @@ -9430,7 +9415,8 @@ ssl3_SendDHServerKeyExchange(sslSocket *ss) SECKEYPublicKey *pubKey; SECKEYPrivateKey *certPrivateKey; const sslNamedGroupDef *groupDef; - sslBuffer dhBuf = { NULL, 0, 0 }; + /* Do this on the heap, this could be over 2k long. */ + sslBuffer dhBuf = SSL_BUFFER_EMPTY; if (kea_def->kea != kea_dhe_dss && kea_def->kea != kea_dhe_rsa) { /* TODO: Support DH_anon. It might be sufficient to drop the signature. @@ -9571,14 +9557,15 @@ ssl3_SendServerKeyExchange(sslSocket *ss) } SECStatus -ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint8 *buf, unsigned maxLen, PRUint32 *len) +ssl3_EncodeSigAlgs(const sslSocket *ss, sslBuffer *buf) { + unsigned int lengthOffset; unsigned int i; - PRUint8 *p = buf; + PRBool found = PR_FALSE; + SECStatus rv; - PORT_Assert(maxLen >= ss->ssl3.signatureSchemeCount * 2); - if (maxLen < ss->ssl3.signatureSchemeCount * 2) { - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + rv = sslBuffer_Skip(buf, 2, &lengthOffset); + if (rv != SECSuccess) { return SECFailure; } @@ -9596,16 +9583,21 @@ ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint8 *buf, unsigned maxLen, PRUint32 if ((NSS_GetAlgorithmPolicy(hashOID, &policy) != SECSuccess) || (policy & NSS_USE_ALG_IN_SSL_KX)) { - p = ssl_EncodeUintX((PRUint32)ss->ssl3.signatureSchemes[i], 2, p); + rv = sslBuffer_AppendNumber(buf, ss->ssl3.signatureSchemes[i], 2); + if (rv != SECSuccess) { + return SECFailure; + } + + found = PR_TRUE; } } - if (p == buf) { + if (!found) { PORT_SetError(SSL_ERROR_NO_SUPPORTED_SIGNATURE_ALGORITHM); return SECFailure; } - *len = p - buf; - return SECSuccess; + + return sslBuffer_InsertLength(buf, lengthOffset, 2); } static SECStatus @@ -9621,8 +9613,8 @@ ssl3_SendCertificateRequest(sslSocket *ss) const SECItem *name; int i; int certTypesLength; - PRUint8 sigAlgs[MAX_SIGNATURE_SCHEMES * 2]; - unsigned int sigAlgsLength = 0; + PRUint8 sigAlgs[2 + MAX_SIGNATURE_SCHEMES * 2]; + sslBuffer sigAlgsBuf = SSL_BUFFER(sigAlgs); SSL_TRC(3, ("%d: SSL3[%d]: send certificate_request handshake", SSL_GETPID(), ss->fd)); @@ -9641,11 +9633,11 @@ ssl3_SendCertificateRequest(sslSocket *ss) length = 1 + certTypesLength + 2 + calen; if (isTLS12) { - rv = ssl3_EncodeSigAlgs(ss, sigAlgs, sizeof(sigAlgs), &sigAlgsLength); + rv = ssl3_EncodeSigAlgs(ss, &sigAlgsBuf); if (rv != SECSuccess) { return rv; } - length += 2 + sigAlgsLength; + length += SSL_BUFFER_LEN(&sigAlgsBuf); } rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_certificate_request, length); @@ -9657,7 +9649,8 @@ ssl3_SendCertificateRequest(sslSocket *ss) return rv; /* err set by AppendHandshake. */ } if (isTLS12) { - rv = ssl3_AppendHandshakeVariable(ss, sigAlgs, sigAlgsLength, 2); + rv = ssl3_AppendHandshake(ss, SSL_BUFFER_BASE(&sigAlgsBuf), + SSL_BUFFER_LEN(&sigAlgsBuf)); if (rv != SECSuccess) { return rv; /* err set by AppendHandshake. */ } @@ -12468,7 +12461,7 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText, sslBuffer *databuf) ssl3CipherSpec *crSpec; SSL3ContentType rType; sslBuffer *plaintext; - sslBuffer temp_buf = { NULL, 0, 0 }; + sslBuffer temp_buf = SSL_BUFFER_EMPTY; /* for decompression */ SSL3AlertDescription alert = internal_error; PORT_Assert(ss->opt.noLocks || ssl_HaveRecvBufLock(ss)); diff --git a/lib/ssl/ssl3ecc.c b/lib/ssl/ssl3ecc.c index 85e2e2722d..75f29a415d 100644 --- a/lib/ssl/ssl3ecc.c +++ b/lib/ssl/ssl3ecc.c @@ -864,8 +864,9 @@ ssl_SendSupportedGroupsXtn(const sslSocket *ss, TLSExtensionData *xtnData, unsigned int i; PRBool ec; PRBool ff = PR_FALSE; + PRBool found = PR_FALSE; SECStatus rv; - sslBuffer tmpBuf = { NULL, 0, 0 }; + unsigned int lengthOffset; /* We only send FF supported groups if we require DH named groups * or if TLS 1.3 is a possibility. */ @@ -881,7 +882,12 @@ ssl_SendSupportedGroupsXtn(const sslSocket *ss, TLSExtensionData *xtnData, ec = ff = PR_TRUE; } - /* Reserve space for the length. */ + /* Mark the location of the length. */ + rv = sslBuffer_Skip(buf, 2, &lengthOffset); + if (rv != SECSuccess) { + return SECFailure; + } + for (i = 0; i < SSL_NAMED_GROUP_COUNT; ++i) { const sslNamedGroupDef *group = ss->namedGroupPreferences[i]; if (!group) { @@ -894,21 +900,19 @@ ssl_SendSupportedGroupsXtn(const sslSocket *ss, TLSExtensionData *xtnData, continue; } - rv = sslBuffer_AppendNumber(&tmpBuf, group->name, 2); + found = PR_TRUE; + rv = sslBuffer_AppendNumber(buf, group->name, 2); if (rv != SECSuccess) { - sslBuffer_Clear(&tmpBuf); return SECFailure; } } - if (!tmpBuf.len) { - sslBuffer_Clear(&tmpBuf); + if (!found) { /* We added nothing, don't send the extension. */ return SECSuccess; } - rv = sslBuffer_AppendBufferVariable(buf, &tmpBuf, 2); - sslBuffer_Clear(&tmpBuf); + rv = sslBuffer_InsertLength(buf, lengthOffset, 2); if (rv != SECSuccess) { return SECFailure; } diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c index 426096992c..a9c678184c 100644 --- a/lib/ssl/ssl3exthandle.c +++ b/lib/ssl/ssl3exthandle.c @@ -665,14 +665,11 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, PK11SymKey *secret, SECItem *ticket_data) { SECStatus rv; - SECItem plaintext; - SECItem plaintext_item = { 0, NULL, 0 }; - PRUint32 plaintext_length; + sslBuffer plaintext = { NULL, 0, 0 }; SECItem ticket_buf = { 0, NULL, 0 }; PRBool ms_is_wrapped; unsigned char wrapped_ms[SSL3_MASTER_SECRET_LENGTH]; SECItem ms_item = { 0, NULL, 0 }; - PRUint32 cert_length = 0; PRTime now; SECItem *srvName = NULL; CK_MECHANISM_TYPE msWrapMech = 0; /* dummy default value, @@ -686,10 +683,6 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss)); PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); - if (ss->opt.requestCertificate && ss->sec.ci.sid->peerCert) { - cert_length = 2 + ss->sec.ci.sid->peerCert->derCert.len; - } - if (ss->ssl3.pwSpec->msItem.len && ss->ssl3.pwSpec->msItem.data) { PORT_Assert(ss->version < SSL_LIBRARY_VERSION_TLS_1_3); /* The master secret is available unwrapped. */ @@ -721,79 +714,40 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, /* Prep to send negotiated name */ srvName = &ss->sec.ci.sid->u.ssl3.srvName; - PORT_Assert(ss->xtnData.nextProtoState == SSL_NEXT_PROTO_SELECTED || - ss->xtnData.nextProtoState == SSL_NEXT_PROTO_NEGOTIATED || - ss->xtnData.nextProto.len == 0); - alpnSelection = &ss->xtnData.nextProto; - - plaintext_length = - sizeof(PRUint16) /* ticket version */ - + sizeof(SSL3ProtocolVersion) /* ssl_version */ - + sizeof(ssl3CipherSuite) /* ciphersuite */ - + 1 /* compression */ - + 10 /* cipher spec parameters */ - + 1 /* certType arguments */ - + 1 /* SessionTicket.ms_is_wrapped */ - + 4 /* msWrapMech */ - + 2 /* master_secret.length */ - + ms_item.len /* master_secret */ - + 1 /* client_auth_type */ - + cert_length /* cert */ - + 2 + srvName->len /* name len + length field */ - + 1 /* extendedMasterSecretUsed */ - + sizeof(now) /* ticket lifetime hint */ - + sizeof(ticket->flags) /* ticket flags */ - + 1 + alpnSelection->len /* alpn value + length field */ - + 4 /* maxEarlyData */ - + 4 /* ticketAgeBaseline */ - + 2 + appTokenLen; /* application token */ - - /* This really only happens if appTokenLen is too much, and that always - * comes from the using application. */ - if (plaintext_length > 0xffff) { - PORT_SetError(SEC_ERROR_INVALID_ARGS); - goto loser; - } - - if (SECITEM_AllocItem(NULL, &plaintext_item, plaintext_length) == NULL) - goto loser; - - plaintext = plaintext_item; - /* ticket version */ - rv = ssl3_AppendNumberToItem(&plaintext, TLS_EX_SESS_TICKET_VERSION, - sizeof(PRUint16)); + rv = sslBuffer_AppendNumber(&plaintext, TLS_EX_SESS_TICKET_VERSION, + sizeof(PRUint16)); if (rv != SECSuccess) goto loser; /* ssl_version */ - rv = ssl3_AppendNumberToItem(&plaintext, ss->version, - sizeof(SSL3ProtocolVersion)); + rv = sslBuffer_AppendNumber(&plaintext, ss->version, + sizeof(SSL3ProtocolVersion)); if (rv != SECSuccess) goto loser; /* ciphersuite */ - rv = ssl3_AppendNumberToItem(&plaintext, ss->ssl3.hs.cipher_suite, - sizeof(ssl3CipherSuite)); + rv = sslBuffer_AppendNumber(&plaintext, ss->ssl3.hs.cipher_suite, + sizeof(ssl3CipherSuite)); if (rv != SECSuccess) goto loser; /* compression */ - rv = ssl3_AppendNumberToItem(&plaintext, ss->ssl3.hs.compression, 1); + rv = sslBuffer_AppendNumber(&plaintext, ss->ssl3.hs.compression, 1); if (rv != SECSuccess) goto loser; /* cipher spec parameters */ - rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.authType, 1); + rv = sslBuffer_AppendNumber(&plaintext, ss->sec.authType, 1); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.authKeyBits, 4); + rv = sslBuffer_AppendNumber(&plaintext, ss->sec.authKeyBits, 4); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.keaType, 1); + rv = sslBuffer_AppendNumber(&plaintext, ss->sec.keaType, 1); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, ss->sec.keaKeyBits, 4); + rv = sslBuffer_AppendNumber(&plaintext, ss->sec.keaKeyBits, 4); if (rv != SECSuccess) goto loser; @@ -804,43 +758,36 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, PORT_Assert(cert->namedCurve); /* EC curves only use the second of the two bytes. */ PORT_Assert(cert->namedCurve->name < 256); - rv = ssl3_AppendNumberToItem(&plaintext, cert->namedCurve->name, 1); + rv = sslBuffer_AppendNumber(&plaintext, cert->namedCurve->name, 1); } else { - rv = ssl3_AppendNumberToItem(&plaintext, 0, 1); + rv = sslBuffer_AppendNumber(&plaintext, 0, 1); } if (rv != SECSuccess) goto loser; /* master_secret */ - rv = ssl3_AppendNumberToItem(&plaintext, ms_is_wrapped, 1); + rv = sslBuffer_AppendNumber(&plaintext, ms_is_wrapped, 1); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, msWrapMech, 4); + rv = sslBuffer_AppendNumber(&plaintext, msWrapMech, 4); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, ms_item.len, 2); - if (rv != SECSuccess) - goto loser; - rv = ssl3_AppendToItem(&plaintext, ms_item.data, ms_item.len); + rv = sslBuffer_AppendVariable(&plaintext, ms_item.data, ms_item.len, 2); if (rv != SECSuccess) goto loser; /* client identity */ if (ss->opt.requestCertificate && ss->sec.ci.sid->peerCert) { - rv = ssl3_AppendNumberToItem(&plaintext, CLIENT_AUTH_CERTIFICATE, 1); - if (rv != SECSuccess) - goto loser; - rv = ssl3_AppendNumberToItem(&plaintext, - ss->sec.ci.sid->peerCert->derCert.len, 2); + rv = sslBuffer_AppendNumber(&plaintext, CLIENT_AUTH_CERTIFICATE, 1); if (rv != SECSuccess) goto loser; - rv = ssl3_AppendToItem(&plaintext, - ss->sec.ci.sid->peerCert->derCert.data, - ss->sec.ci.sid->peerCert->derCert.len); + rv = sslBuffer_AppendVariable(&plaintext, + ss->sec.ci.sid->peerCert->derCert.data, + ss->sec.ci.sid->peerCert->derCert.len, 2); if (rv != SECSuccess) goto loser; } else { - rv = ssl3_AppendNumberToItem(&plaintext, 0, 1); + rv = sslBuffer_AppendNumber(&plaintext, 0, 1); if (rv != SECSuccess) goto loser; } @@ -848,45 +795,39 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, /* timestamp */ now = ssl_TimeUsec(); PORT_Assert(sizeof(now) == 8); - rv = ssl3_AppendNumberToItem(&plaintext, now, 8); + rv = sslBuffer_AppendNumber(&plaintext, now, 8); if (rv != SECSuccess) goto loser; /* HostName (length and value) */ - rv = ssl3_AppendNumberToItem(&plaintext, srvName->len, 2); + rv = sslBuffer_AppendVariable(&plaintext, srvName->data, srvName->len, 2); if (rv != SECSuccess) goto loser; - if (srvName->len) { - rv = ssl3_AppendToItem(&plaintext, srvName->data, srvName->len); - if (rv != SECSuccess) - goto loser; - } /* extendedMasterSecretUsed */ - rv = ssl3_AppendNumberToItem( + rv = sslBuffer_AppendNumber( &plaintext, ss->sec.ci.sid->u.ssl3.keys.extendedMasterSecretUsed, 1); if (rv != SECSuccess) goto loser; /* Flags */ - rv = ssl3_AppendNumberToItem(&plaintext, ticket->flags, - sizeof(ticket->flags)); + rv = sslBuffer_AppendNumber(&plaintext, ticket->flags, + sizeof(ticket->flags)); if (rv != SECSuccess) goto loser; /* ALPN value. */ + PORT_Assert(ss->xtnData.nextProtoState == SSL_NEXT_PROTO_SELECTED || + ss->xtnData.nextProtoState == SSL_NEXT_PROTO_NEGOTIATED || + ss->xtnData.nextProto.len == 0); + alpnSelection = &ss->xtnData.nextProto; PORT_Assert(alpnSelection->len < 256); - rv = ssl3_AppendNumberToItem(&plaintext, alpnSelection->len, 1); + rv = sslBuffer_AppendVariable(&plaintext, alpnSelection->data, + alpnSelection->len, 1); if (rv != SECSuccess) goto loser; - if (alpnSelection->len) { - rv = ssl3_AppendToItem(&plaintext, alpnSelection->data, - alpnSelection->len); - if (rv != SECSuccess) - goto loser; - } - rv = ssl3_AppendNumberToItem(&plaintext, ssl_max_early_data_size, 4); + rv = sslBuffer_AppendNumber(&plaintext, ssl_max_early_data_size, 4); if (rv != SECSuccess) goto loser; @@ -909,28 +850,31 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, */ ticketAgeBaseline = (ssl_TimeUsec() - ss->ssl3.hs.serverHelloTime) / PR_USEC_PER_MSEC; ticketAgeBaseline -= ticket->ticket_age_add; - rv = ssl3_AppendNumberToItem(&plaintext, ticketAgeBaseline, 4); + rv = sslBuffer_AppendNumber(&plaintext, ticketAgeBaseline, 4); if (rv != SECSuccess) goto loser; /* Application token */ - rv = ssl3_AppendNumberToItem(&plaintext, appTokenLen, 2); - if (rv != SECSuccess) - goto loser; - rv = ssl3_AppendToItem(&plaintext, appToken, appTokenLen); + rv = sslBuffer_AppendVariable(&plaintext, appToken, appTokenLen, 2); if (rv != SECSuccess) goto loser; - /* Check that we are totally full. */ - PORT_Assert(plaintext.len == 0); + /* This really only happens if appTokenLen is too much, and that always + * comes from the using application. */ + if (SSL_BUFFER_LEN(&plaintext) > 0xffff) { + PORT_SetError(SEC_ERROR_INVALID_ARGS); + goto loser; + } - /* 128 just gives us enough room for overhead. */ - if (SECITEM_AllocItem(NULL, &ticket_buf, plaintext_length + 128) == NULL) { + ticket_buf.len = ssl_SelfEncryptGetProtectedSize(SSL_BUFFER_LEN(&plaintext)); + PORT_Assert(ticket_buf.len > 0); + if (SECITEM_AllocItem(NULL, &ticket_buf, ticket_buf.len) == NULL) { goto loser; } /* Finally, encrypt the ticket. */ - rv = ssl_SelfEncryptProtect(ss, plaintext_item.data, plaintext_item.len, + rv = ssl_SelfEncryptProtect(ss, SSL_BUFFER_BASE(&plaintext), + SSL_BUFFER_LEN(&plaintext), ticket_buf.data, &ticket_buf.len, ticket_buf.len); if (rv != SECSuccess) { goto loser; @@ -939,13 +883,11 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, /* Give ownership of memory to caller. */ *ticket_data = ticket_buf; - SECITEM_FreeItem(&plaintext_item, PR_FALSE); + sslBuffer_Clear(&plaintext); return SECSuccess; loser: - if (plaintext_item.data) { - SECITEM_FreeItem(&plaintext_item, PR_FALSE); - } + sslBuffer_Clear(&plaintext); if (ticket_buf.data) { SECITEM_FreeItem(&ticket_buf, PR_FALSE); } @@ -1745,22 +1687,16 @@ SECStatus ssl3_SendSigAlgsXtn(const sslSocket *ss, TLSExtensionData *xtnData, sslBuffer *buf, PRBool *added) { - PRUint8 schemes[MAX_SIGNATURE_SCHEMES * 2]; - PRUint32 len; SECStatus rv; if (ss->vrange.max < SSL_LIBRARY_VERSION_TLS_1_2) { return SECSuccess; } - rv = ssl3_EncodeSigAlgs(ss, schemes, sizeof(schemes), &len); + rv = ssl3_EncodeSigAlgs(ss, buf); if (rv != SECSuccess) { return SECFailure; } - rv = sslBuffer_AppendVariable(buf, schemes, len, 2); - if (rv != SECSuccess) { - return -1; - } *added = PR_TRUE; return SECSuccess; diff --git a/lib/ssl/sslencode.c b/lib/ssl/sslencode.c index 25369e86e1..2f127fe8fd 100644 --- a/lib/ssl/sslencode.c +++ b/lib/ssl/sslencode.c @@ -12,16 +12,16 @@ #include "sslimpl.h" /* Helper function to encode an unsigned integer into a buffer. */ -PRUint8 * -ssl_EncodeUintX(PRUint64 value, unsigned int bytes, PRUint8 *to) +static void +ssl_EncodeUintX(PRUint8 *to, PRUint64 value, unsigned int bytes) { PRUint64 encoded; PORT_Assert(bytes > 0 && bytes <= sizeof(encoded)); encoded = PR_htonll(value); - memcpy(to, ((unsigned char *)(&encoded)) + (sizeof(encoded) - bytes), bytes); - return to + bytes; + PORT_Memcpy(to, ((unsigned char *)(&encoded)) + (sizeof(encoded) - bytes), + bytes); } /* Grow a buffer to hold newLen bytes of data. When used for recv/xmit buffers, @@ -29,6 +29,15 @@ ssl_EncodeUintX(PRUint64 value, unsigned int bytes, PRUint8 *to) SECStatus sslBuffer_Grow(sslBuffer *b, unsigned int newLen) { + if (b->fixed) { + PORT_Assert(newLen <= b->space); + if (newLen > b->space) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return SECFailure; + } + return SECSuccess; + } + newLen = PR_MAX(newLen, b->len + 1024); if (newLen > b->space) { unsigned char *newBuf; @@ -51,9 +60,9 @@ sslBuffer_Append(sslBuffer *b, const void *data, unsigned int len) { SECStatus rv = sslBuffer_Grow(b, b->len + len); if (rv != SECSuccess) { - return rv; /* Code already set. */ + return SECFailure; /* Code already set. */ } - PORT_Memcpy(b->buf + b->len, data, len); + PORT_Memcpy(SSL_BUFFER_NEXT(b), data, len); b->len += len; return SECSuccess; } @@ -63,9 +72,9 @@ sslBuffer_AppendNumber(sslBuffer *b, PRUint64 v, unsigned int size) { SECStatus rv = sslBuffer_Grow(b, b->len + size); if (rv != SECSuccess) { - return rv; + return SECFailure; } - (void)ssl_EncodeUintX(v, size, b->buf + b->len); + ssl_EncodeUintX(SSL_BUFFER_NEXT(b), v, size); b->len += size; return SECSuccess; } @@ -74,13 +83,19 @@ SECStatus sslBuffer_AppendVariable(sslBuffer *b, const PRUint8 *data, unsigned int len, unsigned int size) { - SECStatus rv = sslBuffer_Grow(b, b->len + len + size); - if (rv != SECSuccess) { - return rv; + PORT_Assert(size <= 4 && size > 0); + if (len >= (1ULL << (8 * size))) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return SECFailure; + } + + if (sslBuffer_Grow(b, b->len + len + size) != SECSuccess) { + return SECFailure; } - (void)ssl_EncodeUintX(len, size, b->buf + b->len); + + ssl_EncodeUintX(SSL_BUFFER_NEXT(b), len, size); b->len += size; - PORT_Memcpy(b->buf + b->len, data, len); + PORT_Memcpy(SSL_BUFFER_NEXT(b), data, len); b->len += len; return SECSuccess; } @@ -98,44 +113,63 @@ sslBuffer_AppendBufferVariable(sslBuffer *b, const sslBuffer *append, return sslBuffer_AppendVariable(b, append->buf, append->len, size); } -void -sslBuffer_Clear(sslBuffer *b) +SECStatus +sslBuffer_Skip(sslBuffer *b, unsigned int size, unsigned int *savedOffset) { - if (b->buf) { - PORT_Free(b->buf); - b->buf = NULL; - b->len = 0; - b->space = 0; + if (sslBuffer_Grow(b, b->len + size) != SECSuccess) { + return SECFailure; + } + + if (savedOffset) { + *savedOffset = b->len; } + b->len += size; + return SECSuccess; } +/* A common problem is that a buffer is used to construct a variable length + * structure of unknown length. The length field for that structure is then + * populated afterwards. This function makes this process a little easier. + * + * To use this, before encoding the variable length structure, skip the spot + * where the length would be using sslBuffer_Skip(). After encoding the + * structure, and before encoding anything else, call this function passing the + * value returned from sslBuffer_Skip() as |at| to have the length inserted. + */ SECStatus -ssl3_AppendToItem(SECItem *item, const PRUint8 *buf, unsigned int size) +sslBuffer_InsertLength(sslBuffer *b, unsigned int at, unsigned int size) { - if (size > item->len) { - PORT_SetError(SEC_ERROR_INVALID_ARGS); + unsigned int len; + + PORT_Assert(b->len >= at + size); + PORT_Assert(b->space >= at + size); + len = b->len - (at + size); + + PORT_Assert(size <= 4 && size > 0); + if (len >= (1ULL << (8 * size))) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); return SECFailure; } - PORT_Memcpy(item->data, buf, size); - item->data += size; - item->len -= size; + ssl_EncodeUintX(SSL_BUFFER_BASE(b) + at, len, size); return SECSuccess; } -SECStatus -ssl3_AppendNumberToItem(SECItem *item, PRUint64 num, unsigned int size) +void +sslBuffer_Clear(sslBuffer *b) { - SECStatus rv; - PRUint8 b[sizeof(num)]; - - ssl_EncodeUintX(num, size, b); - rv = ssl3_AppendToItem(item, &b[0], size); - return rv; + if (!b->fixed) { + if (b->buf) { + PORT_Free(b->buf); + b->buf = NULL; + } + b->space = 0; + } + b->len = 0; } SECStatus -ssl3_ConsumeFromItem(SECItem *item, PRUint8 **buf, unsigned int size) +ssl3_ConsumeFromItem(SECItem *item, unsigned char **buf, unsigned int size) { if (size > item->len) { PORT_SetError(SEC_ERROR_BAD_DATA); @@ -168,3 +202,95 @@ ssl3_ConsumeNumberFromItem(SECItem *item, PRUint32 *num, unsigned int size) return SECSuccess; } + +/************************************************************************** + * Append Handshake functions. + * All these functions set appropriate error codes. + * Most rely on ssl3_AppendHandshake to set the error code. + **************************************************************************/ +#define MAX_SEND_BUF_LENGTH 32000 /* watch for 16-bit integer overflow */ +#define MIN_SEND_BUF_LENGTH 4000 + +SECStatus +ssl3_AppendHandshake(sslSocket *ss, const void *void_src, unsigned int bytes) +{ + unsigned char *src = (unsigned char *)void_src; + int room = ss->sec.ci.sendBuf.space - ss->sec.ci.sendBuf.len; + SECStatus rv; + + PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); /* protects sendBuf. */ + + if (!bytes) + return SECSuccess; + if (ss->sec.ci.sendBuf.space < MAX_SEND_BUF_LENGTH && room < bytes) { + rv = sslBuffer_Grow(&ss->sec.ci.sendBuf, PR_MAX(MIN_SEND_BUF_LENGTH, + PR_MIN(MAX_SEND_BUF_LENGTH, ss->sec.ci.sendBuf.len + bytes))); + if (rv != SECSuccess) + return SECFailure; /* sslBuffer_Grow sets a memory error code. */ + room = ss->sec.ci.sendBuf.space - ss->sec.ci.sendBuf.len; + } + + PRINT_BUF(60, (ss, "Append to Handshake", (unsigned char *)void_src, bytes)); + rv = ssl3_UpdateHandshakeHashes(ss, src, bytes); + if (rv != SECSuccess) + return SECFailure; /* error code set by ssl3_UpdateHandshakeHashes */ + + while (bytes > room) { + if (room > 0) + PORT_Memcpy(ss->sec.ci.sendBuf.buf + ss->sec.ci.sendBuf.len, src, + room); + ss->sec.ci.sendBuf.len += room; + rv = ssl3_FlushHandshake(ss, ssl_SEND_FLAG_FORCE_INTO_BUFFER); + if (rv != SECSuccess) { + return SECFailure; /* error code set by ssl3_FlushHandshake */ + } + bytes -= room; + src += room; + room = ss->sec.ci.sendBuf.space; + PORT_Assert(ss->sec.ci.sendBuf.len == 0); + } + PORT_Memcpy(ss->sec.ci.sendBuf.buf + ss->sec.ci.sendBuf.len, src, bytes); + ss->sec.ci.sendBuf.len += bytes; + return SECSuccess; +} + +SECStatus +ssl3_AppendHandshakeNumber(sslSocket *ss, PRUint64 num, unsigned int lenSize) +{ + PRUint8 b[sizeof(num)]; + SSL_TRC(60, ("%d: number:", SSL_GETPID())); + ssl_EncodeUintX(b, num, lenSize); + return ssl3_AppendHandshake(ss, b, lenSize); +} + +SECStatus +ssl3_AppendHandshakeVariable(sslSocket *ss, const PRUint8 *src, + unsigned int bytes, unsigned int lenSize) +{ + SECStatus rv; + + PORT_Assert((bytes < (1 << 8) && lenSize == 1) || + (bytes < (1L << 16) && lenSize == 2) || + (bytes < (1L << 24) && lenSize == 3)); + + SSL_TRC(60, ("%d: append variable:", SSL_GETPID())); + rv = ssl3_AppendHandshakeNumber(ss, bytes, lenSize); + if (rv != SECSuccess) { + return SECFailure; /* error code set by AppendHandshake. */ + } + SSL_TRC(60, ("data:")); + return ssl3_AppendHandshake(ss, src, bytes); +} + +SECStatus +ssl3_AppendBufferToHandshake(sslSocket *ss, sslBuffer *buf) +{ + return ssl3_AppendHandshake(ss, buf->buf, buf->len); +} + +SECStatus +ssl3_AppendBufferToHandshakeVariable(sslSocket *ss, sslBuffer *buf, + unsigned int lenSize) +{ + return ssl3_AppendHandshakeVariable(ss, buf->buf, buf->len, lenSize); +} diff --git a/lib/ssl/sslencode.h b/lib/ssl/sslencode.h index 11a97c71dd..0cb920b481 100644 --- a/lib/ssl/sslencode.h +++ b/lib/ssl/sslencode.h @@ -9,17 +9,30 @@ #ifndef __sslencode_h_ #define __sslencode_h_ -PRUint8 *ssl_EncodeUintX(PRUint64 value, unsigned int bytes, PRUint8 *to); - -/* - ** A buffer object. - */ +/* A buffer object, used for assembling messages. */ typedef struct sslBufferStr { PRUint8 *buf; unsigned int len; unsigned int space; + /* Set to true if the storage for the buffer is fixed, such as a stack + * variable or a view on another buffer. Growing a fixed buffer fails. */ + PRBool fixed; } sslBuffer; +#define SSL_BUFFER_EMPTY \ + { \ + NULL, 0, 0, PR_FALSE \ + } +#define SSL_BUFFER_FIXED(b, maxlen) \ + { \ + b, 0, maxlen, PR_TRUE \ + } +#define SSL_BUFFER(b) SSL_BUFFER_FIXED(b, sizeof(b)) +#define SSL_BUFFER_BASE(b) ((b)->buf) +#define SSL_BUFFER_LEN(b) ((b)->len) +#define SSL_BUFFER_NEXT(b) ((b)->buf + (b)->len) +#define SSL_BUFFER_SPACE(b) ((b)->space - (b)->len) + SECStatus sslBuffer_Grow(sslBuffer *b, unsigned int newLen); SECStatus sslBuffer_Append(sslBuffer *b, const void *data, unsigned int len); SECStatus sslBuffer_AppendNumber(sslBuffer *b, PRUint64 v, unsigned int size); @@ -28,17 +41,32 @@ SECStatus sslBuffer_AppendVariable(sslBuffer *b, const PRUint8 *data, SECStatus sslBuffer_AppendBuffer(sslBuffer *b, const sslBuffer *append); SECStatus sslBuffer_AppendBufferVariable(sslBuffer *b, const sslBuffer *append, unsigned int size); +SECStatus sslBuffer_Skip(sslBuffer *b, unsigned int size, + unsigned int *savedOffset); +SECStatus sslBuffer_InsertLength(sslBuffer *b, unsigned int at, + unsigned int size); void sslBuffer_Clear(sslBuffer *b); /* All of these functions modify the underlying SECItem, and so should * be performed on a shallow copy.*/ -SECStatus ssl3_AppendToItem(SECItem *item, - const PRUint8 *buf, PRUint32 bytes); -SECStatus ssl3_AppendNumberToItem(SECItem *item, - PRUint64 num, unsigned int size); SECStatus ssl3_ConsumeFromItem(SECItem *item, PRUint8 **buf, unsigned int size); SECStatus ssl3_ConsumeNumberFromItem(SECItem *item, PRUint32 *num, unsigned int size); +/* These are used for building the handshake. */ +typedef struct sslSocketStr sslSocket; + +SECStatus ssl3_AppendHandshake(sslSocket *ss, const void *void_src, + unsigned int bytes); +SECStatus ssl3_AppendHandshakeHeader(sslSocket *ss, + SSLHandshakeType t, unsigned int length); +SECStatus ssl3_AppendHandshakeNumber(sslSocket *ss, PRUint64 num, + unsigned int lenSize); +SECStatus ssl3_AppendHandshakeVariable(sslSocket *ss, const PRUint8 *src, + unsigned int bytes, unsigned int lenSize); +SECStatus ssl3_AppendBufferToHandshake(sslSocket *ss, sslBuffer *buf); +SECStatus ssl3_AppendBufferToHandshakeVariable(sslSocket *ss, sslBuffer *buf, + unsigned int lenSize); + #endif /* __sslencode_h_ */ diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index d2c6246850..8425e9b160 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -1671,17 +1671,6 @@ extern SECStatus ssl3_ComputeCommonKeyHash(SSLHashType hashAlg, SSL3Hashes *hashes); extern void ssl3_DestroyCipherSpec(ssl3CipherSpec *spec, PRBool freeSrvName); extern SECStatus ssl3_InitPendingCipherSpec(sslSocket *ss, PK11SymKey *pms); -extern SECStatus ssl3_AppendHandshake(sslSocket *ss, const void *void_src, - unsigned int bytes); -extern SECStatus ssl3_AppendHandshakeHeader(sslSocket *ss, - SSLHandshakeType t, unsigned int length); -extern SECStatus ssl3_AppendHandshakeNumber(sslSocket *ss, PRUint64 num, - unsigned int lenSize); -extern SECStatus ssl3_AppendHandshakeVariable(sslSocket *ss, const PRUint8 *src, - unsigned int bytes, unsigned int lenSize); -extern SECStatus ssl3_AppendBufferToHandshake(sslSocket *ss, sslBuffer *buf); -extern SECStatus ssl3_AppendBufferToHandshakeVariable(sslSocket *ss, sslBuffer *buf, - unsigned int lenSize); extern SECStatus ssl3_AppendSignatureAndHashAlgorithm( sslSocket *ss, const SSLSignatureAndHashAlg *sigAndHash); extern SECStatus ssl3_ConsumeHandshake(sslSocket *ss, void *v, PRUint32 bytes, @@ -1814,8 +1803,7 @@ SECStatus ssl3_SendCertificateStatus(sslSocket *ss); SECStatus ssl3_AuthCertificate(sslSocket *ss); SECStatus ssl_ReadCertificateStatus(sslSocket *ss, PRUint8 *b, PRUint32 length); -SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint8 *buf, - unsigned maxLen, PRUint32 *len); +SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, sslBuffer *buf); SECStatus ssl_GetCertificateRequestCAs(const sslSocket *ss, unsigned int *calenp, const SECItem **namesp, diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index 63537cc4f0..f873cad994 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -1644,7 +1644,7 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss, sslBuffer *buffer) { SECStatus rv; - sslBuffer extensionsBuf = { NULL, 0, 0 }; + sslBuffer extensionsBuf = SSL_BUFFER_EMPTY; PORT_Assert(buffer->len == 0); rv = sslBuffer_AppendNumber(buffer, @@ -1668,7 +1668,8 @@ tls13_ConstructHelloRetryRequest(sslSocket *ss, if (rv != SECSuccess) { goto loser; } - PORT_Assert(extensionsBuf.len > 0); /* These can't be empty. */ + /* These extensions can't be empty. */ + PORT_Assert(SSL_BUFFER_LEN(&extensionsBuf) > 0); /* Clean up cookie so we're not pointing at random memory. */ ss->xtnData.cookie.data = NULL; @@ -1695,7 +1696,7 @@ tls13_SendHelloRetryRequest(sslSocket *ss, SECStatus rv; unsigned int cookieLen; PRUint8 cookie[1024]; - sslBuffer messageBuf = { NULL, 0, 0 }; + sslBuffer messageBuf = SSL_BUFFER_EMPTY; SSL_TRC(3, ("%d: TLS13[%d]: send hello retry request handshake", SSL_GETPID(), ss->fd)); @@ -1723,7 +1724,7 @@ tls13_SendHelloRetryRequest(sslSocket *ss, /* And send it. */ ssl_GetXmitBufLock(ss); rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_hello_retry_request, - messageBuf.len); + SSL_BUFFER_LEN(&messageBuf)); if (rv != SECSuccess) { goto loser; } @@ -1824,7 +1825,7 @@ static SECStatus tls13_SendCertificateRequest(sslSocket *ss) { SECStatus rv; - sslBuffer extensionBuf = { NULL, 0, 0 }; + sslBuffer extensionBuf = SSL_BUFFER_EMPTY; SSL_TRC(3, ("%d: TLS13[%d]: begin send certificate_request", SSL_GETPID(), ss->fd)); @@ -1833,12 +1834,13 @@ tls13_SendCertificateRequest(sslSocket *ss) if (rv != SECSuccess) { return SECFailure; /* Code already set. */ } - PORT_Assert(extensionBuf.len > 0); /* These can't be empty. */ + /* We should always have at least one of these. */ + PORT_Assert(SSL_BUFFER_LEN(&extensionBuf) > 0); rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_certificate_request, 1 + 0 + /* empty request context */ 2 + /* extension length */ - extensionBuf.len /* extensions */); + SSL_BUFFER_LEN(&extensionBuf)); if (rv != SECSuccess) { goto loser; /* err set by AppendHandshake. */ } @@ -2402,7 +2404,7 @@ tls13_SendCertificate(sslSocket *ss) int certChainLen = 0; int i; SECItem context = { siBuffer, NULL, 0 }; - sslBuffer extensionBuf = { NULL, 0, 0 }; + sslBuffer extensionBuf = SSL_BUFFER_EMPTY; SSL_TRC(3, ("%d: TLS1.3[%d]: send certificate handshake", SSL_GETPID(), ss->fd)); @@ -2442,7 +2444,7 @@ tls13_SendCertificate(sslSocket *ss) return SECFailure; /* code already set */ } /* extensionBuf.len is only added once, for the leaf cert. */ - certChainLen += extensionBuf.len; + certChainLen += SSL_BUFFER_LEN(&extensionBuf); } rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_certificate, @@ -3404,7 +3406,7 @@ tls13_HandleEncryptedExtensions(sslSocket *ss, PRUint8 *b, PRUint32 length) static SECStatus tls13_SendEncryptedExtensions(sslSocket *ss) { - sslBuffer extensions = { NULL, 0, 0 }; + sslBuffer extensions = SSL_BUFFER_EMPTY; SECStatus rv; SSL_TRC(3, ("%d: TLS13[%d]: send encrypted extensions handshake", @@ -3419,7 +3421,7 @@ tls13_SendEncryptedExtensions(sslSocket *ss) } rv = ssl3_AppendHandshakeHeader(ss, ssl_hs_encrypted_extensions, - extensions.len + 2); + SSL_BUFFER_LEN(&extensions) + 2); if (rv != SECSuccess) { LOG_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE); goto loser; @@ -4158,6 +4160,7 @@ tls13_SendNewSessionTicket(sslSocket *ss, const PRUint8 *appToken, NewSessionTicket ticket = { 0 }; PRUint32 max_early_data_size_len = 0; PRUint8 ticketNonce[sizeof(ss->ssl3.hs.ticketNonce)]; + sslBuffer ticketNonceBuf = SSL_BUFFER(ticketNonce); SSL_TRC(3, ("%d: TLS13[%d]: send new session ticket message %d", SSL_GETPID(), ss->fd, ss->ssl3.hs.ticketNonce)); @@ -4175,8 +4178,11 @@ tls13_SendNewSessionTicket(sslSocket *ss, const PRUint8 *appToken, if (rv != SECSuccess) goto loser; - (void)ssl_EncodeUintX(ss->ssl3.hs.ticketNonce, sizeof(ticketNonce), - ticketNonce); + rv = sslBuffer_AppendNumber(&ticketNonceBuf, ss->ssl3.hs.ticketNonce, + sizeof(ticketNonce)); + if (rv != SECSuccess) { + goto loser; + } ++ss->ssl3.hs.ticketNonce; rv = tls13_HkdfExpandLabel(ss->ssl3.hs.resumptionMasterSecret, tls13_GetHash(ss), @@ -4511,15 +4517,14 @@ tls13_ExtensionStatus(PRUint16 extension, SSLHandshakeType message) * number and that's what we put here. The TLS 1.3 AEAD functions * just use this input as the sequence number and not as additional * data. */ -static void +static SECStatus tls13_FormatAdditionalData(PRUint8 *aad, unsigned int length, sslSequenceNumber seqNum) { - PRUint8 *ptr = aad; + sslBuffer buf = SSL_BUFFER_FIXED(aad, length); PORT_Assert(length == 8); - ptr = ssl_EncodeUintX(seqNum, 8, ptr); - PORT_Assert((ptr - aad) == length); + return sslBuffer_AppendNumber(&buf, seqNum, length); } PRInt32 @@ -4579,7 +4584,10 @@ tls13_ProtectRecord(sslSocket *ss, /* Add the content type at the end. */ wrBuf->buf[contentLen] = type; - tls13_FormatAdditionalData(aad, sizeof(aad), cwSpec->write_seq_num); + rv = tls13_FormatAdditionalData(aad, sizeof(aad), cwSpec->write_seq_num); + if (rv != SECSuccess) { + return SECFailure; + } rv = cwSpec->aead( ss->sec.isServer ? &cwSpec->server : &cwSpec->client, PR_FALSE, /* do encrypt */ @@ -4653,9 +4661,12 @@ tls13_UnprotectRecord(sslSocket *ss, SSL3Ciphertext *cText, sslBuffer *plaintext /* Decrypt */ PORT_Assert(cipher_def->type == type_aead); - tls13_FormatAdditionalData(aad, sizeof(aad), - IS_DTLS(ss) ? cText->seq_num - : crSpec->read_seq_num); + rv = tls13_FormatAdditionalData(aad, sizeof(aad), + IS_DTLS(ss) ? cText->seq_num + : crSpec->read_seq_num); + if (rv != SECSuccess) { + return SECFailure; + } rv = crSpec->aead( ss->sec.isServer ? &crSpec->client : &crSpec->server, PR_TRUE, /* do decrypt */ diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h index 920f49d6c4..ca376f5f62 100644 --- a/lib/ssl/tls13con.h +++ b/lib/ssl/tls13con.h @@ -77,7 +77,8 @@ SECStatus tls13_HandlePostHelloHandshakeMessage(sslSocket *ss, PRUint8 *b, SECStatus tls13_ConstructHelloRetryRequest(sslSocket *ss, ssl3CipherSuite cipherSuite, const sslNamedGroupDef *selectedGroup, - PRUint8 *cookie, unsigned int cookieLen, + PRUint8 *cookie, + unsigned int cookieLen, sslBuffer *buffer); SECStatus tls13_HandleHelloRetryRequest(sslSocket *ss, PRUint8 *b, PRUint32 length); diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c index 68632ef608..a94db7bcc6 100644 --- a/lib/ssl/tls13exthandle.c +++ b/lib/ssl/tls13exthandle.c @@ -135,8 +135,10 @@ tls13_ClientSendKeyShareXtn(const sslSocket *ss, TLSExtensionData *xtnData, SSL_GETPID(), ss->fd)); /* Save the offset to the length. */ - lengthOffset = buf->len; - buf->len += 2; + rv = sslBuffer_Skip(buf, 2, &lengthOffset); + if (rv != SECSuccess) { + return SECFailure; + } for (cursor = PR_NEXT_LINK(&ss->ephemeralKeyPairs); cursor != &ss->ephemeralKeyPairs; @@ -147,9 +149,10 @@ tls13_ClientSendKeyShareXtn(const sslSocket *ss, TLSExtensionData *xtnData, return SECFailure; } } - PORT_Assert(buf->len >= lengthOffset + 2); - (void)ssl_EncodeUintX(buf->len - lengthOffset - 2, 2, - buf->buf + lengthOffset); + rv = sslBuffer_InsertLength(buf, lengthOffset, 2); + if (rv != SECSuccess) { + return SECFailure; + } *added = PR_TRUE; return SECSuccess; @@ -737,7 +740,7 @@ tls13_ClientSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD sslBuffer *buf, PRBool *added) { PRUint16 version; - unsigned int size; + unsigned int lengthOffset; SECStatus rv; if (ss->vrange.max < SSL_LIBRARY_VERSION_TLS_1_3) { @@ -747,15 +750,11 @@ tls13_ClientSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD SSL_TRC(3, ("%d: TLS13[%d]: send supported_versions extension", SSL_GETPID(), ss->fd)); - size = 2 * (ss->vrange.max - ss->vrange.min + 1); - if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { - size += 2; + rv = sslBuffer_Skip(buf, 1, &lengthOffset); + if (rv != SECSuccess) { + return SECFailure; } - rv = sslBuffer_AppendNumber(buf, size, 1); - if (rv != SECSuccess) - return -1; - if (ss->opt.enableAltHandshaketype && !IS_DTLS(ss)) { rv = sslBuffer_AppendNumber( buf, tls13_EncodeAltDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3), @@ -766,8 +765,14 @@ tls13_ClientSendSupportedVersionsXtn(const sslSocket *ss, TLSExtensionData *xtnD } for (version = ss->vrange.max; version >= ss->vrange.min; --version) { rv = sslBuffer_AppendNumber(buf, tls13_EncodeDraftVersion(version), 2); - if (rv != SECSuccess) + if (rv != SECSuccess) { return SECFailure; + } + } + + rv = sslBuffer_InsertLength(buf, lengthOffset, 1); + if (rv != SECSuccess) { + return SECFailure; } *added = PR_TRUE; diff --git a/lib/ssl/tls13hashstate.c b/lib/ssl/tls13hashstate.c index 5b4e011a18..4c5f383454 100644 --- a/lib/ssl/tls13hashstate.c +++ b/lib/ssl/tls13hashstate.c @@ -35,48 +35,43 @@ tls13_MakeHrrCookie(sslSocket *ss, const sslNamedGroupDef *selectedGroup, { SECStatus rv; SSL3Hashes hashes; - PRUint8 encodedCookie[1024]; - SECItem cookieItem = { siBuffer, encodedCookie, sizeof(encodedCookie) }; - - PORT_Assert(sizeof(encodedCookie) >= maxlen); + PRUint8 cookie[1024]; + sslBuffer cookieBuf = SSL_BUFFER(cookie); + static const PRUint8 indicator = 0xff; /* Encode header. */ - rv = ssl3_AppendNumberToItem(&cookieItem, 0xff, 1); + rv = sslBuffer_Append(&cookieBuf, &indicator, 1); if (rv != SECSuccess) { return SECFailure; } - rv = ssl3_AppendNumberToItem(&cookieItem, ss->ssl3.hs.cipher_suite, 2); + rv = sslBuffer_AppendNumber(&cookieBuf, ss->ssl3.hs.cipher_suite, 2); if (rv != SECSuccess) { return SECFailure; } - rv = ssl3_AppendNumberToItem(&cookieItem, selectedGroup ? selectedGroup->name : 0, 2); + rv = sslBuffer_AppendNumber(&cookieBuf, + selectedGroup ? selectedGroup->name : 0, 2); if (rv != SECSuccess) { return SECFailure; } /* Application token. */ - rv = ssl3_AppendNumberToItem(&cookieItem, appTokenLen, 2); - if (rv != SECSuccess) { - return SECFailure; - } - rv = ssl3_AppendToItem(&cookieItem, appToken, appTokenLen); + rv = sslBuffer_AppendVariable(&cookieBuf, appToken, appTokenLen, 2); if (rv != SECSuccess) { return SECFailure; } - /* Encode the hash state */ + /* Compute and encode hashes. */ rv = tls13_ComputeHandshakeHashes(ss, &hashes); if (rv != SECSuccess) { return SECFailure; } - rv = ssl3_AppendToItem(&cookieItem, hashes.u.raw, hashes.len); + rv = sslBuffer_Append(&cookieBuf, hashes.u.raw, hashes.len); if (rv != SECSuccess) { return SECFailure; } /* Encrypt right into the buffer. */ - rv = ssl_SelfEncryptProtect(ss, - encodedCookie, cookieItem.data - encodedCookie, + rv = ssl_SelfEncryptProtect(ss, cookieBuf.buf, cookieBuf.len, buf, len, maxlen); if (rv != SECSuccess) { return SECFailure; @@ -95,7 +90,7 @@ tls13_RecoverHashState(sslSocket *ss, SECStatus rv; unsigned char plaintext[1024]; SECItem ptItem = { siBuffer, plaintext, 0 }; - sslBuffer messageBuf = { NULL, 0, 0 }; + sslBuffer messageBuf = SSL_BUFFER_EMPTY; PRUint32 sentinel; PRUint32 cipherSuite; PRUint32 group; @@ -174,7 +169,8 @@ tls13_RecoverHashState(sslSocket *ss, } rv = ssl_HashHandshakeMessageInt(ss, ssl_hs_hello_retry_request, 0, - messageBuf.buf, messageBuf.len); + SSL_BUFFER_BASE(&messageBuf), + SSL_BUFFER_LEN(&messageBuf)); sslBuffer_Clear(&messageBuf); if (rv != SECSuccess) { return SECFailure; diff --git a/lib/ssl/tls13hkdf.c b/lib/ssl/tls13hkdf.c index 9da92df010..0cb38b99c9 100644 --- a/lib/ssl/tls13hkdf.c +++ b/lib/ssl/tls13hkdf.c @@ -13,6 +13,7 @@ #include "sslt.h" #include "sslerr.h" #include "sslimpl.h" +#include "sslencode.h" /* This table contains the mapping between TLS hash identifiers and the * PKCS#11 identifiers */ @@ -134,9 +135,9 @@ tls13_HkdfExpandLabel(PK11SymKey *prk, SSLHashType baseHash, * Label, plus HandshakeHash. If it's ever to small, the code will abort. */ PRUint8 info[256]; - PRUint8 *ptr = info; - unsigned int infoLen; + sslBuffer infoBuf = SSL_BUFFER(info); PK11SymKey *derived; + SECStatus rv; const char *kLabelPrefix = "tls13 "; const unsigned int kLabelPrefixLen = strlen(kLabelPrefix); @@ -170,29 +171,31 @@ tls13_HkdfExpandLabel(PK11SymKey *prk, SSLHashType baseHash, * - HkdfLabel.label is "TLS 1.3, " + Label * */ - infoLen = 2 + 1 + kLabelPrefixLen + labelLen + 1 + handshakeHashLen; - if (infoLen > sizeof(info)) { - PORT_Assert(0); - goto abort; + rv = sslBuffer_AppendNumber(&infoBuf, keySize, 2); + if (rv != SECSuccess) { + return SECFailure; } - - ptr = ssl_EncodeUintX(keySize, 2, ptr); - ptr = ssl_EncodeUintX(labelLen + kLabelPrefixLen, 1, ptr); - PORT_Memcpy(ptr, kLabelPrefix, kLabelPrefixLen); - ptr += kLabelPrefixLen; - PORT_Memcpy(ptr, label, labelLen); - ptr += labelLen; - ptr = ssl_EncodeUintX(handshakeHashLen, 1, ptr); - if (handshakeHash) { - PORT_Memcpy(ptr, handshakeHash, handshakeHashLen); - ptr += handshakeHashLen; + rv = sslBuffer_AppendNumber(&infoBuf, labelLen + kLabelPrefixLen, 1); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_Append(&infoBuf, kLabelPrefix, kLabelPrefixLen); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_Append(&infoBuf, label, labelLen); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_AppendVariable(&infoBuf, handshakeHash, handshakeHashLen, 1); + if (rv != SECSuccess) { + return SECFailure; } - PORT_Assert((ptr - info) == infoLen); params.bExtract = CK_FALSE; params.bExpand = CK_TRUE; - params.pInfo = info; - params.ulInfoLen = infoLen; + params.pInfo = SSL_BUFFER_BASE(&infoBuf); + params.ulInfoLen = SSL_BUFFER_LEN(&infoBuf); paramsi.data = (unsigned char *)¶ms; paramsi.len = sizeof(params); @@ -216,15 +219,12 @@ tls13_HkdfExpandLabel(PK11SymKey *prk, SSLHashType baseHash, } PRINT_KEY(50, (NULL, "PRK", prk)); PRINT_BUF(50, (NULL, "Hash", handshakeHash, handshakeHashLen)); - PRINT_BUF(50, (NULL, "Info", info, infoLen)); + PRINT_BUF(50, (NULL, "Info", SSL_BUFFER_BASE(&infoBuf), + SSL_BUFFER_LEN(&infoBuf))); PRINT_KEY(50, (NULL, "Derived key", derived)); #endif return SECSuccess; - -abort: - PORT_SetError(SSL_ERROR_SYM_KEY_CONTEXT_FAILURE); - return SECFailure; } SECStatus