Fix explicit EC curve encoding.

Per SEC 1, the curve coefficients must be padded up to size. See C.2's
definition of Curve, C.1's definition of FieldElement, and 2.3.5's definition
of how to encode the field elements in http://www.secg.org/sec1-v2.pdf.

This comes up for P-521, where b needs a leading zero.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6314)
This commit is contained in:
David Benjamin 2018-05-20 14:33:49 -04:00
parent 55a6250f1e
commit fc6f579a9e
2 changed files with 102 additions and 46 deletions

View File

@ -367,10 +367,8 @@ static int ec_asn1_group2curve(const EC_GROUP *group, X9_62_CURVE *curve)
{
int ok = 0, nid;
BIGNUM *tmp_1 = NULL, *tmp_2 = NULL;
unsigned char *buffer_1 = NULL, *buffer_2 = NULL,
*a_buf = NULL, *b_buf = NULL;
size_t len_1, len_2;
unsigned char char_zero = 0;
unsigned char *a_buf = NULL, *b_buf = NULL;
size_t len;
if (!group || !curve || !curve->a || !curve->b)
return 0;
@ -398,44 +396,26 @@ static int ec_asn1_group2curve(const EC_GROUP *group, X9_62_CURVE *curve)
}
}
#endif
len_1 = (size_t)BN_num_bytes(tmp_1);
len_2 = (size_t)BN_num_bytes(tmp_2);
if (len_1 == 0) {
/* len_1 == 0 => a == 0 */
a_buf = &char_zero;
len_1 = 1;
} else {
if ((buffer_1 = OPENSSL_malloc(len_1)) == NULL) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_MALLOC_FAILURE);
goto err;
}
if ((len_1 = BN_bn2bin(tmp_1, buffer_1)) == 0) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_BN_LIB);
goto err;
}
a_buf = buffer_1;
/*
* Per SEC 1, the curve coefficients must be padded up to size. See C.2's
* definition of Curve, C.1's definition of FieldElement, and 2.3.5's
* definition of how to encode the field elements.
*/
len = ((size_t)EC_GROUP_get_degree(group) + 7) / 8;
if ((a_buf = OPENSSL_malloc(len)) == NULL
|| (b_buf = OPENSSL_malloc(len)) == NULL) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_MALLOC_FAILURE);
goto err;
}
if (len_2 == 0) {
/* len_2 == 0 => b == 0 */
b_buf = &char_zero;
len_2 = 1;
} else {
if ((buffer_2 = OPENSSL_malloc(len_2)) == NULL) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_MALLOC_FAILURE);
goto err;
}
if ((len_2 = BN_bn2bin(tmp_2, buffer_2)) == 0) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_BN_LIB);
goto err;
}
b_buf = buffer_2;
if (BN_bn2binpad(tmp_1, a_buf, len) < 0
|| BN_bn2binpad(tmp_2, b_buf, len) < 0) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_BN_LIB);
goto err;
}
/* set a and b */
if (!ASN1_OCTET_STRING_set(curve->a, a_buf, len_1) ||
!ASN1_OCTET_STRING_set(curve->b, b_buf, len_2)) {
if (!ASN1_OCTET_STRING_set(curve->a, a_buf, len)
|| !ASN1_OCTET_STRING_set(curve->b, b_buf, len)) {
ECerr(EC_F_EC_ASN1_GROUP2CURVE, ERR_R_ASN1_LIB);
goto err;
}
@ -462,8 +442,8 @@ static int ec_asn1_group2curve(const EC_GROUP *group, X9_62_CURVE *curve)
ok = 1;
err:
OPENSSL_free(buffer_1);
OPENSSL_free(buffer_2);
OPENSSL_free(a_buf);
OPENSSL_free(b_buf);
BN_free(tmp_1);
BN_free(tmp_2);
return ok;
@ -611,7 +591,12 @@ EC_GROUP *EC_GROUP_new_from_ecparameters(const ECPARAMETERS *params)
goto err;
}
/* now extract the curve parameters a and b */
/*
* Now extract the curve parameters a and b. Note that, although SEC 1
* specifies the length of their encodings, historical versions of OpenSSL
* encoded them incorrectly, so we must accept any length for backwards
* compatibility.
*/
if (!params->curve || !params->curve->a ||
!params->curve->a->data || !params->curve->b ||
!params->curve->b->data) {

View File

@ -1407,20 +1407,91 @@ err:
}
# endif
static const unsigned char p521_named[] = {
0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x23,
};
static const unsigned char p521_explicit[] = {
0x30, 0x82, 0x01, 0xc3, 0x02, 0x01, 0x01, 0x30, 0x4d, 0x06, 0x07, 0x2a,
0x86, 0x48, 0xce, 0x3d, 0x01, 0x01, 0x02, 0x42, 0x01, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0x30, 0x81, 0x9f, 0x04, 0x42, 0x01, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xfc, 0x04, 0x42, 0x00, 0x51, 0x95, 0x3e, 0xb9, 0x61, 0x8e, 0x1c, 0x9a,
0x1f, 0x92, 0x9a, 0x21, 0xa0, 0xb6, 0x85, 0x40, 0xee, 0xa2, 0xda, 0x72,
0x5b, 0x99, 0xb3, 0x15, 0xf3, 0xb8, 0xb4, 0x89, 0x91, 0x8e, 0xf1, 0x09,
0xe1, 0x56, 0x19, 0x39, 0x51, 0xec, 0x7e, 0x93, 0x7b, 0x16, 0x52, 0xc0,
0xbd, 0x3b, 0xb1, 0xbf, 0x07, 0x35, 0x73, 0xdf, 0x88, 0x3d, 0x2c, 0x34,
0xf1, 0xef, 0x45, 0x1f, 0xd4, 0x6b, 0x50, 0x3f, 0x00, 0x03, 0x15, 0x00,
0xd0, 0x9e, 0x88, 0x00, 0x29, 0x1c, 0xb8, 0x53, 0x96, 0xcc, 0x67, 0x17,
0x39, 0x32, 0x84, 0xaa, 0xa0, 0xda, 0x64, 0xba, 0x04, 0x81, 0x85, 0x04,
0x00, 0xc6, 0x85, 0x8e, 0x06, 0xb7, 0x04, 0x04, 0xe9, 0xcd, 0x9e, 0x3e,
0xcb, 0x66, 0x23, 0x95, 0xb4, 0x42, 0x9c, 0x64, 0x81, 0x39, 0x05, 0x3f,
0xb5, 0x21, 0xf8, 0x28, 0xaf, 0x60, 0x6b, 0x4d, 0x3d, 0xba, 0xa1, 0x4b,
0x5e, 0x77, 0xef, 0xe7, 0x59, 0x28, 0xfe, 0x1d, 0xc1, 0x27, 0xa2, 0xff,
0xa8, 0xde, 0x33, 0x48, 0xb3, 0xc1, 0x85, 0x6a, 0x42, 0x9b, 0xf9, 0x7e,
0x7e, 0x31, 0xc2, 0xe5, 0xbd, 0x66, 0x01, 0x18, 0x39, 0x29, 0x6a, 0x78,
0x9a, 0x3b, 0xc0, 0x04, 0x5c, 0x8a, 0x5f, 0xb4, 0x2c, 0x7d, 0x1b, 0xd9,
0x98, 0xf5, 0x44, 0x49, 0x57, 0x9b, 0x44, 0x68, 0x17, 0xaf, 0xbd, 0x17,
0x27, 0x3e, 0x66, 0x2c, 0x97, 0xee, 0x72, 0x99, 0x5e, 0xf4, 0x26, 0x40,
0xc5, 0x50, 0xb9, 0x01, 0x3f, 0xad, 0x07, 0x61, 0x35, 0x3c, 0x70, 0x86,
0xa2, 0x72, 0xc2, 0x40, 0x88, 0xbe, 0x94, 0x76, 0x9f, 0xd1, 0x66, 0x50,
0x02, 0x42, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfa,
0x51, 0x86, 0x87, 0x83, 0xbf, 0x2f, 0x96, 0x6b, 0x7f, 0xcc, 0x01, 0x48,
0xf7, 0x09, 0xa5, 0xd0, 0x3b, 0xb5, 0xc9, 0xb8, 0x89, 0x9c, 0x47, 0xae,
0xbb, 0x6f, 0xb7, 0x1e, 0x91, 0x38, 0x64, 0x09, 0x02, 0x01, 0x01,
};
static int parameter_test(void)
{
EC_GROUP *group = NULL, *group2 = NULL;
ECPARAMETERS *ecparameters = NULL;
int r;
unsigned char *buf = NULL;
int r = 0, len;
r = TEST_ptr(group = EC_GROUP_new_by_curve_name(NID_secp112r1))
&& TEST_ptr(ecparameters = EC_GROUP_get_ecparameters(group, NULL))
&& TEST_ptr(group2 = EC_GROUP_new_from_ecparameters(ecparameters))
&& TEST_int_eq(EC_GROUP_cmp(group, group2, NULL), 0);
if (!TEST_ptr(group = EC_GROUP_new_by_curve_name(NID_secp112r1))
|| !TEST_ptr(ecparameters = EC_GROUP_get_ecparameters(group, NULL))
|| !TEST_ptr(group2 = EC_GROUP_new_from_ecparameters(ecparameters))
|| !TEST_int_eq(EC_GROUP_cmp(group, group2, NULL), 0))
goto err;
EC_GROUP_free(group);
group = NULL;
/* Test the named curve encoding, which should be default. */
if (!TEST_ptr(group = EC_GROUP_new_by_curve_name(NID_secp521r1))
|| !TEST_true((len = i2d_ECPKParameters(group, &buf)) >= 0)
|| !TEST_mem_eq(buf, len, p521_named, sizeof(p521_named)))
goto err;
OPENSSL_free(buf);
buf = NULL;
/*
* Test the explicit encoding. P-521 requires correctly zero-padding the
* curve coefficients.
*/
EC_GROUP_set_asn1_flag(group, OPENSSL_EC_EXPLICIT_CURVE);
if (!TEST_true((len = i2d_ECPKParameters(group, &buf)) >= 0)
|| !TEST_mem_eq(buf, len, p521_explicit, sizeof(p521_explicit)))
goto err;
r = 1;
err:
EC_GROUP_free(group);
EC_GROUP_free(group2);
ECPARAMETERS_free(ecparameters);
OPENSSL_free(buf);
return r;
}
#endif