diff options
author | Alex Chernyakhovsky <achernya@google.com> | 2014-07-05 09:12:34 +0400 |
---|---|---|
committer | Adam Langley <agl@google.com> | 2014-07-08 00:27:04 +0400 |
commit | 31955f91dcdb1a30bc7fd560ee9054321fc514e9 (patch) | |
tree | 66a8bdbfebfa9644ca6895a58d034f0c9b722c41 | |
parent | 9c890d4b70b3859efcd949be1476868a2a9ce0ff (diff) |
Port Certificate Request parsing to crypto/bytestring
Along the way, clean up the certificate types code to not have the
hard-coded fixed-size array.
Change-Id: If3e5978f7c5099478a3dfa37a0a7059072f5454a
Reviewed-on: https://boringssl-review.googlesource.com/1103
Reviewed-by: Adam Langley <agl@google.com>
-rw-r--r-- | ssl/s3_clnt.c | 81 | ||||
-rw-r--r-- | ssl/s3_lib.c | 10 | ||||
-rw-r--r-- | ssl/ssl.h | 1 | ||||
-rw-r--r-- | ssl/ssl3.h | 7 | ||||
-rw-r--r-- | ssl/ssl_error.c | 1 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 3 | ||||
-rw-r--r-- | ssl/t1_lib.c | 15 | ||||
-rw-r--r-- | ssl/tls1.h | 3 |
8 files changed, 43 insertions, 78 deletions
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index dfcc568f..7b140470 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1835,12 +1835,14 @@ static int ca_dn_cmp(const X509_NAME **a, const X509_NAME **b) int ssl3_get_certificate_request(SSL *s) { int ok,ret=0; - unsigned long n,nc,l; - unsigned int llen, ctype_num,i; + unsigned long n; + unsigned int i; X509_NAME *xn=NULL; - const unsigned char *p,*q; - unsigned char *d; STACK_OF(X509_NAME) *ca_sk=NULL; + CBS cbs; + CBS certificate_types; + CBS certificate_authorities; + const uint8_t *data; n=s->method->ssl_get_message(s, SSL3_ST_CR_CERT_REQ_A, @@ -1885,7 +1887,7 @@ int ssl3_get_certificate_request(SSL *s) } } - p=d=(unsigned char *)s->init_msg; + CBS_init(&cbs, (uint8_t *)s->init_msg, n); ca_sk = sk_X509_NAME_new(ca_dn_cmp); if (ca_sk == NULL) @@ -1895,33 +1897,24 @@ int ssl3_get_certificate_request(SSL *s) } /* get the certificate types */ - ctype_num= *(p++); - if (s->cert->ctypes) + if (!CBS_get_u8_length_prefixed(&cbs, &certificate_types)) { - OPENSSL_free(s->cert->ctypes); - s->cert->ctypes = NULL; + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR); + goto err; } - if (ctype_num > SSL3_CT_NUMBER) + if (!CBS_stow(&certificate_types, &s->cert->ctypes, &s->cert->ctype_num)) { - /* If we exceed static buffer copy all to cert structure */ - s->cert->ctypes = OPENSSL_malloc(ctype_num); - memcpy(s->cert->ctypes, p, ctype_num); - s->cert->ctype_num = (size_t)ctype_num; - ctype_num=SSL3_CT_NUMBER; + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + goto err; } - for (i=0; i<ctype_num; i++) - s->s3->tmp.ctype[i]= p[i]; - p+=p[-1]; if (SSL_USE_SIGALGS(s)) { - n2s(p, llen); - /* Check we have enough room for signature algorithms and - * following length value. - */ - if ((unsigned long)(p - d + llen + 2) > n) + CBS supported_signature_algorithms; + if (!CBS_get_u16_length_prefixed(&cbs, &supported_signature_algorithms)) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); - OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DATA_LENGTH_TOO_LONG); + OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR); goto err; } /* Clear certificate digests and validity flags */ @@ -1930,53 +1923,49 @@ int ssl3_get_certificate_request(SSL *s) s->cert->pkeys[i].digest = NULL; s->cert->pkeys[i].valid_flags = 0; } - if ((llen & 1) || !tls1_process_sigalgs(s, p, llen)) + if (!tls1_process_sigalgs(s, + CBS_data(&supported_signature_algorithms), + CBS_len(&supported_signature_algorithms))) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_SIGNATURE_ALGORITHMS_ERROR); goto err; } - p += llen; } /* get the CA RDNs */ - n2s(p,llen); -#if 0 -{ -FILE *out; -out=fopen("/tmp/vsign.der","w"); -fwrite(p,1,llen,out); -fclose(out); -} -#endif - - if ((unsigned long)(p - d + llen) != n) + if (!CBS_get_u16_length_prefixed(&cbs, &certificate_authorities)) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_LENGTH_MISMATCH); goto err; } - for (nc=0; nc<llen; ) + while (CBS_len(&certificate_authorities) > 0) { - n2s(p,l); - if ((l+nc+2) > llen) + CBS distinguished_name; + if (!CBS_get_u16_length_prefixed(&certificate_authorities, &distinguished_name)) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_CA_DN_TOO_LONG); goto err; } - q=p; - - if ((xn=d2i_X509_NAME(NULL,&q,l)) == NULL) + data = CBS_data(&distinguished_name); + if ((xn=d2i_X509_NAME(NULL, &data, CBS_len(&distinguished_name))) == NULL) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, ERR_R_ASN1_LIB); goto err; } - if (q != (p+l)) + if (CBS_skip(&distinguished_name, data - CBS_data(&distinguished_name))) + { + ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); + OPENSSL_PUT_ERROR(SSL, ssl3_get_server_certificate, ERR_R_INTERNAL_ERROR); + goto err; + } + if (CBS_len(&distinguished_name) != 0) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_CA_DN_LENGTH_MISMATCH); @@ -1987,14 +1976,10 @@ fclose(out); OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, ERR_R_MALLOC_FAILURE); goto err; } - - p+=l; - nc+=l+2; } /* we should setup a certificate to return.... */ s->s3->tmp.cert_req=1; - s->s3->tmp.ctype_num=ctype_num; if (s->s3->tmp.ca_names != NULL) sk_X509_NAME_pop_free(s->s3->tmp.ca_names,X509_NAME_free); s->s3->tmp.ca_names=ca_sk; diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index ec777a79..8908a6a2 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3097,15 +3097,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) const unsigned char **pctype = parg; if (s->server || !s->s3->tmp.cert_req) return 0; - if (s->cert->ctypes) - { - if (pctype) - *pctype = s->cert->ctypes; - return (int)s->cert->ctype_num; - } if (pctype) - *pctype = s->s3->tmp.ctype; - return (int)s->s3->tmp.ctype_num; + *pctype = s->cert->ctypes; + return (int)s->cert->ctype_num; } case SSL_CTRL_SET_CLIENT_CERT_TYPES: @@ -2938,5 +2938,6 @@ void ERR_load_SSL_strings(void); #define SSL_R_INAPPROPRIATE_FALLBACK 436 #define SSL_R_CLIENTHELLO_PARSE_FAILED 437 #define SSL_R_CONNECTION_REJECTED 438 +#define SSL_R_DECODE_ERROR 439 #endif @@ -386,11 +386,6 @@ typedef struct ssl3_buffer_st #define SSL3_CT_RSA_EPHEMERAL_DH 5 #define SSL3_CT_DSS_EPHEMERAL_DH 6 #define SSL3_CT_FORTEZZA_DMS 20 -/* SSL3_CT_NUMBER is used to size arrays and it must be large - * enough to contain all of the cert types defined either for - * SSLv3 and TLSv1. - */ -#define SSL3_CT_NUMBER 9 #define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 @@ -511,8 +506,6 @@ typedef struct ssl3_state_st /* used for certificate requests */ int cert_req; - size_t ctype_num; - unsigned char ctype[SSL3_CT_NUMBER]; STACK_OF(X509_NAME) *ca_names; int use_rsa_tmp; diff --git a/ssl/ssl_error.c b/ssl/ssl_error.c index 9d2c4bd7..ac61bcfc 100644 --- a/ssl/ssl_error.c +++ b/ssl/ssl_error.c @@ -272,6 +272,7 @@ const ERR_STRING_DATA SSL_error_string_data[] = { {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_D2I_ECDSA_SIG), "D2I_ECDSA_SIG"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DATA_BETWEEN_CCS_AND_FINISHED), "DATA_BETWEEN_CCS_AND_FINISHED"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DATA_LENGTH_TOO_LONG), "DATA_LENGTH_TOO_LONG"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECODE_ERROR), "DECODE_ERROR"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECRYPTION_FAILED), "DECRYPTION_FAILED"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC), "DECRYPTION_FAILED_OR_BAD_RECORD_MAC"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG), "DH_PUBLIC_VALUE_LENGTH_IS_WRONG"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index dfa7df46..714e3200 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -565,8 +565,7 @@ typedef struct cert_st CERT_PKEY pkeys[SSL_PKEY_NUM]; /* Certificate types (received or sent) in certificate request - * message. On receive this is only set if number of certificate - * types exceeds SSL3_CT_NUMBER. + * message. */ unsigned char *ctypes; size_t ctype_num; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6f3089da..7bcdf90d 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -3354,6 +3354,9 @@ int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize) /* Extension ignored for inappropriate versions */ if (!SSL_USE_SIGALGS(s)) return 1; + /* Length must be even */ + if (dsize % 2 != 0) + return 0; /* Should never happen */ if (!c) return 0; @@ -3915,16 +3918,8 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain, { const unsigned char *ctypes; int ctypelen; - if (c->ctypes) - { - ctypes = c->ctypes; - ctypelen = (int)c->ctype_num; - } - else - { - ctypes = (unsigned char *)s->s3->tmp.ctype; - ctypelen = s->s3->tmp.ctype_num; - } + ctypes = c->ctypes; + ctypelen = (int)c->ctype_num; for (i = 0; i < ctypelen; i++) { if (ctypes[i] == check_type) @@ -708,9 +708,6 @@ SSL_CTX_callback_ctrl(ssl,SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB,(void (*)(void))cb) #define TLS_CT_ECDSA_FIXED_ECDH 66 #define TLS_CT_GOST94_SIGN 21 #define TLS_CT_GOST01_SIGN 22 -/* when correcting this number, correct also SSL3_CT_NUMBER in ssl3.h (see - * comment there) */ -#define TLS_CT_NUMBER 9 #define TLS1_FINISH_MAC_LENGTH 12 |