Clean up |ECDH_compute_key|.

1. Check for the presence of the private key before allocating or
   computing anything.
2. Check the return value of |BN_CTX_get|.
3. Don't bother computing the Y coordinate since it is not used.
4. Remove conditional logic in cleanup section.

Change-Id: I4d8611603363c7e5d16a8e9f1d6c3a56809f27ae
Reviewed-on: https://boringssl-review.googlesource.com/6171
Reviewed-by: Adam Langley <alangley@gmail.com>
This commit is contained in:
Brian Smith 2015-10-08 11:52:56 -10:00 committed by Adam Langley
parent 274341dd6e
commit 0dc2a8aee2
2 changed files with 35 additions and 43 deletions

View File

@ -75,33 +75,27 @@
int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
EC_KEY *priv_key, void *(*KDF)(const void *in, size_t inlen,
void *out, size_t *outlen)) {
BN_CTX *ctx;
EC_POINT *tmp = NULL;
BIGNUM *x = NULL, *y = NULL;
const BIGNUM *priv;
const EC_GROUP *group;
int ret = -1;
size_t buflen;
uint8_t *buf = NULL;
if ((ctx = BN_CTX_new()) == NULL) {
goto err;
}
BN_CTX_start(ctx);
x = BN_CTX_get(ctx);
y = BN_CTX_get(ctx);
priv = EC_KEY_get0_private_key(priv_key);
EC_KEY *priv_key,
void *(*kdf)(const void *in, size_t inlen, void *out,
size_t *outlen)) {
const BIGNUM *const priv = EC_KEY_get0_private_key(priv_key);
if (priv == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_NO_PRIVATE_VALUE);
goto err;
return -1;
}
group = EC_KEY_get0_group(priv_key);
BN_CTX *ctx = BN_CTX_new();
if (ctx == NULL) {
return -1;
}
BN_CTX_start(ctx);
tmp = EC_POINT_new(group);
int ret = -1;
size_t buflen = 0;
uint8_t *buf = NULL;
const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
EC_POINT *tmp = EC_POINT_new(group);
if (tmp == NULL) {
OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
goto err;
@ -112,7 +106,13 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
goto err;
}
if (!EC_POINT_get_affine_coordinates_GFp(group, tmp, x, y, ctx)) {
BIGNUM *x = BN_CTX_get(ctx);
if (!x) {
OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
goto err;
}
if (!EC_POINT_get_affine_coordinates_GFp(group, tmp, x, NULL, ctx)) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
goto err;
}
@ -129,33 +129,25 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
goto err;
}
if (KDF != 0) {
if (KDF(buf, buflen, out, &outlen) == NULL) {
if (kdf != NULL) {
if (kdf(buf, buflen, out, &outlen) == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_KDF_FAILED);
goto err;
}
ret = outlen;
} else {
/* no KDF, just copy as much as we can */
if (outlen > buflen) {
if (buflen < outlen) {
outlen = buflen;
}
memcpy(out, buf, outlen);
ret = outlen;
}
ret = outlen;
err:
if (tmp) {
EC_POINT_free(tmp);
}
if (ctx) {
BN_CTX_end(ctx);
}
if (ctx) {
BN_CTX_free(ctx);
}
if (buf) {
OPENSSL_free(buf);
}
OPENSSL_free(buf);
EC_POINT_free(tmp);
BN_CTX_end(ctx);
BN_CTX_free(ctx);
return ret;
}

View File

@ -80,14 +80,14 @@ extern "C" {
/* ECDH_compute_key calculates the shared key between |pub_key| and |priv_key|.
* If |KDF| is not NULL, then it is called with the bytes of the shared key and
* the parameter |out|. When |KDF| returns, the value of |*outlen| becomes the
* If |kdf| is not NULL, then it is called with the bytes of the shared key and
* the parameter |out|. When |kdf| returns, the value of |*outlen| becomes the
* return value. Otherwise, as many bytes of the shared key as will fit are
* copied directly to, at most, |outlen| bytes at |out|. It returns the number
* of bytes written to |out|, or -1 on error. */
OPENSSL_EXPORT int ECDH_compute_key(void *out, size_t outlen,
const EC_POINT *pub_key, EC_KEY *priv_key,
void *(*KDF)(const void *in, size_t inlen,
void *(*kdf)(const void *in, size_t inlen,
void *out, size_t *outlen));