1
0
mirror of https://github.com/openbsd/src.git synced 2025-01-10 06:47:55 -08:00

Convert ssl3_get_server_kex_ecdhe() to CBS, simplifying tls1_check_curve()

in the process. This also fixes a long standing bug where
tls1_ec_curve_id2nid() is called with only one byte of the curve ID.

ok beck@ miod@
This commit is contained in:
jsing 2016-11-05 08:26:36 +00:00
parent f208b8a077
commit 4665520474
3 changed files with 41 additions and 62 deletions

View File

@ -1,4 +1,4 @@
/* $OpenBSD: s3_clnt.c,v 1.143 2016/11/04 19:11:43 jsing Exp $ */
/* $OpenBSD: s3_clnt.c,v 1.144 2016/11/05 08:26:36 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -1175,64 +1175,63 @@ ssl3_get_server_kex_dhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
static int
ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
{
CBS cbs, ecpoint;
uint8_t curve_type;
uint16_t curve_id;
EC_POINT *srvr_ecpoint = NULL;
EC_KEY *ecdh = NULL;
BN_CTX *bn_ctx = NULL;
const EC_GROUP *group;
EC_GROUP *ngroup;
SESS_CERT *sc;
unsigned char *p;
int al, param_len;
long alg_a, n;
int curve_nid = 0;
int encoded_pt_len = 0;
int curve_nid;
long alg_a;
int al;
alg_a = s->s3->tmp.new_cipher->algorithm_auth;
n = *nn;
p = *pp;
sc = s->session->sess_cert;
if (*nn < 0)
goto err;
CBS_init(&cbs, *pp, *nn);
/*
* Extract EC parameters and the server's ephemeral ECDH public key.
*/
if ((ecdh = EC_KEY_new()) == NULL) {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
goto err;
}
/*
* Extract elliptic curve parameters and the server's ephemeral ECDH
* public key. Keep accumulating lengths of various components in
* param_len and make sure it never exceeds n.
*/
/*
* XXX: For now we only support named (not generic) curves
* and the ECParameters in this case is just three bytes.
*/
param_len = 3;
if (param_len > n) {
/* Only named curves are supported. */
if (!CBS_get_u8(&cbs, &curve_type) ||
curve_type != NAMED_CURVE_TYPE ||
!CBS_get_u16(&cbs, &curve_id)) {
al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT);
goto f_err;
}
/*
* Check curve is one of our preferences, if not server has
* sent an invalid curve.
* Check that the curve is one of our preferences - if it is not,
* the server has sent us an invalid curve.
*/
if (tls1_check_curve(s, p, param_len) != 1) {
if (tls1_check_curve(s, curve_id) != 1) {
al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_WRONG_CURVE);
goto f_err;
}
if ((curve_nid = tls1_ec_curve_id2nid(*(p + 2))) == 0) {
if ((curve_nid = tls1_ec_curve_id2nid(curve_id)) == 0) {
al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
goto f_err;
}
ngroup = EC_GROUP_new_by_curve_name(curve_nid);
if (ngroup == NULL) {
if ((ngroup = EC_GROUP_new_by_curve_name(curve_nid)) == NULL) {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_EC_LIB);
goto err;
}
@ -1244,31 +1243,23 @@ ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
group = EC_KEY_get0_group(ecdh);
p += 3;
/* Next, get the encoded ECPoint */
if (((srvr_ecpoint = EC_POINT_new(group)) == NULL) ||
((bn_ctx = BN_CTX_new()) == NULL)) {
if ((srvr_ecpoint = EC_POINT_new(group)) == NULL ||
(bn_ctx = BN_CTX_new()) == NULL) {
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
goto err;
}
if (param_len + 1 > n)
if (!CBS_get_u8_length_prefixed(&cbs, &ecpoint))
goto truncated;
encoded_pt_len = *p;
/* length of encoded point */
p += 1;
param_len += (1 + encoded_pt_len);
if ((param_len > n) || (EC_POINT_oct2point(group, srvr_ecpoint,
p, encoded_pt_len, bn_ctx) == 0)) {
if (EC_POINT_oct2point(group, srvr_ecpoint, CBS_data(&ecpoint),
CBS_len(&ecpoint), bn_ctx) == 0) {
al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_ECPOINT);
goto f_err;
}
n -= param_len;
p += encoded_pt_len;
/*
* The ECC/TLS specification does not mention the use of DSA to sign
* ECParameters in the server key exchange message. We do support RSA
@ -1288,8 +1279,8 @@ ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
BN_CTX_free(bn_ctx);
EC_POINT_free(srvr_ecpoint);
*nn = n;
*pp = p;
*nn = CBS_len(&cbs);
*pp = (unsigned char *)CBS_data(&cbs);
return (1);

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssl_locl.h,v 1.134 2016/11/04 19:11:43 jsing Exp $ */
/* $OpenBSD: ssl_locl.h,v 1.135 2016/11/05 08:26:36 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -772,9 +772,9 @@ int ssl_ok(SSL *s);
int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s);
int tls1_ec_curve_id2nid(uint16_t curve_id);
uint16_t tls1_ec_nid2curve_id(int nid);
int tls1_check_curve(SSL *s, const unsigned char *p, size_t len);
int tls1_ec_curve_id2nid(const uint16_t curve_id);
uint16_t tls1_ec_nid2curve_id(const int nid);
int tls1_check_curve(SSL *s, const uint16_t curve_id);
int tls1_get_shared_curve(SSL *s);
unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p,

View File

@ -1,4 +1,4 @@
/* $OpenBSD: t1_lib.c,v 1.93 2016/10/19 16:38:40 jsing Exp $ */
/* $OpenBSD: t1_lib.c,v 1.94 2016/11/05 08:26:37 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -282,7 +282,7 @@ static const uint16_t eccurves_default[] = {
};
int
tls1_ec_curve_id2nid(uint16_t curve_id)
tls1_ec_curve_id2nid(const uint16_t curve_id)
{
/* ECC curves from draft-ietf-tls-ecc-12.txt (Oct. 17, 2005) */
if ((curve_id < 1) ||
@ -405,27 +405,15 @@ tls1_get_curvelist(SSL *s, int client_curves, const uint16_t **pcurves,
/* Check that a curve is one of our preferences. */
int
tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
tls1_check_curve(SSL *s, const uint16_t curve_id)
{
CBS cbs;
const uint16_t *curves;
size_t curveslen, i;
uint8_t type;
uint16_t cid;
CBS_init(&cbs, p, len);
/* Only named curves are supported. */
if (CBS_len(&cbs) != 3 ||
!CBS_get_u8(&cbs, &type) ||
type != NAMED_CURVE_TYPE ||
!CBS_get_u16(&cbs, &cid))
return (0);
tls1_get_curvelist(s, 0, &curves, &curveslen);
for (i = 0; i < curveslen; i++) {
if (curves[i] == cid)
if (curves[i] == curve_id)
return (1);
}
return (0);