Use BN_bn2bin_padded rather than doing math to figure out leading zeros. Saves doing it ad-hoc all the time. Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8 Reviewed-on: https://boringssl-review.googlesource.com/2241 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec/oct.c b/crypto/ec/oct.c index 1c5ea14..487cc4e 100644 --- a/crypto/ec/oct.c +++ b/crypto/ec/oct.c
@@ -81,7 +81,7 @@ BN_CTX *new_ctx = NULL; int used_ctx = 0; BIGNUM *x, *y; - size_t field_len, i, skip; + size_t field_len, i; if ((form != POINT_CONVERSION_COMPRESSED) && (form != POINT_CONVERSION_UNCOMPRESSED) && @@ -117,58 +117,45 @@ if (ctx == NULL) { ctx = new_ctx = BN_CTX_new(); - if (ctx == NULL) + if (ctx == NULL) { return 0; + } } BN_CTX_start(ctx); used_ctx = 1; x = BN_CTX_get(ctx); y = BN_CTX_get(ctx); - if (y == NULL) + if (y == NULL) { goto err; + } - if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) + if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) { goto err; + } if ((form == POINT_CONVERSION_COMPRESSED || form == POINT_CONVERSION_HYBRID) && - BN_is_odd(y)) + BN_is_odd(y)) { buf[0] = form + 1; - else + } else { buf[0] = form; - + } i = 1; - skip = field_len - BN_num_bytes(x); - if (skip > field_len) { + if (!BN_bn2bin_padded(buf + i, field_len, x)) { OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR); goto err; } - while (skip > 0) { - buf[i++] = 0; - skip--; - } - skip = BN_bn2bin(x, buf + i); - i += skip; - if (i != 1 + field_len) { - OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR); - goto err; - } + i += field_len; if (form == POINT_CONVERSION_UNCOMPRESSED || form == POINT_CONVERSION_HYBRID) { - skip = field_len - BN_num_bytes(y); - if (skip > field_len) { + if (!BN_bn2bin_padded(buf + i, field_len, y)) { OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR); goto err; } - while (skip > 0) { - buf[i++] = 0; - skip--; - } - skip = BN_bn2bin(y, buf + i); - i += skip; + i += field_len; } if (i != ret) { @@ -177,17 +164,21 @@ } } - if (used_ctx) + if (used_ctx) { BN_CTX_end(ctx); - if (new_ctx != NULL) + } + if (new_ctx != NULL) { BN_CTX_free(new_ctx); + } return ret; err: - if (used_ctx) + if (used_ctx) { BN_CTX_end(ctx); - if (new_ctx != NULL) + } + if (new_ctx != NULL) { BN_CTX_free(new_ctx); + } return 0; }
diff --git a/crypto/ecdh/ecdh.c b/crypto/ecdh/ecdh.c index c64a0ad..0d05653 100644 --- a/crypto/ecdh/ecdh.c +++ b/crypto/ecdh/ecdh.c
@@ -81,7 +81,7 @@ const BIGNUM *priv; const EC_GROUP *group; int ret = -1; - size_t buflen, len; + size_t buflen; uint8_t *buf = NULL; if ((ctx = BN_CTX_new()) == NULL) { @@ -116,20 +116,14 @@ } buflen = (EC_GROUP_get_degree(group) + 7) / 8; - len = BN_num_bytes(x); - if (len > buflen) { - OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_INTERNAL_ERROR); - goto err; - } buf = OPENSSL_malloc(buflen); if (buf == NULL) { OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_MALLOC_FAILURE); goto err; } - memset(buf, 0, buflen - len); - if (len != (size_t)BN_bn2bin(x, buf + buflen - len)) { - OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_BN_LIB); + if (!BN_bn2bin_padded(buf, buflen, x)) { + OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_INTERNAL_ERROR); goto err; }
diff --git a/crypto/ecdsa/ecdsa_test.c b/crypto/ecdsa/ecdsa_test.c index 523cbdf..127d76f 100644 --- a/crypto/ecdsa/ecdsa_test.c +++ b/crypto/ecdsa/ecdsa_test.c
@@ -217,14 +217,15 @@ goto builtin_err; } buf_len = 2 * bn_len; - raw_buf = OPENSSL_malloc(buf_len); + raw_buf = OPENSSL_malloc(2 * bn_len); if (raw_buf == NULL) { goto builtin_err; } /* Pad the bignums with leading zeroes. */ - memset(raw_buf, 0, buf_len); - BN_bn2bin(ecdsa_sig->r, raw_buf + bn_len - r_len); - BN_bn2bin(ecdsa_sig->s, raw_buf + buf_len - s_len); + if (!BN_bn2bin_padded(raw_buf, bn_len, ecdsa_sig->r) || + !BN_bn2bin_padded(raw_buf + bn_len, bn_len, ecdsa_sig->s)) { + goto builtin_err; + } /* Modify a single byte in the buffer. */ offset = raw_buf[10] % buf_len;