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

Make CBB_len relative to its argument.

Browse Source 
Rather than the length of the top-level CBB, which is kind of odd when ASN.1
length prefixes are not yet determined, return the number of bytes written to
the CBB so far. This can be computed without increasing the size of CBB at all.
Have offset and pending_*.

This means functions which take in a CBB as argument will not be sensitive to
whether the CBB is a top-level or child CBB. The extensions logic had to be
careful to only ever compare differences of lengths, which was awkward.

The reversal will also allow for the following pattern in the future, once
CBB_add_space is split into, say, CBB_reserve and CBB_did_write and we add a
CBB_data:

  uint8_t *signature;
  size_t signature_len = 0;
  if (!CBB_add_asn1(out, &cert, CBB_ASN1_SEQUENCE) ||
      /* Emit the TBSCertificate. */
      !CBB_add_asn1(&cert, &tbs_cert, CBS_ASN1_SEQUENCE) ||
      !CBB_add_tbs_cert_stuff(&tbs_cert, stuff) ||
      !CBB_flush(&cert) ||
      /* Feed it into md_ctx. */
      !EVP_DigestSignInit(&md_ctx, NULL, EVP_sha256(), NULL, pkey) ||
      !EVP_DigestSignUpdate(&md_ctx, CBB_data(&cert), CBB_len(&cert)) ||
      /* Emit the signature algorithm. */
      !CBB_add_asn1(&cert, &sig_alg, CBS_ASN1_SEQUENCE) ||
      !CBB_add_sigalg_stuff(&sig_alg, other_stuff) ||
      /* Emit the signature. */
      !EVP_DigestSignFinal(&md_ctx, NULL, &signature_len) ||
      !CBB_reserve(&cert, &signature, signature_len) ||
      !EVP_DigestSignFinal(&md_ctx, signature, &signature_len) ||
      !CBB_did_write(&cert, signature_len)) {
    goto err;
  }

(Were TBSCertificate not the first field, we'd still have to sample
CBB_len(&cert), but at least that's reasonable straight-forward. The
alternative would be if CBB_data and CBB_len somehow worked on
recently-invalidated CBBs, but that would go wrong once the invalidated CBB's
parent flushed and possibly shifts everything.)

And similar for signing ServerKeyExchange.

Change-Id: I7761e492ae472d7632875b5666b6088970261b14
Reviewed-on: https://boringssl-review.googlesource.com/6681
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-12-08 18:56:31 -05:00 committed by Adam Langley
parent 77385bb43d
commit a01deee96b
4 changed files with 41 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
crypto/bytestring/bytestring_test.cc
View File

@ -344,12 +344,14 @@ static bool TestCBBPrefixed() {
size_t buf_len;
CBB cbb, contents, inner_contents, inner_inner_contents;
if (!CBB_init(&cbb, 0)) {
return false;
}
if (!CBB_add_u8_length_prefixed(&cbb, &contents) ||
if (!CBB_init(&cbb, 0) ||
CBB_len(&cbb) != 0 ||
!CBB_add_u8_length_prefixed(&cbb, &contents) ||
!CBB_add_u8_length_prefixed(&cbb, &contents) ||
!CBB_add_u8(&contents, 1) ||
CBB_len(&contents) != 1 ||
!CBB_flush(&cbb) ||
CBB_len(&cbb) != 3 ||
!CBB_add_u16_length_prefixed(&cbb, &contents) ||
!CBB_add_u16(&contents, 0x203) ||
!CBB_add_u24_length_prefixed(&cbb, &contents) ||

44
crypto/bytestring/cbb.c
View File

@ -179,28 +179,28 @@ int CBB_flush(CBB *cbb) {
return 0;
}
if (cbb->child == NULL || cbb->pending_len_len == 0) {
if (cbb->child == NULL || cbb->child->pending_len_len == 0) {
return 1;
}
child_start = cbb->offset + cbb->pending_len_len;
child_start = cbb->child->offset + cbb->child->pending_len_len;
if (!CBB_flush(cbb->child) ||
child_start < cbb->offset ||
child_start < cbb->child->offset ||
cbb->base->len < child_start) {
return 0;
}
len = cbb->base->len - child_start;
if (cbb->pending_is_asn1) {
if (cbb->child->pending_is_asn1) {
/* For ASN.1 we assume that we'll only need a single byte for the length.
* If that turned out to be incorrect, we have to move the contents along
* in order to make space. */
size_t len_len;
uint8_t initial_length_byte;
assert (cbb->pending_len_len == 1);
assert (cbb->child->pending_len_len == 1);
if (len > 0xfffffffe) {
/* Too large. */
@ -232,12 +232,13 @@ int CBB_flush(CBB *cbb) {
memmove(cbb->base->buf + child_start + extra_bytes,
cbb->base->buf + child_start, len);
}
cbb->base->buf[cbb->offset++] = initial_length_byte;
cbb->pending_len_len = len_len - 1;
cbb->base->buf[cbb->child->offset++] = initial_length_byte;
cbb->child->pending_len_len = len_len - 1;
}
for (i = cbb->pending_len_len - 1; i < cbb->pending_len_len; i--) {
cbb->base->buf[cbb->offset + i] = len;
for (i = cbb->child->pending_len_len - 1; i < cbb->child->pending_len_len;
i--) {
cbb->base->buf[cbb->child->offset + i] = len;
len >>= 8;
}
if (len != 0) {
@ -246,17 +247,15 @@ int CBB_flush(CBB *cbb) {
cbb->child->base = NULL;
cbb->child = NULL;
cbb->pending_len_len = 0;
cbb->pending_is_asn1 = 0;
cbb->offset = 0;
return 1;
}
size_t CBB_len(const CBB *cbb) {
assert(cbb->child == NULL);
assert(cbb->offset + cbb->pending_len_len <= cbb->base->len);
return cbb->base->len;
return cbb->base->len - cbb->offset - cbb->pending_len_len;
}
static int cbb_add_length_prefixed(CBB *cbb, CBB *out_contents,
@ -267,7 +266,7 @@ static int cbb_add_length_prefixed(CBB *cbb, CBB *out_contents,
return 0;
}
cbb->offset = cbb->base->len;
size_t offset = cbb->base->len;
if (!cbb_buffer_add(cbb->base, &prefix_bytes, len_len)) {
return 0;
}
@ -276,8 +275,9 @@ static int cbb_add_length_prefixed(CBB *cbb, CBB *out_contents,
memset(out_contents, 0, sizeof(CBB));
out_contents->base = cbb->base;
cbb->child = out_contents;
cbb->pending_len_len = len_len;
cbb->pending_is_asn1 = 0;
cbb->child->offset = offset;
cbb->child->pending_len_len = len_len;
cbb->child->pending_is_asn1 = 0;
return 1;
}
@ -305,7 +305,7 @@ int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag) {
return 0;
}
cbb->offset = cbb->base->len;
size_t offset = cbb->base->len;
if (!CBB_add_u8(cbb, 0)) {
return 0;
}
@ -313,8 +313,9 @@ int CBB_add_asn1(CBB *cbb, CBB *out_contents, uint8_t tag) {
memset(out_contents, 0, sizeof(CBB));
out_contents->base = cbb->base;
cbb->child = out_contents;
cbb->pending_len_len = 1;
cbb->pending_is_asn1 = 1;
cbb->child->offset = offset;
cbb->child->pending_len_len = 1;
cbb->child->pending_is_asn1 = 1;
return 1;
}
@ -367,13 +368,10 @@ void CBB_discard_child(CBB *cbb) {
return;
}
cbb->base->len = cbb->offset;
cbb->base->len = cbb->child->offset;
cbb->child->base = NULL;
cbb->child = NULL;
cbb->pending_len_len = 0;
cbb->pending_is_asn1 = 0;
cbb->offset = 0;
}
int CBB_add_asn1_uint64(CBB *cbb, uint64_t value) {

19
include/openssl/bytestring.h
View File

@ -238,13 +238,13 @@ struct cbb_buffer_st {
struct cbb_st {
struct cbb_buffer_st *base;
/* offset is the offset from the start of |base->buf| to the position of any
* pending length-prefix. */
size_t offset;
/* child points to a child CBB if a length-prefix is pending. */
CBB *child;
/* pending_len_len contains the number of bytes in a pending length-prefix,
* or zero if no length-prefix is pending. */
/* offset is the number of bytes from the start of |base->buf| to this |CBB|'s
* pending length prefix. */
size_t offset;
/* pending_len_len contains the number of bytes in this |CBB|'s pending
* length-prefix, or zero if no length-prefix is pending. */
uint8_t pending_len_len;
char pending_is_asn1;
/* is_top_level is true iff this is a top-level |CBB| (as opposed to a child
@ -292,12 +292,11 @@ OPENSSL_EXPORT int CBB_finish(CBB *cbb, uint8_t **out_data, size_t *out_len);
* on error. */
OPENSSL_EXPORT int CBB_flush(CBB *cbb);
/* CBB_len returns the number of bytes written to |cbb|'s top-level |CBB|. It
* may be compared before and after an operation to determine how many bytes
* were written.
/* CBB_len returns the number of bytes written to |cbb|. It does not flush
* |cbb|.
*
* It is a fatal error to call this on a CBB with any active children. This does
* not flush |cbb|. */
* To avoid unfinalized length prefixes, it is a fatal error to call this on a
* CBB with any active children. */
OPENSSL_EXPORT size_t CBB_len(const CBB *cbb);
/* CBB_add_u8_length_prefixed sets |*out_contents| to a new child of |cbb|. The

17
ssl/t1_lib.c
View File

@ -2185,7 +2185,6 @@ int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len) {
return 1;
}
size_t orig_len = CBB_len(out);
CBB extensions;
if (!CBB_add_u16_length_prefixed(out, &extensions)) {
goto err;
@ -2219,7 +2218,7 @@ int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len) {
}
if (!SSL_IS_DTLS(ssl)) {
header_len += CBB_len(&extensions) - orig_len;
header_len += 2 + CBB_len(&extensions);
if (header_len > 0xff && header_len < 0x200) {
/* Add padding to workaround bugs in F5 terminators. See RFC 7685.
*
@ -2246,10 +2245,8 @@ int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len) {
}
}
/* If only two bytes have been written then the extensions are actually empty
* and those two bytes are the zero length. In that case, we don't bother
* sending the extensions length. */
if (CBB_len(&extensions) - orig_len == 2) {
/* Discard empty extensions blocks. */
if (CBB_len(&extensions) == 0) {
CBB_discard_child(out);
}
@ -2261,8 +2258,6 @@ err:
}
int ssl_add_serverhello_tlsext(SSL *ssl, CBB *out) {
const size_t orig_len = CBB_len(out);
CBB extensions;
if (!CBB_add_u16_length_prefixed(out, &extensions)) {
goto err;
@ -2286,10 +2281,8 @@ int ssl_add_serverhello_tlsext(SSL *ssl, CBB *out) {
goto err;
}
/* If only two bytes have been written then the extensions are actually empty
* and those two bytes are the zero length. In that case, we don't bother
* sending the extensions length. */
if (CBB_len(&extensions) - orig_len == 2) {
/* Discard empty extensions blocks. */
if (CBB_len(&extensions) == 0) {
CBB_discard_child(out);
}