From 491956c8668a4a5da13120212a10b0c3183dec63 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 27 Oct 2014 02:20:16 -0400 Subject: [PATCH] Fix ECDHE_PSK key exchange. The current implementation switches the order of other_secret and psk; other_secret is first. Fix it and rewrite with CBB instead. The server half got fixed on accident in a prior refactor. Change-Id: Ib52a756aadd66e4bf22c66794447f71f4772da09 Reviewed-on: https://boringssl-review.googlesource.com/2052 Reviewed-by: Adam Langley --- ssl/s3_clnt.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 0321fd12..15684a64 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -2192,23 +2192,23 @@ int ssl3_send_client_key_exchange(SSL *s) /* ECDHE PSK ciphersuites from RFC 5489 */ if ((alg_a & SSL_aPSK) && psk_len != 0) { - uint8_t *t; + CBB cbb, child; - pms_len = 2+psk_len+2+n; - pms = OPENSSL_malloc(pms_len); - if (pms == NULL) + if (!CBB_init(&cbb, 2+psk_len+2+n)) { OPENSSL_PUT_ERROR(SSL, ssl3_send_client_key_exchange, ERR_R_MALLOC_FAILURE); goto err; } - - t = pms; - memset(t, 0, pms_len); - s2n(psk_len, t); - memcpy(t, psk, psk_len); - t += psk_len; - s2n(n, t); - memcpy(t, p, n); + if (!CBB_add_u16_length_prefixed(&cbb, &child) || + !CBB_add_bytes(&child, p, n) || + !CBB_add_u16_length_prefixed(&cbb, &child) || + !CBB_add_bytes(&child, psk, psk_len) || + !CBB_finish(&cbb, &pms, &pms_len)) + { + CBB_cleanup(&cbb); + OPENSSL_PUT_ERROR(SSL, ssl3_send_client_key_exchange, ERR_R_INTERNAL_ERROR); + goto err; + } } if (!(alg_a & SSL_aPSK)) {