From 671ccb1a98fae26bf9c115068b2993418cecc800 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 20 Oct 2022 16:16:50 -0400 Subject: [PATCH] Make EVP_PKEY_*_tls_encodedpoint work with EVP_PKEY_EC. Some third-party code requires it. For now, I've just introduced a new hook on the method table. This is rather goofy though. First, making EVP know about TLS is a layering violation that OpenSSL introduced. They've since fixed this and added EVP_PKEY_get1_encoded_public_key in OpenSSL 3.0, but callers expect the TLS one to exist in OpenSSL 1.1.1, so implement that one. Along the way, implement EC_KEY_oct2key from upstream, which is slightly less tedious when you're already working in EC_KEY. To make this third-party code work (and to write a test without dipping out of EVP, or using the very tedious EVP_PKEY_paramgen API), we also need to change EVP_PKEY_copy_parameters to work when the source EVP_PKEY is empty, per upstream's 2986ecdc08016de978f1134315623778420b51e5. OpenSSL's API has *multiple* levels of empty states to worry about! Something to avoid when we get to rethinking this error-prone API. Bug: b:238920520 Change-Id: I3fd99be560db313c1bf549a4e46ffccc31e746e1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54905 Auto-Submit: David Benjamin Commit-Queue: David Benjamin Reviewed-by: Bob Beck --- crypto/evp/evp.c | 19 ++++ crypto/evp/evp_extra_test.cc | 66 +++++++++++++ crypto/evp/internal.h | 11 +++ crypto/evp/p_dsa_asn1.c | 43 +++++---- crypto/evp/p_ec_asn1.c | 96 ++++++++++++------- crypto/evp/p_ed25519_asn1.c | 10 +- crypto/evp/p_rsa_asn1.c | 41 ++++---- crypto/evp/p_x25519_asn1.c | 57 +++++------ crypto/fipsmodule/ec/ec_key.c | 14 +++ include/openssl/ec_key.h | 7 ++ include/openssl/evp.h | 10 +- .../acvp/modulewrapper/modulewrapper.cc | 12 +-- 12 files changed, 266 insertions(+), 120 deletions(-) diff --git a/crypto/evp/evp.c b/crypto/evp/evp.c index bb316450f..e982af79b 100644 --- a/crypto/evp/evp.c +++ b/crypto/evp/evp.c @@ -448,6 +448,25 @@ void OpenSSL_add_all_digests(void) {} void EVP_cleanup(void) {} +int EVP_PKEY_set1_tls_encodedpoint(EVP_PKEY *pkey, const uint8_t *in, + size_t len) { + if (pkey->ameth->set1_tls_encodedpoint == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE); + return 0; + } + + return pkey->ameth->set1_tls_encodedpoint(pkey, in, len); +} + +size_t EVP_PKEY_get1_tls_encodedpoint(const EVP_PKEY *pkey, uint8_t **out_ptr) { + if (pkey->ameth->get1_tls_encodedpoint == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE); + return 0; + } + + return pkey->ameth->get1_tls_encodedpoint(pkey, out_ptr); +} + int EVP_PKEY_base_id(const EVP_PKEY *pkey) { // OpenSSL has two notions of key type because it supports multiple OIDs for // the same algorithm: NID_rsa vs NID_rsaEncryption and five distinct spelling diff --git a/crypto/evp/evp_extra_test.cc b/crypto/evp/evp_extra_test.cc index f52059887..0b915bd15 100644 --- a/crypto/evp/evp_extra_test.cc +++ b/crypto/evp/evp_extra_test.cc @@ -782,3 +782,69 @@ TEST(EVPExtraTest, Ed25519Keygen) { ASSERT_TRUE(EVP_DigestVerify(ctx.get(), sig, len, reinterpret_cast("hello"), 5)); } + +// Test that OpenSSL's legacy TLS-specific APIs in EVP work correctly. When we +// target OpenSSL 3.0, these should be renamed to +// |EVP_PKEY_get1_encoded_public_key|. +TEST(EVPExtraTest, TLSEncodedPoint) { + const struct { + int pkey_type; + std::vector spki; + std::vector encoded_point; + } kTests[] = { + {EVP_PKEY_EC, + {0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03, + 0x42, 0x00, 0x04, 0x2c, 0x15, 0x0f, 0x42, 0x9c, 0xe7, 0x0f, 0x21, 0x6c, + 0x25, 0x2c, 0xf5, 0xe0, 0x62, 0xce, 0x1f, 0x63, 0x9c, 0xd5, 0xd1, 0x65, + 0xc7, 0xf8, 0x94, 0x24, 0x07, 0x2c, 0x27, 0x19, 0x7d, 0x78, 0xb3, 0x3b, + 0x92, 0x0e, 0x95, 0xcd, 0xb6, 0x64, 0xe9, 0x90, 0xdc, 0xf0, 0xcf, 0xea, + 0x0d, 0x94, 0xe2, 0xa8, 0xe6, 0xaf, 0x9d, 0x0e, 0x58, 0x05, 0x6e, 0x65, + 0x31, 0x04, 0x92, 0x5b, 0x9f, 0xe6, 0xc9}, + {0x04, 0x2c, 0x15, 0x0f, 0x42, 0x9c, 0xe7, 0x0f, 0x21, 0x6c, 0x25, + 0x2c, 0xf5, 0xe0, 0x62, 0xce, 0x1f, 0x63, 0x9c, 0xd5, 0xd1, 0x65, + 0xc7, 0xf8, 0x94, 0x24, 0x07, 0x2c, 0x27, 0x19, 0x7d, 0x78, 0xb3, + 0x3b, 0x92, 0x0e, 0x95, 0xcd, 0xb6, 0x64, 0xe9, 0x90, 0xdc, 0xf0, + 0xcf, 0xea, 0x0d, 0x94, 0xe2, 0xa8, 0xe6, 0xaf, 0x9d, 0x0e, 0x58, + 0x05, 0x6e, 0x65, 0x31, 0x04, 0x92, 0x5b, 0x9f, 0xe6, 0xc9}}, + {EVP_PKEY_X25519, + {0x30, 0x2a, 0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x6e, 0x03, 0x21, + 0x00, 0xe6, 0xdb, 0x68, 0x67, 0x58, 0x30, 0x30, 0xdb, 0x35, 0x94, + 0xc1, 0xa4, 0x24, 0xb1, 0x5f, 0x7c, 0x72, 0x66, 0x24, 0xec, 0x26, + 0xb3, 0x35, 0x3b, 0x10, 0xa9, 0x03, 0xa6, 0xd0, 0xab, 0x1c, 0x4c}, + {0xe6, 0xdb, 0x68, 0x67, 0x58, 0x30, 0x30, 0xdb, 0x35, 0x94, 0xc1, + 0xa4, 0x24, 0xb1, 0x5f, 0x7c, 0x72, 0x66, 0x24, 0xec, 0x26, 0xb3, + 0x35, 0x3b, 0x10, 0xa9, 0x03, 0xa6, 0xd0, 0xab, 0x1c, 0x4c}}}; + for (const auto& test : kTests) { + SCOPED_TRACE(test.pkey_type); + SCOPED_TRACE(Bytes(test.spki)); + CBS spki; + CBS_init(&spki, test.spki.data(), test.spki.size()); + bssl::UniquePtr from_spki(EVP_parse_public_key(&spki)); + ASSERT_TRUE(from_spki); + + uint8_t *data; + size_t len = EVP_PKEY_get1_tls_encodedpoint(from_spki.get(), &data); + ASSERT_GT(len, 0u); + EXPECT_EQ(Bytes(data, len), Bytes(test.encoded_point)); + OPENSSL_free(data); + + bssl::UniquePtr from_encoded_point(EVP_PKEY_new()); + ASSERT_TRUE(from_encoded_point); + ASSERT_TRUE(EVP_PKEY_set_type(from_encoded_point.get(), test.pkey_type)); + if (test.pkey_type == EVP_PKEY_EC) { + // |EVP_PKEY_EC| should have been |EVP_PKEY_EC_P256|, etc., but instead + // part of the type is buried inside parameters. + ASSERT_TRUE( + EVP_PKEY_copy_parameters(from_encoded_point.get(), from_spki.get())); + } + ASSERT_TRUE(EVP_PKEY_set1_tls_encodedpoint(from_encoded_point.get(), + test.encoded_point.data(), + test.encoded_point.size())); + + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), test.spki.size())); + ASSERT_TRUE(EVP_marshal_public_key(cbb.get(), from_encoded_point.get())); + EXPECT_EQ(Bytes(CBB_data(cbb.get()), CBB_len(cbb.get())), Bytes(test.spki)); + } +} diff --git a/crypto/evp/internal.h b/crypto/evp/internal.h index 0037de825..f189d4e69 100644 --- a/crypto/evp/internal.h +++ b/crypto/evp/internal.h @@ -103,6 +103,17 @@ struct evp_pkey_asn1_method_st { int (*get_priv_raw)(const EVP_PKEY *pkey, uint8_t *out, size_t *out_len); int (*get_pub_raw)(const EVP_PKEY *pkey, uint8_t *out, size_t *out_len); + // TODO(davidben): Can these be merged with the functions above? OpenSSL does + // not implement |EVP_PKEY_get_raw_public_key|, etc., for |EVP_PKEY_EC|, but + // the distinction seems unimportant. OpenSSL 3.0 has since renamed + // |EVP_PKEY_get1_tls_encodedpoint| to |EVP_PKEY_get1_encoded_public_key|, and + // what is the difference between "raw" and an "encoded" public key. + // + // One nuisance is the notion of "raw" is slightly ambiguous for EC keys. Is + // it a DER ECPrivateKey or just the scalar? + int (*set1_tls_encodedpoint)(EVP_PKEY *pkey, const uint8_t *in, size_t len); + size_t (*get1_tls_encodedpoint)(const EVP_PKEY *pkey, uint8_t **out_ptr); + // pkey_opaque returns 1 if the |pk| is opaque. Opaque keys are backed by // custom implementations which do not expose key material and parameters. int (*pkey_opaque)(const EVP_PKEY *pk); diff --git a/crypto/evp/p_dsa_asn1.c b/crypto/evp/p_dsa_asn1.c index 98615b738..cd787dd54 100644 --- a/crypto/evp/p_dsa_asn1.c +++ b/crypto/evp/p_dsa_asn1.c @@ -248,34 +248,37 @@ static int dsa_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) { static void int_dsa_free(EVP_PKEY *pkey) { DSA_free(pkey->pkey.dsa); } const EVP_PKEY_ASN1_METHOD dsa_asn1_meth = { - EVP_PKEY_DSA, - // 1.2.840.10040.4.1 - {0x2a, 0x86, 0x48, 0xce, 0x38, 0x04, 0x01}, 7, + EVP_PKEY_DSA, + // 1.2.840.10040.4.1 + {0x2a, 0x86, 0x48, 0xce, 0x38, 0x04, 0x01}, + 7, - NULL /* pkey_method */, + /*pkey_method=*/NULL, - dsa_pub_decode, - dsa_pub_encode, - dsa_pub_cmp, + dsa_pub_decode, + dsa_pub_encode, + dsa_pub_cmp, - dsa_priv_decode, - dsa_priv_encode, + dsa_priv_decode, + dsa_priv_encode, - NULL /* set_priv_raw */, - NULL /* set_pub_raw */, - NULL /* get_priv_raw */, - NULL /* get_pub_raw */, + /*set_priv_raw=*/NULL, + /*set_pub_raw=*/NULL, + /*get_priv_raw=*/NULL, + /*get_pub_raw=*/NULL, + /*set1_tls_encodedpoint=*/NULL, + /*get1_tls_encodedpoint=*/NULL, - NULL /* pkey_opaque */, + /*pkey_opaque=*/NULL, - int_dsa_size, - dsa_bits, + int_dsa_size, + dsa_bits, - dsa_missing_parameters, - dsa_copy_parameters, - dsa_cmp_parameters, + dsa_missing_parameters, + dsa_copy_parameters, + dsa_cmp_parameters, - int_dsa_free, + int_dsa_free, }; int EVP_PKEY_CTX_set_dsa_paramgen_bits(EVP_PKEY_CTX *ctx, int nbits) { diff --git a/crypto/evp/p_ec_asn1.c b/crypto/evp/p_ec_asn1.c index dd421217c..c21cfd8cb 100644 --- a/crypto/evp/p_ec_asn1.c +++ b/crypto/evp/p_ec_asn1.c @@ -93,7 +93,6 @@ static int eckey_pub_decode(EVP_PKEY *out, CBS *params, CBS *key) { // See RFC 5480, section 2. // The parameters are a named curve. - EC_POINT *point = NULL; EC_KEY *eckey = NULL; EC_GROUP *group = EC_KEY_parse_curve_name(params); if (group == NULL || CBS_len(params) != 0) { @@ -102,25 +101,18 @@ static int eckey_pub_decode(EVP_PKEY *out, CBS *params, CBS *key) { } eckey = EC_KEY_new(); - if (eckey == NULL || !EC_KEY_set_group(eckey, group)) { - goto err; - } - - point = EC_POINT_new(group); - if (point == NULL || - !EC_POINT_oct2point(group, point, CBS_data(key), CBS_len(key), NULL) || - !EC_KEY_set_public_key(eckey, point)) { + if (eckey == NULL || // + !EC_KEY_set_group(eckey, group) || + !EC_KEY_oct2key(eckey, CBS_data(key), CBS_len(key), NULL)) { goto err; } EC_GROUP_free(group); - EC_POINT_free(point); EVP_PKEY_assign_EC_KEY(out, eckey); return 1; err: EC_GROUP_free(group); - EC_POINT_free(point); EC_KEY_free(eckey); return 0; } @@ -188,6 +180,28 @@ static int eckey_priv_encode(CBB *out, const EVP_PKEY *key) { return 1; } +static int eckey_set1_tls_encodedpoint(EVP_PKEY *pkey, const uint8_t *in, + size_t len) { + EC_KEY *ec_key = pkey->pkey.ec; + if (ec_key == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET); + return 0; + } + + return EC_KEY_oct2key(ec_key, in, len, NULL); +} + +static size_t eckey_get1_tls_encodedpoint(const EVP_PKEY *pkey, + uint8_t **out_ptr) { + const EC_KEY *ec_key = pkey->pkey.ec; + if (ec_key == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET); + return 0; + } + + return EC_KEY_key2buf(ec_key, POINT_CONVERSION_UNCOMPRESSED, out_ptr, NULL); +} + static int int_ec_size(const EVP_PKEY *pkey) { return ECDSA_size(pkey->pkey.ec); } @@ -206,7 +220,22 @@ static int ec_missing_parameters(const EVP_PKEY *pkey) { } static int ec_copy_parameters(EVP_PKEY *to, const EVP_PKEY *from) { - return EC_KEY_set_group(to->pkey.ec, EC_KEY_get0_group(from->pkey.ec)); + if (from->pkey.ec == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET); + return 0; + } + const EC_GROUP *group = EC_KEY_get0_group(from->pkey.ec); + if (group == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_MISSING_PARAMETERS); + return 0; + } + if (to->pkey.ec == NULL) { + to->pkey.ec = EC_KEY_new(); + if (to->pkey.ec == NULL) { + return 0; + } + } + return EC_KEY_set_group(to->pkey.ec, group); } static int ec_cmp_parameters(const EVP_PKEY *a, const EVP_PKEY *b) { @@ -226,32 +255,35 @@ static int eckey_opaque(const EVP_PKEY *pkey) { } const EVP_PKEY_ASN1_METHOD ec_asn1_meth = { - EVP_PKEY_EC, - // 1.2.840.10045.2.1 - {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01}, 7, + EVP_PKEY_EC, + // 1.2.840.10045.2.1 + {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01}, + 7, - &ec_pkey_meth, + &ec_pkey_meth, - eckey_pub_decode, - eckey_pub_encode, - eckey_pub_cmp, + eckey_pub_decode, + eckey_pub_encode, + eckey_pub_cmp, - eckey_priv_decode, - eckey_priv_encode, + eckey_priv_decode, + eckey_priv_encode, - NULL /* set_priv_raw */, - NULL /* set_pub_raw */, - NULL /* get_priv_raw */, - NULL /* get_pub_raw */, + /*set_priv_raw=*/NULL, + /*set_pub_raw=*/NULL, + /*get_priv_raw=*/NULL, + /*get_pub_raw=*/NULL, + eckey_set1_tls_encodedpoint, + eckey_get1_tls_encodedpoint, - eckey_opaque, + eckey_opaque, - int_ec_size, - ec_bits, + int_ec_size, + ec_bits, - ec_missing_parameters, - ec_copy_parameters, - ec_cmp_parameters, + ec_missing_parameters, + ec_copy_parameters, + ec_cmp_parameters, - int_ec_free, + int_ec_free, }; diff --git a/crypto/evp/p_ed25519_asn1.c b/crypto/evp/p_ed25519_asn1.c index f823c0dbb..9f9251c95 100644 --- a/crypto/evp/p_ed25519_asn1.c +++ b/crypto/evp/p_ed25519_asn1.c @@ -214,11 +214,13 @@ const EVP_PKEY_ASN1_METHOD ed25519_asn1_meth = { ed25519_set_pub_raw, ed25519_get_priv_raw, ed25519_get_pub_raw, - NULL /* pkey_opaque */, + /*set1_tls_encodedpoint=*/NULL, + /*get1_tls_encodedpoint=*/NULL, + /*pkey_opaque=*/NULL, ed25519_size, ed25519_bits, - NULL /* param_missing */, - NULL /* param_copy */, - NULL /* param_cmp */, + /*param_missing=*/NULL, + /*param_copy=*/NULL, + /*param_cmp=*/NULL, ed25519_free, }; diff --git a/crypto/evp/p_rsa_asn1.c b/crypto/evp/p_rsa_asn1.c index 2e4942a12..cfaf69472 100644 --- a/crypto/evp/p_rsa_asn1.c +++ b/crypto/evp/p_rsa_asn1.c @@ -167,30 +167,35 @@ static int rsa_bits(const EVP_PKEY *pkey) { static void int_rsa_free(EVP_PKEY *pkey) { RSA_free(pkey->pkey.rsa); } const EVP_PKEY_ASN1_METHOD rsa_asn1_meth = { - EVP_PKEY_RSA, - // 1.2.840.113549.1.1.1 - {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01}, 9, + EVP_PKEY_RSA, + // 1.2.840.113549.1.1.1 + {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01}, + 9, - &rsa_pkey_meth, + &rsa_pkey_meth, - rsa_pub_decode, - rsa_pub_encode, - rsa_pub_cmp, + rsa_pub_decode, + rsa_pub_encode, + rsa_pub_cmp, - rsa_priv_decode, - rsa_priv_encode, + rsa_priv_decode, + rsa_priv_encode, - NULL /* set_priv_raw */, - NULL /* set_pub_raw */, - NULL /* get_priv_raw */, - NULL /* get_pub_raw */, + /*set_priv_raw=*/NULL, + /*set_pub_raw=*/NULL, + /*get_priv_raw=*/NULL, + /*get_pub_raw=*/NULL, + /*set1_tls_encodedpoint=*/NULL, + /*get1_tls_encodedpoint=*/NULL, - rsa_opaque, + rsa_opaque, - int_rsa_size, - rsa_bits, + int_rsa_size, + rsa_bits, - 0,0,0, + 0, + 0, + 0, - int_rsa_free, + int_rsa_free, }; diff --git a/crypto/evp/p_x25519_asn1.c b/crypto/evp/p_x25519_asn1.c index 182f6a2d9..e36b41e36 100644 --- a/crypto/evp/p_x25519_asn1.c +++ b/crypto/evp/p_x25519_asn1.c @@ -110,6 +110,23 @@ static int x25519_get_pub_raw(const EVP_PKEY *pkey, uint8_t *out, return 1; } +static int x25519_set1_tls_encodedpoint(EVP_PKEY *pkey, const uint8_t *in, + size_t len) { + return x25519_set_pub_raw(pkey, in, len); +} + +static size_t x25519_get1_tls_encodedpoint(const EVP_PKEY *pkey, + uint8_t **out_ptr) { + const X25519_KEY *key = pkey->pkey.ptr; + if (key == NULL) { + OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET); + return 0; + } + + *out_ptr = OPENSSL_memdup(key->pub, 32); + return *out_ptr == NULL ? 0 : 32; +} + static int x25519_pub_decode(EVP_PKEY *out, CBS *params, CBS *key) { // See RFC 8410, section 4. @@ -209,41 +226,13 @@ const EVP_PKEY_ASN1_METHOD x25519_asn1_meth = { x25519_set_pub_raw, x25519_get_priv_raw, x25519_get_pub_raw, - NULL /* pkey_opaque */, + x25519_set1_tls_encodedpoint, + x25519_get1_tls_encodedpoint, + /*pkey_opaque=*/NULL, x25519_size, x25519_bits, - NULL /* param_missing */, - NULL /* param_copy */, - NULL /* param_cmp */, + /*param_missing=*/NULL, + /*param_copy=*/NULL, + /*param_cmp=*/NULL, x25519_free, }; - -int EVP_PKEY_set1_tls_encodedpoint(EVP_PKEY *pkey, const uint8_t *in, - size_t len) { - // TODO(davidben): In OpenSSL, this function also works for |EVP_PKEY_EC| - // keys. Add support if it ever comes up. - if (pkey->type != EVP_PKEY_X25519) { - OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_PUBLIC_KEY_TYPE); - return 0; - } - - return x25519_set_pub_raw(pkey, in, len); -} - -size_t EVP_PKEY_get1_tls_encodedpoint(const EVP_PKEY *pkey, uint8_t **out_ptr) { - // TODO(davidben): In OpenSSL, this function also works for |EVP_PKEY_EC| - // keys. Add support if it ever comes up. - if (pkey->type != EVP_PKEY_X25519) { - OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_PUBLIC_KEY_TYPE); - return 0; - } - - const X25519_KEY *key = pkey->pkey.ptr; - if (key == NULL) { - OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET); - return 0; - } - - *out_ptr = OPENSSL_memdup(key->pub, 32); - return *out_ptr == NULL ? 0 : 32; -} diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c index e676e3c05..3b3da24b6 100644 --- a/crypto/fipsmodule/ec/ec_key.c +++ b/crypto/fipsmodule/ec/ec_key.c @@ -394,6 +394,20 @@ err: return ok; } +int EC_KEY_oct2key(EC_KEY *key, const uint8_t *in, size_t len, BN_CTX *ctx) { + if (key->group == NULL) { + OPENSSL_PUT_ERROR(EC, EC_R_MISSING_PARAMETERS); + return 0; + } + + EC_POINT *point = EC_POINT_new(key->group); + int ok = point != NULL && + EC_POINT_oct2point(key->group, point, in, len, ctx) && + EC_KEY_set_public_key(key, point); + EC_POINT_free(point); + return ok; +} + size_t EC_KEY_key2buf(const EC_KEY *key, point_conversion_form_t form, unsigned char **out_buf, BN_CTX *ctx) { if (key == NULL || key->pub_key == NULL || key->group == NULL) { diff --git a/include/openssl/ec_key.h b/include/openssl/ec_key.h index 502bfc2d9..3143290d0 100644 --- a/include/openssl/ec_key.h +++ b/include/openssl/ec_key.h @@ -179,6 +179,13 @@ OPENSSL_EXPORT int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key, const BIGNUM *x, const BIGNUM *y); +// EC_KEY_oct2key decodes |len| bytes from |in| as an EC public key in X9.62 +// form. |key| must already have a group configured. On success, it sets the +// public key in |key| to the result and returns one. Otherwise, it returns +// zero. +OPENSSL_EXPORT int EC_KEY_oct2key(EC_KEY *key, const uint8_t *in, size_t len, + BN_CTX *ctx); + // EC_KEY_key2buf encodes the public key in |key| to an allocated octet string // and sets |*out_buf| to point to it. It returns the length of the encoded // octet string or zero if an error occurred. diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 37daa9ade..d8bd01123 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -936,7 +936,10 @@ OPENSSL_EXPORT int EVP_PKEY_CTX_set_ec_param_enc(EVP_PKEY_CTX *ctx, // EVP_PKEY_set1_tls_encodedpoint replaces |pkey| with a public key encoded by // |in|. It returns one on success and zero on error. // -// This function only works on X25519 keys. +// If |pkey| is an EC key, the format is an X9.62 point and |pkey| must already +// have an EC group configured. If it is an X25519 key, it is the 32-byte X25519 +// public key representation. This function is not supported for other key types +// and will fail. OPENSSL_EXPORT int EVP_PKEY_set1_tls_encodedpoint(EVP_PKEY *pkey, const uint8_t *in, size_t len); @@ -946,7 +949,10 @@ OPENSSL_EXPORT int EVP_PKEY_set1_tls_encodedpoint(EVP_PKEY *pkey, // |OPENSSL_free| to release this buffer. The function returns the length of the // buffer on success and zero on error. // -// This function only works on X25519 keys. +// If |pkey| is an EC key, the format is an X9.62 point with uncompressed +// coordinates. If it is an X25519 key, it is the 32-byte X25519 public key +// representation. This function is not supported for other key types and will +// fail. OPENSSL_EXPORT size_t EVP_PKEY_get1_tls_encodedpoint(const EVP_PKEY *pkey, uint8_t **out_ptr); diff --git a/util/fipstools/acvp/modulewrapper/modulewrapper.cc b/util/fipstools/acvp/modulewrapper/modulewrapper.cc index f65492c00..460a8e9ff 100644 --- a/util/fipstools/acvp/modulewrapper/modulewrapper.cc +++ b/util/fipstools/acvp/modulewrapper/modulewrapper.cc @@ -1558,12 +1558,8 @@ static bool ECDSAKeyVer(const Span args[], ReplyCallback write_re bssl::UniquePtr x(BytesToBIGNUM(args[1])); bssl::UniquePtr y(BytesToBIGNUM(args[2])); - bssl::UniquePtr point(EC_POINT_new(EC_KEY_get0_group(key.get()))); uint8_t reply[1]; - if (!EC_POINT_set_affine_coordinates_GFp(EC_KEY_get0_group(key.get()), - point.get(), x.get(), y.get(), - /*ctx=*/nullptr) || - !EC_KEY_set_public_key(key.get(), point.get()) || + if (!EC_KEY_set_public_key_affine_coordinates(key.get(), x.get(), y.get()) || !EC_KEY_check_fips(key.get())) { reply[0] = 0; } else { @@ -1636,12 +1632,8 @@ static bool ECDSASigVer(const Span args[], ReplyCallback write_re return false; } - bssl::UniquePtr point(EC_POINT_new(EC_KEY_get0_group(key.get()))); uint8_t reply[1]; - if (!EC_POINT_set_affine_coordinates_GFp(EC_KEY_get0_group(key.get()), - point.get(), x.get(), y.get(), - /*ctx=*/nullptr) || - !EC_KEY_set_public_key(key.get(), point.get()) || + if (!EC_KEY_set_public_key_affine_coordinates(key.get(), x.get(), y.get()) || !EC_KEY_check_fips(key.get()) || !ECDSA_do_verify(digest, digest_len, &sig, key.get())) { reply[0] = 0;