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

TLS extension limit check fixes.

Browse Source 
Fix limit checks in ssl_add_clienthello_tlsext and
ssl_add_serverhello_tlsext.

Some of the limit checks reference p rather than ret. p is the original
buffer position, not the current one. Fix those and rename p to orig so
it's clearer.
This commit is contained in:
Adam Langley 2014-06-20 12:00:00 -07:00
parent 2970779684
commit b0c235ed36
4 changed files with 54 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ssl/s23_clnt.c
View File

@ -477,7 +477,10 @@ static int ssl23_client_hello(SSL *s)
{
/* create Client Hello in SSL 3.0/TLS 1.0 format */
/* do the record header (5 bytes) and handshake message header (4 bytes) last */
/* do the record header (5 bytes) and handshake message
* header (4 bytes) last. Note: the final argument to
* ssl_add_clienthello_tlsext below depends on the size
* of this prefix. */
d = p = &(buf[9]);
*(p++) = version_major;
@ -520,7 +523,11 @@ static int ssl23_client_hello(SSL *s)
OPENSSL_PUT_ERROR(SSL, ssl23_client_hello, SSL_R_CLIENTHELLO_TLSEXT);
return -1;
}
if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL)
/* The buffer includes the 5 byte record header, so
* subtract it to compute hlen for
* ssl_add_clienthello_tlsext. */
if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH, p-buf-5)) == NULL)
{
OPENSSL_PUT_ERROR(SSL, ssl23_client_hello, ERR_R_INTERNAL_ERROR);
return -1;

6
ssl/s3_clnt.c
View File

@ -794,7 +794,9 @@ int ssl3_client_hello(SSL *s)
ssl_fill_hello_random(s, 0, p,
sizeof(s->s3->client_random));
/* Do the message type and length last */
/* Do the message type and length last.
* Note: the final argument to ssl_add_clienthello_tlsext below
* depends on the size of this prefix. */
d=p= ssl_handshake_start(s);
/* version indicates the negotiated version: for example from
@ -899,7 +901,7 @@ int ssl3_client_hello(SSL *s)
OPENSSL_PUT_ERROR(SSL, ssl3_client_hello, SSL_R_CLIENTHELLO_TLSEXT);
goto err;
}
if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL)
if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH, p-buf)) == NULL)
{
OPENSSL_PUT_ERROR(SSL, ssl3_client_hello, ERR_R_INTERNAL_ERROR);
goto err;

4
ssl/ssl_locl.h
View File

@ -1298,8 +1298,8 @@ int tls1_shared_list(SSL *s,
const unsigned char *l1, size_t l1len,
const unsigned char *l2, size_t l2len,
int nmatch);
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit);
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit);
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit, size_t header_len);
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit);
int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n);
int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n);

75
ssl/t1_lib.c
View File

@ -1098,10 +1098,14 @@ static int byte_compare(const void *in_a, const void *in_b)
return 0;
}
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
/* header_len is the length of the ClientHello header written so far, used to
* compute padding. It does not include the record header. Pass 0 if no padding
* is to be done. */
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit, size_t header_len)
{
int extdatalen=0;
unsigned char *ret = p;
unsigned char *ret = buf;
unsigned char *orig = buf;
#ifndef OPENSSL_NO_EC
/* See if we support any ECC ciphersuites */
int using_ecc = 0;
@ -1130,7 +1134,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
/* don't add extensions for SSLv3 unless doing secure renegotiation */
if (s->client_version == SSL3_VERSION
&& !s->s3->send_connection_binding)
return p;
return orig;
ret+=2;
@ -1179,7 +1183,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
return NULL;
}
if((limit - p - 4 - el) < 0) return NULL;
if((limit - ret - 4 - el) < 0) return NULL;
s2n(TLSEXT_TYPE_renegotiate,ret);
s2n(el,ret);
@ -1420,7 +1424,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
ssl_add_clienthello_use_srtp_ext(s, 0, &el, 0);
if((limit - p - 4 - el) < 0) return NULL;
if((limit - ret - 4 - el) < 0) return NULL;
s2n(TLSEXT_TYPE_use_srtp,ret);
s2n(el,ret);
@ -1489,44 +1493,43 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
#ifdef TLSEXT_TYPE_padding
/* Add padding to workaround bugs in F5 terminators.
* See https://tools.ietf.org/html/draft-agl-tls-padding-02
* See https://tools.ietf.org/html/draft-agl-tls-padding-03
*
* NB: because this code works out the length of all existing
* extensions it MUST always appear last.
*/
{
int hlen = ret - (unsigned char *)s->init_buf->data;
/* The code in s23_clnt.c to build ClientHello messages includes the
* 5-byte record header in the buffer, while the code in s3_clnt.c does
* not. */
if (s->state == SSL23_ST_CW_CLNT_HELLO_A)
hlen -= 5;
if (hlen > 0xff && hlen < 0x200)
* extensions it MUST always appear last. */
if (header_len > 0)
{
hlen = 0x200 - hlen;
if (hlen >= 4)
hlen -= 4;
else
hlen = 0;
header_len += ret - orig;
if (header_len > 0xff && header_len < 0x200)
{
size_t padding_len = 0x200 - header_len;
if (padding_len >= 4)
padding_len -= 4;
else
padding_len = 0;
if (limit - ret - 4 - (long)padding_len < 0)
return NULL;
s2n(TLSEXT_TYPE_padding, ret);
s2n(hlen, ret);
memset(ret, 0, hlen);
ret += hlen;
s2n(TLSEXT_TYPE_padding, ret);
s2n(padding_len, ret);
memset(ret, 0, padding_len);
ret += padding_len;
}
}
#endif
if ((extdatalen = ret-p-2) == 0)
return p;
if ((extdatalen = ret-orig-2)== 0)
return orig;
s2n(extdatalen,p);
s2n(extdatalen, orig);
return ret;
}
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, unsigned char *limit)
{
int extdatalen=0;
unsigned char *ret = p;
unsigned char *orig = buf;
unsigned char *ret = buf;
#ifndef OPENSSL_NO_NEXTPROTONEG
int next_proto_neg_seen;
#endif
@ -1538,7 +1541,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha
#endif
/* don't add extensions for SSLv3, unless doing secure renegotiation */
if (s->version == SSL3_VERSION && !s->s3->send_connection_binding)
return p;
return orig;
ret+=2;
if (ret>=limit) return NULL; /* this really never occurs, but ... */
@ -1561,7 +1564,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha
return NULL;
}
if((limit - p - 4 - el) < 0) return NULL;
if((limit - ret - 4 - el) < 0) return NULL;
s2n(TLSEXT_TYPE_renegotiate,ret);
s2n(el,ret);
@ -1642,7 +1645,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha
ssl_add_serverhello_use_srtp_ext(s, 0, &el, 0);
if((limit - p - 4 - el) < 0) return NULL;
if((limit - ret - 4 - el) < 0) return NULL;
s2n(TLSEXT_TYPE_use_srtp,ret);
s2n(el,ret);
@ -1852,10 +1855,10 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha
s2n(0,ret);
}
if ((extdatalen = ret-p-2)== 0)
return p;
if ((extdatalen = ret-orig-2) == 0)
return orig;
s2n(extdatalen,p);
s2n(extdatalen, orig);
return ret;
}