Port ssl3_get_client_key_exchange to CBS. Change-Id: I065554d058395322a4ac675155bfe66c874b47ad Reviewed-on: https://boringssl-review.googlesource.com/1171 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 2882c79..3ea2c4b 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -515,7 +515,6 @@ #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0x00000010L #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L #define SSL_OP_SAFARI_ECDHE_ECDSA_BUG 0x00000040L -#define SSL_OP_SSLEAY_080_CLIENT_DH_BUG 0x00000080L #define SSL_OP_TLS_D5_BUG 0x00000100L #define SSL_OP_TLS_BLOCK_PADDING_BUG 0x00000200L
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 5fb07e8..31c2e37 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c
@@ -2023,11 +2023,12 @@ int ssl3_get_client_key_exchange(SSL *s) { - int i,al,ok; + int al,ok; long n; + CBS client_key_exchange; unsigned long alg_k; unsigned long alg_a; - unsigned char *p; + uint8_t *premaster_secret = NULL; RSA *rsa=NULL; EVP_PKEY *pkey=NULL; #ifndef OPENSSL_NO_DH @@ -2054,7 +2055,7 @@ &ok); if (!ok) return((int)n); - p = s->init_msg; + CBS_init(&client_key_exchange, s->init_msg, n); alg_k=s->s3->tmp.new_cipher->algorithm_mkey; alg_a=s->s3->tmp.new_cipher->algorithm_auth; @@ -2062,47 +2063,58 @@ #ifndef OPENSSL_NO_PSK if (alg_a & SSL_aPSK) { + CBS psk_identity; + int psk_err = -1; unsigned char *t = NULL; unsigned char pre_ms[PSK_MAX_PSK_LEN*2+4]; unsigned int pre_ms_len = 0; - int psk_err = 1; - char tmp_id[PSK_MAX_IDENTITY_LEN+1]; - al=SSL_AD_HANDSHAKE_FAILURE; - - n2s(p, i); - if (n != i+2 && !(alg_k & SSL_kEECDH)) + /* If using PSK, the ClientKeyExchange contains a + * psk_identity. If PSK, then this is the only field + * in the message. */ + if (!CBS_get_u16_length_prefixed(&client_key_exchange, &psk_identity) || + ((alg_k & SSL_kPSK) && CBS_len(&client_key_exchange) != 0)) { - OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_LENGTH_MISMATCH); + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DECODE_ERROR); + al = SSL_AD_DECODE_ERROR; goto psk_err; } - if (i > PSK_MAX_IDENTITY_LEN) - { - OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DATA_LENGTH_TOO_LONG); - goto psk_err; - } + if (s->psk_server_callback == NULL) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_PSK_NO_SERVER_CB); + al = SSL_AD_INTERNAL_ERROR; goto psk_err; } - /* Create guaranteed NUL-terminated identity - * string for the callback */ - memcpy(tmp_id, p, i); - memset(tmp_id+i, 0, PSK_MAX_IDENTITY_LEN+1-i); - psk_len = s->psk_server_callback(s, tmp_id, psk, sizeof(psk)); + if (CBS_len(&psk_identity) > PSK_MAX_IDENTITY_LEN || + CBS_contains_zero_byte(&psk_identity)) + { + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DATA_LENGTH_TOO_LONG); + al = SSL_AD_ILLEGAL_PARAMETER; + goto psk_err; + } + if (!CBS_strdup(&psk_identity, &s->session->psk_identity)) + { + al = SSL_AD_INTERNAL_ERROR; + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); + goto psk_err; + } + + /* Look up the key for the identity. */ + psk_len = s->psk_server_callback(s, s->session->psk_identity, psk, sizeof(psk)); if (psk_len > PSK_MAX_PSK_LEN) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_INTERNAL_ERROR); + al = SSL_AD_INTERNAL_ERROR; goto psk_err; } else if (psk_len == 0) { /* PSK related to the given identity not found */ OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_PSK_IDENTITY_NOT_FOUND); - al=SSL_AD_UNKNOWN_PSK_IDENTITY; + al = SSL_AD_UNKNOWN_PSK_IDENTITY; goto psk_err; } if (!(alg_k & SSL_kEECDH)) @@ -2120,18 +2132,7 @@ s->method->ssl3_enc->generate_master_secret(s, s->session->master_key, pre_ms, pre_ms_len); } - if (s->session->psk_identity != NULL) - OPENSSL_free(s->session->psk_identity); - s->session->psk_identity = BUF_strdup(tmp_id); - OPENSSL_cleanse(tmp_id, PSK_MAX_IDENTITY_LEN+1); - if (s->session->psk_identity == NULL) - { - OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); - goto psk_err; - } - p += i; - n -= (i + 2); psk_err = 0; psk_err: OPENSSL_cleanse(pre_ms, sizeof(pre_ms)); @@ -2140,9 +2141,9 @@ } #endif /* OPENSSL_NO_PSK */ - if (0) {} - else if (alg_k & SSL_kRSA) + if (alg_k & SSL_kRSA) { + CBS encrypted_premaster_secret; unsigned char rand_premaster_secret[SSL_MAX_MASTER_KEY_LENGTH]; int decrypt_len, decrypt_good_mask; unsigned char version_good; @@ -2177,11 +2178,18 @@ rsa=pkey->pkey.rsa; } - /* TLS and [incidentally] DTLS{0xFEFF} */ - if (s->version > SSL3_VERSION && s->version != DTLS1_BAD_VER) + /* TLS and [incidentally] DTLS{0xFEFF} + * + * TODO(davidben): Should this (and + * ssl3_send_client_key_exchange) include DTLS1_BAD_VER? + * Alternatively, get rid of DTLS1_BAD_VER? + */ + if (s->version > SSL3_VERSION) { - n2s(p,i); - if (n != i+2) + CBS copy = client_key_exchange; + if (!CBS_get_u16_length_prefixed(&client_key_exchange, + &encrypted_premaster_secret) || + CBS_len(&client_key_exchange) != 0) { if (!(s->options & SSL_OP_TLS_D5_BUG)) { @@ -2190,11 +2198,11 @@ goto f_err; } else - p-=2; + encrypted_premaster_secret = copy; } - else - n=i; } + else + encrypted_premaster_secret = client_key_exchange; /* Reject overly short RSA ciphertext because we want to be * sure that the buffer size makes it safe to iterate over the @@ -2202,7 +2210,7 @@ * (SSL_MAX_MASTER_KEY_LENGTH). The actual expected size is * larger due to RSA padding, but the bound is sufficient to be * safe. */ - if (n < SSL_MAX_MASTER_KEY_LENGTH) + if (CBS_len(&encrypted_premaster_secret) < SSL_MAX_MASTER_KEY_LENGTH) { al = SSL_AD_DECRYPT_ERROR; OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DECRYPTION_FAILED); @@ -2219,7 +2227,21 @@ sizeof(rand_premaster_secret)) <= 0) goto err; - decrypt_len = RSA_private_decrypt((int)n,p,p,rsa,RSA_PKCS1_PADDING); + /* Allocate a buffer large enough for an RSA decryption. */ + premaster_secret = OPENSSL_malloc(RSA_size(rsa)); + if (premaster_secret == NULL) + { + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); + goto err; + } + + decrypt_len = RSA_private_decrypt( + CBS_len(&encrypted_premaster_secret), + CBS_data(&encrypted_premaster_secret), + premaster_secret, + rsa, + RSA_PKCS1_PADDING); + ERR_clear_error(); /* decrypt_len should be SSL_MAX_MASTER_KEY_LENGTH. @@ -2233,8 +2255,8 @@ * number check as a "bad version oracle". Thus version checks * are done in constant time and are treated like any other * decryption error. */ - version_good = p[0] ^ (s->client_version>>8); - version_good |= p[1] ^ (s->client_version&0xff); + version_good = premaster_secret[0] ^ (s->client_version>>8); + version_good |= premaster_secret[1] ^ (s->client_version&0xff); /* The premaster secret must contain the same version number as * the ClientHello to detect version rollback attacks @@ -2256,8 +2278,8 @@ workaround_mask |= workaround_mask >> 1; workaround_mask = ~((workaround_mask & 1) - 1); - workaround = p[0] ^ (s->version>>8); - workaround |= p[1] ^ (s->version&0xff); + workaround = premaster_secret[0] ^ (s->version>>8); + workaround |= premaster_secret[1] ^ (s->version&0xff); /* If workaround_mask is 0xff (i.e. there was a version * mismatch) then we copy the value of workaround over @@ -2288,42 +2310,39 @@ decrypt_good_mask &= 1; decrypt_good_mask--; - /* Now copy rand_premaster_secret over p using + /* Now copy rand_premaster_secret over premaster_secret using * decrypt_good_mask. */ for (j = 0; j < sizeof(rand_premaster_secret); j++) { - p[j] = (p[j] & decrypt_good_mask) | + premaster_secret[j] = (premaster_secret[j] & decrypt_good_mask) | (rand_premaster_secret[j] & ~decrypt_good_mask); } s->session->master_key_length= s->method->ssl3_enc->generate_master_secret(s, s->session->master_key, - p,sizeof(rand_premaster_secret)); - OPENSSL_cleanse(p,sizeof(rand_premaster_secret)); + premaster_secret, sizeof(rand_premaster_secret)); + + OPENSSL_cleanse(premaster_secret, sizeof(rand_premaster_secret)); + OPENSSL_free(premaster_secret); + premaster_secret = NULL; } #ifndef OPENSSL_NO_DH else if (alg_k & (SSL_kEDH|SSL_kDHr|SSL_kDHd)) { + CBS dh_Yc; int idx = -1; + int dh_len; EVP_PKEY *skey = NULL; - if (n) - n2s(p,i); - else - i = 0; - if (n && n != i+2) + + if (!CBS_get_u16_length_prefixed(&client_key_exchange, &dh_Yc) || + CBS_len(&client_key_exchange) != 0) { - if (!(s->options & SSL_OP_SSLEAY_080_CLIENT_DH_BUG)) - { - OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG); - goto err; - } - else - { - p-=2; - i=(int)n; - } + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG); + al = SSL_R_DECODE_ERROR; + goto f_err; } + if (alg_k & SSL_kDHr) idx = SSL_PKEY_DH_RSA; else if (alg_k & SSL_kDHd) @@ -2350,9 +2369,16 @@ else dh_srvr=s->s3->tmp.dh; - if (n == 0L) + if (CBS_len(&dh_Yc) == 0) { - /* Get pubkey from cert */ + /* Get pubkey from the client certificate. This is the + * 'implicit' case of ClientDiffieHellman. + * + * TODO(davidben): Either lose this code or fix a bug + * (or get the spec changed): if there is a fixed_dh + * client certificate, per spec, the 'implicit' mode + * MUST be used. This logic will still accept 'explicit' + * mode. */ EVP_PKEY *clkey=X509_get_pubkey(s->session->peer); if (clkey) { @@ -2369,16 +2395,23 @@ pub = dh_clnt->pub_key; } else - pub=BN_bin2bn(p,i,NULL); + pub = BN_bin2bn(CBS_data(&dh_Yc), CBS_len(&dh_Yc), NULL); if (pub == NULL) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_BN_LIB); goto err; } - i=DH_compute_key(p,pub,dh_srvr); + /* Allocate a buffer for the premaster secret. */ + premaster_secret = OPENSSL_malloc(DH_size(dh_srvr)); + if (premaster_secret == NULL) + { + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); + goto err; + } - if (i <= 0) + dh_len = DH_compute_key(premaster_secret, pub, dh_srvr); + if (dh_len <= 0) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_DH_LIB); BN_clear_free(pub); @@ -2394,8 +2427,10 @@ pub=NULL; s->session->master_key_length= s->method->ssl3_enc->generate_master_secret(s, - s->session->master_key,p,i); - OPENSSL_cleanse(p,i); + s->session->master_key, premaster_secret, dh_len); + OPENSSL_cleanse(premaster_secret, dh_len); + OPENSSL_free(premaster_secret); + premaster_secret = NULL; if (dh_clnt) return 2; } @@ -2405,15 +2440,10 @@ else if (alg_k & (SSL_kEECDH|SSL_kECDHr|SSL_kECDHe)) { int ret = 1; - int field_size = 0; + int field_size = 0, ecdh_len; const EC_KEY *tkey; const EC_GROUP *group; const BIGNUM *priv_key; -#ifndef OPENSSL_NO_PSK - unsigned char *pre_ms; - unsigned int pre_ms_len; - unsigned char *t; -#endif /* OPENSSL_NO_PSK */ /* initialize structures for server's ECDH key pair */ if ((srvr_ecdh = EC_KEY_new()) == NULL) @@ -2453,7 +2483,7 @@ goto err; } - if (n == 0L) + if (CBS_len(&client_key_exchange) == 0) { /* Client Publickey was in Client Certificate */ @@ -2493,44 +2523,53 @@ } else { + CBS ecdh_Yc; + /* Get client's public key from encoded point * in the ClientKeyExchange message. */ + if (!CBS_get_u8_length_prefixed(&client_key_exchange, &ecdh_Yc) || + CBS_len(&client_key_exchange) != 0) + { + al = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, SSL_R_DECODE_ERROR); + goto f_err; + } + if ((bn_ctx = BN_CTX_new()) == NULL) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); goto err; } - /* Get encoded point length */ - i = *p; - p += 1; - if (n != 1 + i) + if (!EC_POINT_oct2point(group, clnt_ecpoint, + CBS_data(&ecdh_Yc), CBS_len(&ecdh_Yc), bn_ctx)) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_EC_LIB); goto err; } - if (EC_POINT_oct2point(group, - clnt_ecpoint, p, i, bn_ctx) == 0) - { - OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_EC_LIB); - goto err; - } - /* p is pointing to somewhere in the buffer - * currently, so set it to the start - */ - p=(unsigned char *)s->init_buf->data; } - /* Compute the shared pre-master secret */ + /* Allocate a buffer for both the secret and the PSK. */ field_size = EC_GROUP_get_degree(group); if (field_size <= 0) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_ECDH_LIB); goto err; } - i = ECDH_compute_key(p, (field_size+7)/8, clnt_ecpoint, srvr_ecdh, NULL); - if (i <= 0) + + ecdh_len = (field_size + 7) / 8; + premaster_secret = OPENSSL_malloc(ecdh_len); + if (premaster_secret == NULL) + { + OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_MALLOC_FAILURE); + goto err; + } + + /* Compute the shared pre-master secret */ + ecdh_len = ECDH_compute_key(premaster_secret, + ecdh_len, clnt_ecpoint, srvr_ecdh, NULL); + if (ecdh_len <= 0) { OPENSSL_PUT_ERROR(SSL, ssl3_get_client_key_exchange, ERR_R_ECDH_LIB); goto err; @@ -2545,9 +2584,13 @@ #ifndef OPENSSL_NO_PSK /* ECDHE PSK ciphersuites from RFC 5489 */ - if ((alg_a & SSL_aPSK) && psk_len != 0) + if ((alg_a & SSL_aPSK) && psk_len != 0) { - pre_ms_len = 2+psk_len+2+i; + unsigned char *pre_ms; + unsigned int pre_ms_len; + unsigned char *t; + + pre_ms_len = 2+psk_len+2+ecdh_len; pre_ms = OPENSSL_malloc(pre_ms_len); if (pre_ms == NULL) { @@ -2560,8 +2603,8 @@ s2n(psk_len, t); memcpy(t, psk, psk_len); t += psk_len; - s2n(i, t); - memcpy(t, p, i); + s2n(ecdh_len, t); + memcpy(t, premaster_secret, ecdh_len); s->session->master_key_length = s->method->ssl3_enc \ -> generate_master_secret(s, s->session->master_key, pre_ms, pre_ms_len); @@ -2574,10 +2617,11 @@ /* Compute the master secret */ s->session->master_key_length = s->method->ssl3_enc \ -> generate_master_secret(s, - s->session->master_key, p, i); + s->session->master_key, premaster_secret, ecdh_len); } - OPENSSL_cleanse(p, i); + OPENSSL_cleanse(premaster_secret, ecdh_len); + OPENSSL_free(premaster_secret); return ret; } #endif @@ -2597,6 +2641,8 @@ f_err: ssl3_send_alert(s,SSL3_AL_FATAL,al); err: + if (premaster_secret) + OPENSSL_free(premaster_secret); #ifndef OPENSSL_NO_ECDH EVP_PKEY_free(clnt_pub_pkey); EC_POINT_free(clnt_ecpoint);