Clean up EC_POINT to byte conversions.
With the allocations and BN_CTX gone, ECDH and point2oct are much, much
shorter.
Bug: 242
Change-Id: I3421822e94100f7eb2f5f2373df7fb3b3311365e
Reviewed-on: https://boringssl-review.googlesource.com/c/33071
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ecdh_extra/ecdh_extra.c b/crypto/ecdh_extra/ecdh_extra.c
index eb321f7..1e08099 100644
--- a/crypto/ecdh_extra/ecdh_extra.c
+++ b/crypto/ecdh_extra/ecdh_extra.c
@@ -69,7 +69,6 @@
#include <limits.h>
#include <string.h>
-#include <openssl/bn.h>
#include <openssl/digest.h>
#include <openssl/err.h>
#include <openssl/mem.h>
@@ -78,10 +77,10 @@
#include "../internal.h"
-int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
+int ECDH_compute_key(void *out, size_t out_len, const EC_POINT *pub_key,
const EC_KEY *priv_key,
void *(*kdf)(const void *in, size_t inlen, void *out,
- size_t *outlen)) {
+ size_t *out_len)) {
if (priv_key->priv_key == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_NO_PRIVATE_VALUE);
return -1;
@@ -93,74 +92,33 @@
return -1;
}
- BN_CTX *ctx = BN_CTX_new();
- if (ctx == NULL) {
+ EC_RAW_POINT shared_point;
+ uint8_t buf[EC_MAX_BYTES];
+ size_t buf_len;
+ if (!ec_point_mul_scalar(group, &shared_point, NULL, &pub_key->raw, priv) ||
+ !ec_point_get_affine_coordinate_bytes(group, buf, NULL, &buf_len,
+ sizeof(buf), &shared_point)) {
+ OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
return -1;
}
- BN_CTX_start(ctx);
-
- int ret = -1;
- size_t buflen = 0;
- uint8_t *buf = NULL;
-
- EC_POINT *tmp = EC_POINT_new(group);
- if (tmp == NULL) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (!ec_point_mul_scalar(group, &tmp->raw, NULL, &pub_key->raw, priv)) {
- OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
- goto err;
- }
-
- 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;
- }
-
- buflen = (EC_GROUP_get_degree(group) + 7) / 8;
- buf = OPENSSL_malloc(buflen);
- if (buf == NULL) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (!BN_bn2bin_padded(buf, buflen, x)) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_INTERNAL_ERROR);
- goto err;
- }
if (kdf != NULL) {
- if (kdf(buf, buflen, out, &outlen) == NULL) {
+ if (kdf(buf, buf_len, out, &out_len) == NULL) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_KDF_FAILED);
- goto err;
+ return -1;
}
} else {
// no KDF, just copy as much as we can
- if (buflen < outlen) {
- outlen = buflen;
+ if (buf_len < out_len) {
+ out_len = buf_len;
}
- OPENSSL_memcpy(out, buf, outlen);
+ OPENSSL_memcpy(out, buf, out_len);
}
- if (outlen > INT_MAX) {
+ if (out_len > INT_MAX) {
OPENSSL_PUT_ERROR(ECDH, ERR_R_OVERFLOW);
- goto err;
+ return -1;
}
- ret = (int)outlen;
-
-err:
- OPENSSL_free(buf);
- EC_POINT_free(tmp);
- BN_CTX_end(ctx);
- BN_CTX_free(ctx);
- return ret;
+ return (int)out_len;
}
diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c
index 717e054..bd0662a 100644
--- a/crypto/fipsmodule/ec/ec.c
+++ b/crypto/fipsmodule/ec/ec.c
@@ -973,6 +973,37 @@
return 1;
}
+int ec_point_get_affine_coordinate_bytes(const EC_GROUP *group, uint8_t *out_x,
+ uint8_t *out_y, size_t *out_len,
+ size_t max_out,
+ const EC_RAW_POINT *p) {
+ size_t len = BN_num_bytes(&group->field);
+ assert(len <= EC_MAX_BYTES);
+ if (max_out < len) {
+ OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL);
+ return 0;
+ }
+
+ EC_FELEM x, y;
+ if (!group->meth->point_get_affine_coordinates(
+ group, p, out_x == NULL ? NULL : &x, out_y == NULL ? NULL : &y)) {
+ return 0;
+ }
+
+ if (out_x != NULL) {
+ for (size_t i = 0; i < len; i++) {
+ out_x[i] = x.bytes[len - i - 1];
+ }
+ }
+ if (out_y != NULL) {
+ for (size_t i = 0; i < len; i++) {
+ out_y[i] = y.bytes[len - i - 1];
+ }
+ }
+ *out_len = len;
+ return 1;
+}
+
void EC_GROUP_set_asn1_flag(EC_GROUP *group, int flag) {}
const EC_METHOD *EC_GROUP_method_of(const EC_GROUP *group) {
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index a34ae98..d78d719 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -354,6 +354,17 @@
int ec_get_x_coordinate_as_scalar(const EC_GROUP *group, EC_SCALAR *out,
const EC_RAW_POINT *p);
+// ec_point_get_affine_coordinate_bytes writes |p|'s affine coordinates to
+// |out_x| and |out_y|, each of which must have at must |max_out| bytes. It sets
+// |*out_len| to the number of bytes written in each buffer. Coordinates are
+// written big-endian and zero-padded to the size of the field.
+//
+// Either of |out_x| or |out_y| may be NULL to omit that coordinate. This
+// function returns one on success and zero on failure.
+int ec_point_get_affine_coordinate_bytes(const EC_GROUP *group, uint8_t *out_x,
+ uint8_t *out_y, size_t *out_len,
+ size_t max_out, const EC_RAW_POINT *p);
+
// ec_field_element_to_scalar reduces |r| modulo |group->order|. |r| must
// previously have been reduced modulo |group->field|.
int ec_field_element_to_scalar(const EC_GROUP *group, BIGNUM *r);
diff --git a/crypto/fipsmodule/ec/oct.c b/crypto/fipsmodule/ec/oct.c
index 19e17a7..04b1f2c 100644
--- a/crypto/fipsmodule/ec/oct.c
+++ b/crypto/fipsmodule/ec/oct.c
@@ -74,22 +74,18 @@
static size_t ec_GFp_simple_point2oct(const EC_GROUP *group,
- const EC_POINT *point,
+ const EC_RAW_POINT *point,
point_conversion_form_t form,
- uint8_t *buf, size_t len, BN_CTX *ctx) {
- size_t ret = 0;
- BN_CTX *new_ctx = NULL;
- int used_ctx = 0;
-
- if ((form != POINT_CONVERSION_COMPRESSED) &&
- (form != POINT_CONVERSION_UNCOMPRESSED)) {
+ uint8_t *buf, size_t len) {
+ if (form != POINT_CONVERSION_COMPRESSED &&
+ form != POINT_CONVERSION_UNCOMPRESSED) {
OPENSSL_PUT_ERROR(EC, EC_R_INVALID_FORM);
- goto err;
+ return 0;
}
- if (EC_POINT_is_at_infinity(group, point)) {
+ if (ec_GFp_simple_is_at_infinity(group, point)) {
OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY);
- goto err;
+ return 0;
}
const size_t field_len = BN_num_bytes(&group->field);
@@ -103,64 +99,31 @@
if (buf != NULL) {
if (len < output_len) {
OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL);
- goto err;
+ return 0;
}
- if (ctx == NULL) {
- ctx = new_ctx = BN_CTX_new();
- if (ctx == NULL) {
- goto err;
- }
+ uint8_t y_buf[EC_MAX_BYTES];
+ size_t field_len_out;
+ if (!ec_point_get_affine_coordinate_bytes(
+ group, buf + 1 /* x */,
+ form == POINT_CONVERSION_COMPRESSED ? y_buf : buf + 1 + field_len,
+ &field_len_out, field_len, point)) {
+ return 0;
}
- BN_CTX_start(ctx);
- used_ctx = 1;
- BIGNUM *x = BN_CTX_get(ctx);
- BIGNUM *y = BN_CTX_get(ctx);
- if (y == NULL) {
- goto err;
+ if (field_len_out != field_len) {
+ OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
+ return 0;
}
- if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) {
- goto err;
- }
-
- if ((form == POINT_CONVERSION_COMPRESSED) &&
- BN_is_odd(y)) {
- buf[0] = form + 1;
+ if (form == POINT_CONVERSION_COMPRESSED) {
+ buf[0] = form + (y_buf[field_len - 1] & 1);
} else {
buf[0] = form;
}
- size_t i = 1;
-
- if (!BN_bn2bin_padded(buf + i, field_len, x)) {
- OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
- goto err;
- }
- i += field_len;
-
- if (form == POINT_CONVERSION_UNCOMPRESSED) {
- if (!BN_bn2bin_padded(buf + i, field_len, y)) {
- OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
- goto err;
- }
- i += field_len;
- }
-
- if (i != output_len) {
- OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
- goto err;
- }
}
- ret = output_len;
-
-err:
- if (used_ctx) {
- BN_CTX_end(ctx);
- }
- BN_CTX_free(new_ctx);
- return ret;
+ return output_len;
}
static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
@@ -263,7 +226,7 @@
OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
return 0;
}
- return ec_GFp_simple_point2oct(group, point, form, buf, len, ctx);
+ return ec_GFp_simple_point2oct(group, &point->raw, form, buf, len);
}
int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group,
diff --git a/crypto/fipsmodule/ecdh/ecdh.c b/crypto/fipsmodule/ecdh/ecdh.c
index 19d12c9..b9dc237 100644
--- a/crypto/fipsmodule/ecdh/ecdh.c
+++ b/crypto/fipsmodule/ecdh/ecdh.c
@@ -66,10 +66,8 @@
#include <openssl/ecdh.h>
-#include <limits.h>
#include <string.h>
-#include <openssl/bn.h>
#include <openssl/ec.h>
#include <openssl/ec_key.h>
#include <openssl/err.h>
@@ -92,50 +90,15 @@
return 0;
}
- BN_CTX *ctx = BN_CTX_new();
- if (ctx == NULL) {
+ EC_RAW_POINT shared_point;
+ uint8_t buf[EC_MAX_BYTES];
+ size_t buflen;
+ if (!ec_point_mul_scalar(group, &shared_point, NULL, &pub_key->raw, priv) ||
+ !ec_point_get_affine_coordinate_bytes(group, buf, NULL, &buflen,
+ sizeof(buf), &shared_point)) {
+ OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
return 0;
}
- BN_CTX_start(ctx);
-
- int ret = 0;
- size_t buflen = 0;
- uint8_t *buf = NULL;
-
- EC_POINT *shared_point = EC_POINT_new(group);
- if (shared_point == NULL) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (!ec_point_mul_scalar(group, &shared_point->raw, NULL, &pub_key->raw,
- priv)) {
- OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
- goto err;
- }
-
- 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, shared_point, x, NULL, ctx)) {
- OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
- goto err;
- }
-
- buflen = (EC_GROUP_get_degree(group) + 7) / 8;
- buf = OPENSSL_malloc(buflen);
- if (buf == NULL) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
- goto err;
- }
-
- if (!BN_bn2bin_padded(buf, buflen, x)) {
- OPENSSL_PUT_ERROR(ECDH, ERR_R_INTERNAL_ERROR);
- goto err;
- }
switch (out_len) {
case SHA224_DIGEST_LENGTH:
@@ -152,15 +115,8 @@
break;
default:
OPENSSL_PUT_ERROR(ECDH, ECDH_R_UNKNOWN_DIGEST_LENGTH);
- goto err;
+ return 0;
}
- ret = 1;
-
-err:
- OPENSSL_free(buf);
- EC_POINT_free(shared_point);
- BN_CTX_end(ctx);
- BN_CTX_free(ctx);
- return ret;
+ return 1;
}
