Merge pull request #1302 from samuel40791765/upstream-merge-2023-11-10

Upstream merge 2023-11-10

crypto/ec_extra/ec_derive.c

@@ -55,7 +55,8 @@ EC_KEY *EC_KEY_derive_from_secret(const EC_GROUP *group, const uint8_t *secret,
   }
 
   uint8_t derived[EC_KEY_DERIVE_EXTRA_BYTES + EC_MAX_BYTES];
-  size_t derived_len = BN_num_bytes(&group->order) + EC_KEY_DERIVE_EXTRA_BYTES;
+  size_t derived_len =
+      BN_num_bytes(EC_GROUP_get0_order(group)) + EC_KEY_DERIVE_EXTRA_BYTES;
   assert(derived_len <= sizeof(derived));
   if (!HKDF(derived, derived_len, EVP_sha256(), secret, secret_len,
             /*salt=*/NULL, /*salt_len=*/0, (const uint8_t *)info,
@@ -74,10 +75,10 @@ EC_KEY *EC_KEY_derive_from_secret(const EC_GROUP *group, const uint8_t *secret,
       // enough. 2^(num_bytes(order)) < 2^8 * order, so:
       //
       //    priv < 2^8 * order * 2^128 < order * order < order * R
-      !BN_from_montgomery(priv, priv, group->order_mont, ctx) ||
+      !BN_from_montgomery(priv, priv, group->order, ctx) ||
       // Multiply by R^2 and do another Montgomery reduction to compute
       // priv * R^-1 * R^2 * R^-1 = priv mod order.
-      !BN_to_montgomery(priv, priv, group->order_mont, ctx) ||
+      !BN_to_montgomery(priv, priv, group->order, ctx) ||
       !EC_POINT_mul(group, pub, priv, NULL, NULL, ctx) ||
       !EC_KEY_set_group(key, group) || !EC_KEY_set_public_key(key, pub) ||
       !EC_KEY_set_private_key(key, priv)) {

crypto/ec_extra/hash_to_curve.c

@@ -185,15 +185,16 @@ static int hash_to_field2(const EC_GROUP *group, const EVP_MD *md,
 static int hash_to_scalar(const EC_GROUP *group, const EVP_MD *md,
                           EC_SCALAR *out, const uint8_t *dst, size_t dst_len,
                           unsigned k, const uint8_t *msg, size_t msg_len) {
+  const BIGNUM *order = EC_GROUP_get0_order(group);
   size_t L;
   uint8_t buf[EC_MAX_BYTES * 2];
-  if (!num_bytes_to_derive(&L, &group->order, k) ||
+  if (!num_bytes_to_derive(&L, order, k) ||
       !expand_message_xmd(md, buf, L, msg, msg_len, dst, dst_len)) {
     return 0;
   }
 
   BN_ULONG words[2 * EC_MAX_WORDS];
-  size_t num_words = 2 * group->order.width;
+  size_t num_words = 2 * order->width;
   bn_big_endian_to_words(words, num_words, buf, L);
   ec_scalar_reduce(group, out, words, num_words);
   return 1;

crypto/evp_extra/p_ec_asn1.c

@@ -204,7 +204,7 @@ static int ec_bits(const EVP_PKEY *pkey) {
     ERR_clear_error();
     return 0;
   }
-  return BN_num_bits(EC_GROUP_get0_order(group));
+  return EC_GROUP_order_bits(group);
 }
 
 static int ec_missing_parameters(const EVP_PKEY *pkey) {

crypto/fipsmodule/ec/ec.c

@@ -338,7 +338,6 @@ EC_GROUP *ec_group_new(const EC_METHOD *meth) {
 
   ret->references = 1;
   ret->meth = meth;
-  BN_init(&ret->order);
 
   if (!meth->group_init(ret)) {
     OPENSSL_free(ret);
@@ -352,30 +351,13 @@ static int ec_group_set_generator(EC_GROUP *group, const EC_AFFINE *generator,
                                   const BIGNUM *order) {
   assert(group->generator == NULL);
 
-  if (!BN_copy(&group->order, order)) {
-    return 0;
-  }
-  // Store the order in minimal form, so it can be used with |BN_ULONG| arrays.
-  bn_set_minimal_width(&group->order);
-
-  BN_MONT_CTX_free(group->order_mont);
-  group->order_mont = BN_MONT_CTX_new_for_modulus(&group->order, NULL);
-  if (group->order_mont == NULL) {
+  BN_MONT_CTX_free(group->order);
+  group->order = BN_MONT_CTX_new_for_modulus(order, NULL);
+  if (group->order == NULL) {
     return 0;
   }
 
   group->field_greater_than_order = BN_cmp(&group->field, order) > 0;
-  if (group->field_greater_than_order) {
-    BIGNUM tmp;
-    BN_init(&tmp);
-    int ok =
-        BN_sub(&tmp, &group->field, order) &&
-        bn_copy_words(group->field_minus_order.words, group->field.width, &tmp);
-    BN_free(&tmp);
-    if (!ok) {
-      return 0;
-    }
-  }
 
   group->generator = EC_POINT_new(group);
   if (group->generator == NULL) {
@@ -609,8 +591,7 @@ void EC_GROUP_free(EC_GROUP *group) {
   }
 
   ec_point_free(group->generator, 0 /* don't free group */);
-  BN_free(&group->order);
-  BN_MONT_CTX_free(group->order_mont);
+  BN_MONT_CTX_free(group->order);
 
   OPENSSL_free(group);
 }
@@ -649,7 +630,7 @@ int EC_GROUP_cmp(const EC_GROUP *a, const EC_GROUP *b, BN_CTX *ignored) {
   return a->meth != b->meth ||
          a->generator == NULL ||
          b->generator == NULL ||
-         BN_cmp(&a->order, &b->order) != 0 ||
+         BN_cmp(&a->order->N, &b->order->N) != 0 ||
          BN_cmp(&a->field, &b->field) != 0 ||
          !ec_felem_equal(a, &a->a, &b->a) ||
          !ec_felem_equal(a, &a->b, &b->b) ||
@@ -661,8 +642,8 @@ const EC_POINT *EC_GROUP_get0_generator(const EC_GROUP *group) {
 }
 
 const BIGNUM *EC_GROUP_get0_order(const EC_GROUP *group) {
-  assert(!BN_is_zero(&group->order));
-  return &group->order;
+  assert(group->order != NULL);
+  return &group->order->N;
 }
 
 int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order, BN_CTX *ctx) {
@@ -673,7 +654,7 @@ int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order, BN_CTX *ctx) {
 }
 
 int EC_GROUP_order_bits(const EC_GROUP *group) {
-  return BN_num_bits(&group->order);
+  return BN_num_bits(&group->order->N);
 }
 
 int EC_GROUP_get_cofactor(const EC_GROUP *group, BIGNUM *cofactor,
@@ -979,11 +960,10 @@ static int arbitrary_bignum_to_scalar(const EC_GROUP *group, EC_SCALAR *out,
   ERR_clear_error();
 
   // This is an unusual input, so we do not guarantee constant-time processing.
-  const BIGNUM *order = &group->order;
   BN_CTX_start(ctx);
   BIGNUM *tmp = BN_CTX_get(ctx);
   int ok = tmp != NULL &&
-           BN_nnmod(tmp, in, order, ctx) &&
+           BN_nnmod(tmp, in, EC_GROUP_get0_order(group), ctx) &&
            ec_bignum_to_scalar(group, out, tmp);
   BN_CTX_end(ctx);
   return ok;

crypto/fipsmodule/ec/ec_key.c

@@ -93,8 +93,8 @@ static EC_WRAPPED_SCALAR *ec_wrapped_scalar_new(const EC_GROUP *group) {
 
   OPENSSL_memset(wrapped, 0, sizeof(EC_WRAPPED_SCALAR));
   wrapped->bignum.d = wrapped->scalar.words;
-  wrapped->bignum.width = group->order.width;
-  wrapped->bignum.dmax = group->order.width;
+  wrapped->bignum.width = group->order->N.width;
+  wrapped->bignum.dmax = group->order->N.width;
   wrapped->bignum.flags = BN_FLG_STATIC_DATA;
   return wrapped;
 }
@@ -486,7 +486,7 @@ int EC_KEY_generate_key(EC_KEY *key) {
   }
 
   // Check that the group order is FIPS compliant (FIPS 186-4 B.4.2).
-  if (BN_num_bits(EC_GROUP_get0_order(key->group)) < 160) {
+  if (EC_GROUP_order_bits(key->group) < 160) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
     return 0;
   }

crypto/fipsmodule/ec/ec_montgomery.c

@@ -457,7 +457,7 @@ static int ec_GFp_mont_cmp_x_coordinate(const EC_GROUP *group,
                                         const EC_JACOBIAN *p,
                                         const EC_SCALAR *r) {
   if (!group->field_greater_than_order ||
-      group->field.width != group->order.width) {
+      group->field.width != group->order->N.width) {
     // Do not bother optimizing this case. p > order in all commonly-used
     // curves.
     return ec_GFp_simple_cmp_x_coordinate(group, p, r);
@@ -485,10 +485,11 @@ static int ec_GFp_mont_cmp_x_coordinate(const EC_GROUP *group,
   // Therefore there is a small possibility, less than 1/2^128, that group_order
   // < p.x < P. in that case we need not only to compare against |r| but also to
   // compare against r+group_order.
-  if (bn_less_than_words(r->words, group->field_minus_order.words,
-                         group->field.width)) {
-    // We can ignore the carry because: r + group_order < p < 2^256.
-    bn_add_words(r_Z2.words, r->words, group->order.d, group->field.width);
+  BN_ULONG carry =
+      bn_add_words(r_Z2.words, r->words, group->order->N.d, group->field.width);
+  if (carry == 0 &&
+      bn_less_than_words(r_Z2.words, group->field.d, group->field.width)) {
+    // r + group_order < p, so compare (r + group_order) * Z^2 against X.
     ec_GFp_mont_felem_mul(group, &r_Z2, &r_Z2, &Z2_mont);
     if (ec_felem_equal(group, &r_Z2, &X)) {
       return 1;

crypto/fipsmodule/ec/ec_test.cc

@@ -1267,8 +1267,8 @@ TEST(ECTest, SmallGroupOrder) {
   bssl::UniquePtr<EC_KEY> key2(EC_KEY_new());
   ASSERT_TRUE(key2);
   ASSERT_TRUE(EC_KEY_set_group(key2.get(), group.get()));
-  BN_clear(&key2.get()->group->order);
-  ASSERT_TRUE(BN_set_word(&key2.get()->group->order, 7));
+  BN_clear(&key2.get()->group->order->N);
+  ASSERT_TRUE(BN_set_word(&key2.get()->group->order->N, 7));
   ASSERT_FALSE(EC_KEY_generate_key_fips(key2.get()));
 }
 
@@ -1325,8 +1325,8 @@ TEST(ECDeathTest, SmallGroupOrderAndDie) {
   bssl::UniquePtr<EC_KEY> key2(EC_KEY_new());
   ASSERT_TRUE(key2);
   ASSERT_TRUE(EC_KEY_set_group(key2.get(), group.get()));
-  BN_clear(&key2.get()->group->order);
-  ASSERT_TRUE(BN_set_word(&key2.get()->group->order, 7));
+  BN_clear(&key2.get()->group->order->N);
+  ASSERT_TRUE(BN_set_word(&key2.get()->group->order->N, 7));
   ASSERT_DEATH_IF_SUPPORTED(EC_KEY_generate_key_fips(key2.get()), "");
 }
 

crypto/fipsmodule/ec/internal.h

@@ -441,7 +441,7 @@ void ec_precomp_select(const EC_GROUP *group, EC_PRECOMP *out, BN_ULONG mask,
 
 // ec_cmp_x_coordinate compares the x (affine) coordinate of |p|, mod the group
 // order, with |r|. It returns one if the values match and zero if |p| is the
-// point at infinity of the values do not match.
+// point at infinity of the values do not match. |p| is treated as public.
 int ec_cmp_x_coordinate(const EC_GROUP *group, const EC_JACOBIAN *p,
                         const EC_SCALAR *r);
 
@@ -619,11 +619,10 @@ struct ec_group_st {
   // to avoid a reference cycle. Additionally, Z is guaranteed to be one, so X
   // and Y are suitable for use as an |EC_AFFINE|.
   EC_POINT *generator;
-  BIGNUM order;
 
   int curve_name;  // optional NID for named curve
 
-  BN_MONT_CTX *order_mont;  // data for ECDSA inverse
+  BN_MONT_CTX *order;
 
   // The following members are handled by the method functions,
   // even if they appear generic
@@ -640,13 +639,6 @@ struct ec_group_st {
   // otherwise.
   int field_greater_than_order;
 
-  // field_minus_order, if |field_greater_than_order| is true, is |field| minus
-  // |order| represented as an |EC_FELEM|. Otherwise, it is zero.
-  //
-  // Note: unlike |EC_FELEM|s used as intermediate values internal to the
-  // |EC_METHOD|, this value is not encoded in Montgomery form.
-  EC_FELEM field_minus_order;
-
   CRYPTO_refcount_t references;
 
   BN_MONT_CTX *mont;  // Montgomery structure.

crypto/fipsmodule/ec/p256-nistz.c

@@ -583,8 +583,8 @@ static int ecp_nistz256_scalar_to_montgomery_inv_vartime(const EC_GROUP *group,
   }
 #endif
 
-  assert(group->order.width == P256_LIMBS);
-  if (!beeu_mod_inverse_vartime(out->words, in->words, group->order.d)) {
+  assert(group->order->N.width == P256_LIMBS);
+  if (!beeu_mod_inverse_vartime(out->words, in->words, group->order->N.d)) {
     return 0;
   }
 
@@ -602,7 +602,7 @@ static int ecp_nistz256_cmp_x_coordinate(const EC_GROUP *group,
     return 0;
   }
 
-  assert(group->order.width == P256_LIMBS);
+  assert(group->order->N.width == P256_LIMBS);
   assert(group->field.width == P256_LIMBS);
 
   // We wish to compare X/Z^2 with r. This is equivalent to comparing X with
@@ -621,10 +621,9 @@ static int ecp_nistz256_cmp_x_coordinate(const EC_GROUP *group,
   // Therefore there is a small possibility, less than 1/2^128, that group_order
   // < p.x < P. in that case we need not only to compare against |r| but also to
   // compare against r+group_order.
-  if (bn_less_than_words(r->words, group->field_minus_order.words,
-                         P256_LIMBS)) {
-    // We can ignore the carry because: r + group_order < p < 2^256.
-    bn_add_words(r_Z2, r->words, group->order.d, P256_LIMBS);
+  BN_ULONG carry = bn_add_words(r_Z2, r->words, group->order->N.d, P256_LIMBS);
+  if (carry == 0 && bn_less_than_words(r_Z2, group->field.d, P256_LIMBS)) {
+    // r + group_order < p, so compare (r + group_order) * Z^2 against X.
     ecp_nistz256_mul_mont(r_Z2, r_Z2, Z2_mont);
     if (OPENSSL_memcmp(r_Z2, X, sizeof(r_Z2)) == 0) {
       return 1;

crypto/fipsmodule/ec/p256.c

@@ -710,12 +710,12 @@ static int ec_GFp_nistp256_cmp_x_coordinate(const EC_GROUP *group,
   // Therefore there is a small possibility, less than 1/2^128, that group_order
   // < p.x < P. in that case we need not only to compare against |r| but also to
   // compare against r+group_order.
-  assert(group->field.width == group->order.width);
-  if (bn_less_than_words(r->words, group->field_minus_order.words,
-                         group->field.width)) {
-    // We can ignore the carry because: r + group_order < p < 2^256.
-    EC_FELEM tmp;
-    bn_add_words(tmp.words, r->words, group->order.d, group->order.width);
+  assert(group->field.width == group->order->N.width);
+  EC_FELEM tmp;
+  BN_ULONG carry =
+      bn_add_words(tmp.words, r->words, group->order->N.d, group->field.width);
+  if (carry == 0 &&
+      bn_less_than_words(tmp.words, group->field.d, group->field.width)) {
     fiat_p256_from_generic(r_Z2, &tmp);
     fiat_p256_mul(r_Z2, r_Z2, Z2_mont);
     if (OPENSSL_memcmp(&r_Z2, &X, sizeof(r_Z2)) == 0) {

crypto/fipsmodule/ec/p384.c

@@ -568,7 +568,7 @@ static void ec_GFp_nistp384_mont_felem_to_bytes(
   p384_felem_from_mont(tmp, tmp);
   p384_to_generic(&felem_tmp, tmp);
 
-  bn_words_to_big_endian(out, len, felem_tmp.words, group->order.width);
+  bn_words_to_big_endian(out, len, felem_tmp.words, group->order->N.width);
 
   *out_len = len;
 }
@@ -619,12 +619,12 @@ static int ec_GFp_nistp384_cmp_x_coordinate(const EC_GROUP *group,
   // that group_order < p.x < p.
   // In that case, we need not only to compare against |r| but also to
   // compare against r+group_order.
-  assert(group->field.width == group->order.width);
-  if (bn_less_than_words(r->words, group->field_minus_order.words,
-                         group->field.width)) {
-    // We can ignore the carry because: r + group_order < p < 2^384.
-    EC_FELEM tmp;
-    bn_add_words(tmp.words, r->words, group->order.d, group->order.width);
+  assert(group->field.width == group->order->N.width);
+  EC_FELEM tmp;
+  BN_ULONG carry =
+      bn_add_words(tmp.words, r->words, group->order->N.d, group->field.width);
+  if (carry == 0 &&
+      bn_less_than_words(tmp.words, group->field.d, group->field.width)) {
     p384_from_generic(r_Z2, &tmp);
     p384_felem_mul(r_Z2, r_Z2, Z2_mont);
     if (OPENSSL_memcmp(&r_Z2, &X, sizeof(r_Z2)) == 0) {