[openssl-commits] [openssl] master update

davidben at google.com davidben at google.com
Wed May 23 21:34:17 UTC 2018


The branch master has been updated
       via  e363534cfe0ae01503dde6963e0631ec5f7fef3f (commit)
       via  fc6f579a9e625caa0c8b93d9716ddbc558e21a29 (commit)
      from  55a6250f1e7336e8a7d89fb609eb23398715ff6f (commit)


- Log -----------------------------------------------------------------
commit e363534cfe0ae01503dde6963e0631ec5f7fef3f
Author: David Benjamin <davidben at google.com>
Date:   Sun May 20 14:37:06 2018 -0400

    Use OPENSSL_EC_EXPLICIT_CURVE constant.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6314)

commit fc6f579a9e625caa0c8b93d9716ddbc558e21a29
Author: David Benjamin <davidben at google.com>
Date:   Sun May 20 14:33:49 2018 -0400

    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 at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6314)

-----------------------------------------------------------------------

Summary of changes:
 crypto/ec/ec_asn1.c | 69 ++++++++++++++++++---------------------------
 test/ectest.c       | 81 +++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 103 insertions(+), 47 deletions(-)

diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c
index c7ed01e..33c4c23 100644
--- a/crypto/ec/ec_asn1.c
+++ b/crypto/ec/ec_asn1.c
@@ -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) {
@@ -856,7 +841,7 @@ EC_GROUP *EC_GROUP_new_from_ecpkparameters(const ECPKPARAMETERS *params)
             ECerr(EC_F_EC_GROUP_NEW_FROM_ECPKPARAMETERS, ERR_R_EC_LIB);
             return NULL;
         }
-        EC_GROUP_set_asn1_flag(ret, 0x0);
+        EC_GROUP_set_asn1_flag(ret, OPENSSL_EC_EXPLICIT_CURVE);
     } else if (params->type == 2) { /* implicitlyCA */
         return NULL;
     } else {
diff --git a/test/ectest.c b/test/ectest.c
index 1c31cce..73e8aa8 100644
--- a/test/ectest.c
+++ b/test/ectest.c
@@ -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;
+
+    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;
 
-    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);
+    /* 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


More information about the openssl-commits mailing list