kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Make tls_open_record always in-place.

Browse Source 
The business with ssl_record_prefix_len is rather a hassle. Instead, have
tls_open_record always decrypt in-place and give back a CBS to where the body
is.

This way the caller doesn't need to do an extra check all to avoid creating an
invalid pointer and underflow in subtraction.

Change-Id: I4e12b25a760870d8f8a503673ab00a2d774fc9ee
Reviewed-on: https://boringssl-review.googlesource.com/8173
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin 2016-06-06 18:54:51 -04:00
parent 728f354a2b
commit a7810c12e9
9 changed files with 88 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crypto/bytestring/bytestring_test.cc
View File

@ -43,7 +43,7 @@ static bool TestSkip() {
}
static bool TestGetUint() {
static const uint8_t kData[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
static const uint8_t kData[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11};
uint8_t u8;
uint16_t u16;
uint32_t u32;
@ -58,7 +58,10 @@ static bool TestGetUint() {
u32 == 0x40506 &&
CBS_get_u32(&data, &u32) &&
u32 == 0x708090a &&
!CBS_get_u8(&data, &u8);
CBS_get_last_u8(&data, &u8) &&
u8 == 0xb &&
!CBS_get_u8(&data, &u8) &&
!CBS_get_last_u8(&data, &u8);
}
static bool TestGetPrefixed() {

9
crypto/bytestring/cbs.c
View File

@ -128,6 +128,15 @@ int CBS_get_u32(CBS *cbs, uint32_t *out) {
return cbs_get_u(cbs, out, 4);
}
int CBS_get_last_u8(CBS *cbs, uint8_t *out) {
if (cbs->len == 0) {
return 0;
}
*out = cbs->data[cbs->len - 1];
cbs->len--;
return 1;
}
int CBS_get_bytes(CBS *cbs, CBS *out, size_t len) {
const uint8_t *v;
if (!cbs_get(cbs, &v, len)) {

4
include/openssl/bytestring.h
View File

@ -95,6 +95,10 @@ OPENSSL_EXPORT int CBS_get_u24(CBS *cbs, uint32_t *out);
* and advances |cbs|. It returns one on success and zero on error. */
OPENSSL_EXPORT int CBS_get_u32(CBS *cbs, uint32_t *out);
/* CBS_get_last_u8 sets |*out| to the last uint8_t from |cbs| and shortens
* |cbs|. It returns one on success and zero on error. */
OPENSSL_EXPORT int CBS_get_last_u8(CBS *cbs, uint8_t *out);
/* CBS_get_bytes sets |*out| to the next |len| bytes from |cbs| and advances
* |cbs|. It returns one on success and zero on error. */
OPENSSL_EXPORT int CBS_get_bytes(CBS *cbs, CBS *out, size_t len);

19
ssl/d1_pkt.c
View File

@ -159,18 +159,11 @@ again:
}
assert(ssl_read_buffer_len(ssl) > 0);
/* Ensure the packet is large enough to decrypt in-place. */
if (ssl_read_buffer_len(ssl) < ssl_record_prefix_len(ssl)) {
ssl_read_buffer_clear(ssl);
goto again;
}
uint8_t *out = ssl_read_buffer(ssl) + ssl_record_prefix_len(ssl);
size_t max_out = ssl_read_buffer_len(ssl) - ssl_record_prefix_len(ssl);
CBS body;
uint8_t type, alert;
size_t len, consumed;
size_t consumed;
enum ssl_open_record_t open_ret =
dtls_open_record(ssl, &type, out, &len, &consumed, &alert, max_out,
dtls_open_record(ssl, &type, &body, &consumed, &alert,
ssl_read_buffer(ssl), ssl_read_buffer_len(ssl));
ssl_read_buffer_consume(ssl, consumed);
switch (open_ret) {
@ -179,15 +172,15 @@ again:
break;
case ssl_open_record_success:
if (len > 0xffff) {
if (CBS_len(&body) > 0xffff) {
OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW);
return -1;
}
SSL3_RECORD *rr = &ssl->s3->rrec;
rr->type = type;
rr->length = (uint16_t)len;
rr->data = out;
rr->length = (uint16_t)CBS_len(&body);
rr->data = (uint8_t *)CBS_data(&body);
return 1;
case ssl_open_record_discard:

21
ssl/dtls_record.c
View File

@ -171,10 +171,10 @@ static void dtls1_bitmap_record(DTLS1_BITMAP *bitmap,
}
}
enum ssl_open_record_t dtls_open_record(
SSL *ssl, uint8_t *out_type, uint8_t *out, size_t *out_len,
size_t *out_consumed, uint8_t *out_alert, size_t max_out, const uint8_t *in,
size_t in_len) {
enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
size_t *out_consumed,
uint8_t *out_alert, uint8_t *in,
size_t in_len) {
*out_consumed = 0;
CBS cbs;
@ -211,11 +211,9 @@ enum ssl_open_record_t dtls_open_record(
return ssl_open_record_discard;
}
/* Decrypt the body. */
size_t plaintext_len;
if (!SSL_AEAD_CTX_open(ssl->s3->aead_read_ctx, out, &plaintext_len, max_out,
type, version, sequence, CBS_data(&body),
CBS_len(&body))) {
/* Decrypt the body in-place. */
if (!SSL_AEAD_CTX_open(ssl->s3->aead_read_ctx, out, type, version, sequence,
(uint8_t *)CBS_data(&body), CBS_len(&body))) {
/* Bad packets are silently dropped in DTLS. See section 4.2.1 of RFC 6347.
* Clear the error queue of any errors decryption may have added. Drop the
* entire packet as it must not have come from the peer.
@ -229,7 +227,7 @@ enum ssl_open_record_t dtls_open_record(
*out_consumed = in_len - CBS_len(&cbs);
/* Check the plaintext length. */
if (plaintext_len > SSL3_RT_MAX_PLAIN_LENGTH) {
if (CBS_len(out) > SSL3_RT_MAX_PLAIN_LENGTH) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DATA_LENGTH_TOO_LONG);
*out_alert = SSL_AD_RECORD_OVERFLOW;
return ssl_open_record_error;
@ -241,13 +239,12 @@ enum ssl_open_record_t dtls_open_record(
* useful if we also limit discarded packets. */
if (type == SSL3_RT_ALERT) {
return ssl_process_alert(ssl, out_alert, out, plaintext_len);
return ssl_process_alert(ssl, out_alert, CBS_data(out), CBS_len(out));
}
ssl->s3->warning_alert_count = 0;
*out_type = type;
*out_len = plaintext_len;
return ssl_open_record_success;
}

41
ssl/internal.h
View File

@ -313,15 +313,13 @@ size_t SSL_AEAD_CTX_explicit_nonce_len(SSL_AEAD_CTX *ctx);
* |SSL_AEAD_CTX_seal|. |ctx| may be NULL to denote the null cipher. */
size_t SSL_AEAD_CTX_max_overhead(SSL_AEAD_CTX *ctx);
/* SSL_AEAD_CTX_open authenticates and decrypts |in_len| bytes from |in| and
* writes the result to |out|. It returns one on success and zero on
* error. |ctx| may be NULL to denote the null cipher.
*
* If |in| and |out| alias then |out| must be <= |in| + |explicit_nonce_len|. */
int SSL_AEAD_CTX_open(SSL_AEAD_CTX *ctx, uint8_t *out, size_t *out_len,
size_t max_out, uint8_t type, uint16_t wire_version,
const uint8_t seqnum[8], const uint8_t *in,
size_t in_len);
/* SSL_AEAD_CTX_open authenticates and decrypts |in_len| bytes from |in|
* in-place. On success, it sets |*out| to the plaintext in |in| and returns
* one. Otherwise, it returns zero. |ctx| may be NULL to denote the null cipher.
* The output will always be |explicit_nonce_len| bytes ahead of |in|. */
int SSL_AEAD_CTX_open(SSL_AEAD_CTX *ctx, CBS *out, uint8_t type,
uint16_t wire_version, const uint8_t seqnum[8],
uint8_t *in, size_t in_len);
/* SSL_AEAD_CTX_seal encrypts and authenticates |in_len| bytes from |in| and
* writes the result to |out|. It returns one on success and zero on
@ -370,7 +368,7 @@ enum ssl_open_record_t {
ssl_open_record_error,
};
/* tls_open_record decrypts a record from |in|.
/* tls_open_record decrypts a record from |in| in-place.
*
* If the input did not contain a complete record, it returns
* |ssl_open_record_partial|. It sets |*out_consumed| to the total number of
@ -382,8 +380,8 @@ enum ssl_open_record_t {
* decrypted.
*
* On success, it returns |ssl_open_record_success|. It sets |*out_type| to the
* record type, |*out_len| to the plaintext length, and writes the record body
* to |out|. Note that |*out_len| may be zero.
* record type and |*out| to the record body in |in|. Note that |*out| may be
* empty.
*
* If a record was successfully processed but should be discarded, it returns
* |ssl_open_record_discard|.
@ -392,20 +390,17 @@ enum ssl_open_record_t {
* it returns |ssl_open_record_close_notify| or |ssl_open_record_fatal_alert|.
*
* On failure, it returns |ssl_open_record_error| and sets |*out_alert| to an
* alert to emit.
*
* If |in| and |out| alias, |out| must be <= |in| + |ssl_record_prefix_len|. */
enum ssl_open_record_t tls_open_record(
SSL *ssl, uint8_t *out_type, uint8_t *out, size_t *out_len,
size_t *out_consumed, uint8_t *out_alert, size_t max_out, const uint8_t *in,
size_t in_len);
* alert to emit. */
enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
size_t *out_consumed, uint8_t *out_alert,
uint8_t *in, size_t in_len);
/* dtls_open_record implements |tls_open_record| for DTLS. It never returns
* |ssl_open_record_partial| but otherwise behaves analogously. */
enum ssl_open_record_t dtls_open_record(
SSL *ssl, uint8_t *out_type, uint8_t *out, size_t *out_len,
size_t *out_consumed, uint8_t *out_alert, size_t max_out, const uint8_t *in,
size_t in_len);
enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
size_t *out_consumed,
uint8_t *out_alert, uint8_t *in,
size_t in_len);
/* ssl_seal_prefix_len returns the length of the prefix before the ciphertext
* when sealing a record with |ssl|. Note that this value may differ from

25
ssl/s3_pkt.c
View File

@ -138,41 +138,34 @@ again:
return 0;
}
/* Ensure the buffer is large enough to decrypt in-place. */
int read_ret = ssl_read_buffer_extend_to(ssl, ssl_record_prefix_len(ssl));
if (read_ret <= 0) {
return read_ret;
}
assert(ssl_read_buffer_len(ssl) >= ssl_record_prefix_len(ssl));
uint8_t *out = ssl_read_buffer(ssl) + ssl_record_prefix_len(ssl);
size_t max_out = ssl_read_buffer_len(ssl) - ssl_record_prefix_len(ssl);
CBS body;
uint8_t type, alert;
size_t len, consumed;
size_t consumed;
enum ssl_open_record_t open_ret =
tls_open_record(ssl, &type, out, &len, &consumed, &alert, max_out,
tls_open_record(ssl, &type, &body, &consumed, &alert,
ssl_read_buffer(ssl), ssl_read_buffer_len(ssl));
if (open_ret != ssl_open_record_partial) {
ssl_read_buffer_consume(ssl, consumed);
}
switch (open_ret) {
case ssl_open_record_partial:
read_ret = ssl_read_buffer_extend_to(ssl, consumed);
case ssl_open_record_partial: {
int read_ret = ssl_read_buffer_extend_to(ssl, consumed);
if (read_ret <= 0) {
return read_ret;
}
goto again;
}
case ssl_open_record_success:
if (len > 0xffff) {
if (CBS_len(&body) > 0xffff) {
OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW);
return -1;
}
SSL3_RECORD *rr = &ssl->s3->rrec;
rr->type = type;
rr->length = (uint16_t)len;
rr->data = out;
rr->length = (uint16_t)CBS_len(&body);
rr->data = (uint8_t *)CBS_data(&body);
return 1;
case ssl_open_record_discard:

24
ssl/ssl_aead_ctx.c
View File

@ -166,22 +166,16 @@ static size_t ssl_aead_ctx_get_ad(SSL_AEAD_CTX *aead, uint8_t out[13],
return len;
}
int SSL_AEAD_CTX_open(SSL_AEAD_CTX *aead, uint8_t *out, size_t *out_len,
size_t max_out, uint8_t type, uint16_t wire_version,
const uint8_t seqnum[8], const uint8_t *in,
size_t in_len) {
int SSL_AEAD_CTX_open(SSL_AEAD_CTX *aead, CBS *out, uint8_t type,
uint16_t wire_version, const uint8_t seqnum[8],
uint8_t *in, size_t in_len) {
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
aead = NULL;
#endif
if (aead == NULL) {
/* Handle the initial NULL cipher. */
if (in_len > max_out) {
OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL);
return 0;
}
memmove(out, in, in_len);
*out_len = in_len;
CBS_init(out, in, in_len);
return 1;
}
@ -239,8 +233,14 @@ int SSL_AEAD_CTX_open(SSL_AEAD_CTX *aead, uint8_t *out, size_t *out_len,
}
}
return EVP_AEAD_CTX_open(&aead->ctx, out, out_len, max_out, nonce, nonce_len,
in, in_len, ad, ad_len);
/* Decrypt in-place. */
size_t len;
if (!EVP_AEAD_CTX_open(&aead->ctx, in, &len, in_len, nonce, nonce_len,
in, in_len, ad, ad_len)) {
return 0;
}
CBS_init(out, in, len);
return 1;
}
int SSL_AEAD_CTX_seal(SSL_AEAD_CTX *aead, uint8_t *out, size_t *out_len,

39
ssl/tls_record.c
View File

@ -192,10 +192,9 @@ size_t ssl_max_seal_overhead(const SSL *ssl) {
return ret;
}
enum ssl_open_record_t tls_open_record(
SSL *ssl, uint8_t *out_type, uint8_t *out, size_t *out_len,
size_t *out_consumed, uint8_t *out_alert, size_t max_out, const uint8_t *in,
size_t in_len) {
enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
size_t *out_consumed, uint8_t *out_alert,
uint8_t *in, size_t in_len) {
*out_consumed = 0;
CBS cbs;
@ -236,10 +235,9 @@ enum ssl_open_record_t tls_open_record(
ssl_do_msg_callback(ssl, 0 /* read */, 0, SSL3_RT_HEADER, in,
SSL3_RT_HEADER_LENGTH);
/* Decrypt the body. */
size_t plaintext_len;
if (!SSL_AEAD_CTX_open(ssl->s3->aead_read_ctx, out, &plaintext_len, max_out,
type, version, ssl->s3->read_sequence, CBS_data(&body),
/* Decrypt the body in-place. */
if (!SSL_AEAD_CTX_open(ssl->s3->aead_read_ctx, out, type, version,
ssl->s3->read_sequence, (uint8_t *)CBS_data(&body),
CBS_len(&body))) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
*out_alert = SSL_AD_BAD_RECORD_MAC;
@ -256,28 +254,24 @@ enum ssl_open_record_t tls_open_record(
if (ssl->s3->have_version &&
ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
ssl->s3->aead_read_ctx != NULL) {
while (plaintext_len != 0 && out[plaintext_len - 1] == 0) {
plaintext_len--;
}
if (plaintext_len == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
*out_alert = SSL_AD_DECRYPT_ERROR;
return ssl_open_record_error;
}
type = out[plaintext_len - 1];
plaintext_len--;
do {
if (!CBS_get_last_u8(out, &type)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
*out_alert = SSL_AD_DECRYPT_ERROR;
return ssl_open_record_error;
}
} while (type == 0);
}
/* Check the plaintext length. */
if (plaintext_len > SSL3_RT_MAX_PLAIN_LENGTH) {
if (CBS_len(out) > SSL3_RT_MAX_PLAIN_LENGTH) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DATA_LENGTH_TOO_LONG);
*out_alert = SSL_AD_RECORD_OVERFLOW;
return ssl_open_record_error;
}
/* Limit the number of consecutive empty records. */
if (plaintext_len == 0) {
if (CBS_len(out) == 0) {
ssl->s3->empty_record_count++;
if (ssl->s3->empty_record_count > kMaxEmptyRecords) {
OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MANY_EMPTY_FRAGMENTS);
@ -291,13 +285,12 @@ enum ssl_open_record_t tls_open_record(
}
if (type == SSL3_RT_ALERT) {
return ssl_process_alert(ssl, out_alert, out, plaintext_len);
return ssl_process_alert(ssl, out_alert, CBS_data(out), CBS_len(out));
}
ssl->s3->warning_alert_count = 0;
*out_type = type;
*out_len = plaintext_len;
return ssl_open_record_success;
}