]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Honor OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT as set and default to UNCOMPRESSED
authorNicola Tuveri <nic.tuv@gmail.com>
Sat, 18 Sep 2021 15:17:39 +0000 (18:17 +0300)
committerTomas Mraz <tomas@openssl.org>
Tue, 29 Nov 2022 15:03:04 +0000 (16:03 +0100)
Originally the code to im/export the EC pubkey was meant to be consumed
only by the im/export functions when crossing the provider boundary.
Having our providers exporting to a COMPRESSED format octet string made
sense to avoid memory waste, as it wasn't exposed outside the provider
API, and providers had all tools available to convert across the three
formats.

Later on, with #13139 deprecating the `EC_KEY_*` functions, more state
was added among the params imported/exported on an EC provider-native
key (including `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT`, although it
did not affect the format used to export `OSSL_PKEY_PARAM_PUB_KEY`).

Finally, in #14800, `EVP_PKEY_todata()` was introduced and prominently
exposed directly to users outside the provider API, and the choice of
COMPRESSED over UNCOMPRESSED as the default became less sensible in
light of usability, given the latter is more often needed by
applications and protocols.

This commit fixes it, by using `EC_KEY_get_conv_form()` to get the
point format from the internal state (an `EC_KEY` under the hood) of the
provider-side object, and using it on
`EVP_PKEY_export()`/`EVP_PKEY_todata()` to format
`OSSL_PKEY_PARAM_PUB_KEY`.
The default for an `EC_KEY` was already UNCOMPRESSED, and it is altered
if the user sets `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT` via
`EVP_PKEY_fromdata()`, `EVP_PKEY_set_params()`, or one of the
more specialized methods.

For symmetry, this commit also alters `ec_pkey_export_to()` in
`crypto/ec/ec_ameth.c`, part of the `EVP_PKEY_ASN1_METHOD` for legacy EC
keys: it exclusively used COMPRESSED format, and now it honors the
conversion format specified in the EC_KEY object being exported to a
provider when this function is called.

Expand documentation about `OSSL_PKEY_PARAM_PUB_KEY` and mention the
3.1 change in behavior for our providers.

Fixes #16595

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19681)

(cherry picked from commit 926db476bc669fdcc4c4d2f1cb547060bdbfa153)

CHANGES.md
crypto/ec/ec_ameth.c
doc/man7/EVP_PKEY-EC.pod
providers/implementations/keymgmt/ec_kmgmt.c
test/evp_pkey_provided_test.c

index 1e91bcda2d48f8c5ee95b7204288556d9499da66..bb0170f5cae111410b50453a3b97d8055b3ee3e8 100644 (file)
@@ -92,6 +92,20 @@ OpenSSL 3.2
 
    *Darshan Sen*
 
+ * Our provider implementations of `OSSL_FUNC_KEYMGMT_EXPORT` and
+   `OSSL_FUNC_KEYMGMT_GET_PARAMS` for EC and SM2 keys now honor
+   `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT` as set (and
+   default to `POINT_CONVERSION_UNCOMPRESSED`) when exporting
+   `OSSL_PKEY_PARAM_PUB_KEY`, instead of unconditionally using
+   `POINT_CONVERSION_COMPRESSED` as in previous 3.x releases.
+   For symmetry, our implementation of `EVP_PKEY_ASN1_METHOD->export_to`
+   for legacy EC and SM2 keys is also changed similarly to honor the
+   equivalent conversion format flag as specified in the underlying
+   `EC_KEY` object being exported to a provider, when this function is
+   called through `EVP_PKEY_export()`.
+
+   *Nicola Tuveri*
+
  * RNDR and RNDRRS support in provider functions to provide
    random number generation for Arm CPUs (aarch64).
 
index 290838b18c710f3b3937dbba2529ed2192df8139..ad28ba6827d4ec853bcc1e98489ef849de8f8cd2 100644 (file)
@@ -513,8 +513,10 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata,
 
     if (pub_point != NULL) {
         /* convert pub_point to a octet string according to the SECG standard */
+        point_conversion_form_t format = EC_KEY_get_conv_form(eckey);
+
         if ((pub_key_buflen = EC_POINT_point2buf(ecg, pub_point,
-                                                 POINT_CONVERSION_COMPRESSED,
+                                                 format,
                                                  &pub_key_buf, bnctx)) == 0
             || !OSSL_PARAM_BLD_push_octet_string(tmpl,
                                                  OSSL_PKEY_PARAM_PUB_KEY,
index 0d32c67b438712e38e17f382938fec5876b322e0..143ec4e82f9539c30a6f45093786ca43c3392ff6 100644 (file)
@@ -79,6 +79,10 @@ Enable Cofactor DH (ECC CDH) if this value is 1, otherwise it uses normal EC DH
 if the value is zero. The cofactor variant multiplies the shared secret by the
 EC curve's cofactor (note for some curves the cofactor is 1).
 
+See also L<EVP_KEYEXCH-ECDH(7)> for the related
+B<OSSL_EXCHANGE_PARAM_EC_ECDH_COFACTOR_MODE> parameter that can be set on a
+per-operation basis.
+
 =item "encoding" (B<OSSL_PKEY_PARAM_EC_ENCODING>) <UTF8 string>
 
 Set the format used for serializing the EC group parameters.
@@ -104,15 +108,21 @@ but is equivalent to "named-nist" for the OpenSSL FIPS provider.
 Setting this value to 0 indicates that the public key should not be included when
 encoding the private key. The default value of 1 will include the public key.
 
-See also L<EVP_KEYEXCH-ECDH(7)> for the related
-B<OSSL_EXCHANGE_PARAM_EC_ECDH_COFACTOR_MODE> parameter that can be set on a
-per-operation basis.
-
 =item "pub" (B<OSSL_PKEY_PARAM_PUB_KEY>) <octet string>
 
-The public key value in encoded EC point format. This parameter is used
-when importing or exporting the public key value with the EVP_PKEY_fromdata()
-and EVP_PKEY_todata() functions.
+The public key value in encoded EC point format conforming to Sec. 2.3.3 and
+2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard.
+This parameter is used when importing or exporting the public key value with the
+EVP_PKEY_fromdata() and EVP_PKEY_todata() functions.
+
+Note, in particular, that the choice of point compression format used for
+encoding the exported value via EVP_PKEY_todata() depends on the underlying
+provider implementation.
+Before OpenSSL 3.1, the implementation of providers included with OpenSSL always
+opted for an encoding in compressed format, unconditionally.
+Since OpenSSL 3.1, the implementation has been changed to honor the
+B<OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT> parameter, if set, or to default
+to uncompressed format.
 
 =item "priv" (B<OSSL_PKEY_PARAM_PRIV_KEY>) <unsigned integer>
 
index fe2465a64b0f3f371ae56f40f05aeb282d64f2e6..b23fab5fde2847b8a106c25d866d2f42ebe16390 100644 (file)
@@ -147,8 +147,10 @@ int key_to_params(const EC_KEY *eckey, OSSL_PARAM_BLD *tmpl,
 
         if (p != NULL || tmpl != NULL) {
             /* convert pub_point to a octet string according to the SECG standard */
+            point_conversion_form_t format = EC_KEY_get_conv_form(eckey);
+
             if ((pub_key_len = EC_POINT_point2buf(ecg, pub_point,
-                                                  POINT_CONVERSION_COMPRESSED,
+                                                  format,
                                                   pub_key, bnctx)) == 0
                 || !ossl_param_build_set_octet_string(tmpl, p,
                                                       OSSL_PKEY_PARAM_PUB_KEY,
index 3237884d7c9381e92d54a61bf7810bcaafb68d8c..3f490954abaf8d44a4dadf994c820b0560981a73 100644 (file)
@@ -1185,13 +1185,20 @@ static int test_fromdata_ec(void)
        0x7f, 0x59, 0x5f, 0x8c, 0xd1, 0x96, 0x0b, 0xdf,
        0x29, 0x3e, 0x49, 0x07, 0x88, 0x3f, 0x9a, 0x29
     };
+    /* SAME BUT COMPRESSED FORMAT */
+    static const unsigned char ec_pub_keydata_compressed[] = {
+       POINT_CONVERSION_COMPRESSED+1,
+       0x1b, 0x93, 0x67, 0x55, 0x1c, 0x55, 0x9f, 0x63,
+       0xd1, 0x22, 0xa4, 0xd8, 0xd1, 0x0a, 0x60, 0x6d,
+       0x02, 0xa5, 0x77, 0x57, 0xc8, 0xa3, 0x47, 0x73,
+       0x3a, 0x6a, 0x08, 0x28, 0x39, 0xbd, 0xc9, 0xd2
+    };
     static const unsigned char ec_priv_keydata[] = {
         0x33, 0xd0, 0x43, 0x83, 0xa9, 0x89, 0x56, 0x03,
         0xd2, 0xd7, 0xfe, 0x6b, 0x01, 0x6f, 0xe4, 0x59,
         0xcc, 0x0d, 0x9a, 0x24, 0x6c, 0x86, 0x1b, 0x2e,
         0xdc, 0x4b, 0x4d, 0x35, 0x43, 0xe1, 0x1b, 0xad
     };
-    const int compressed_sz = 1 + (sizeof(ec_pub_keydata) - 1) / 2;
     unsigned char out_pub[sizeof(ec_pub_keydata)];
     char out_curve_name[80];
     const OSSL_PARAM *gettable = NULL;
@@ -1214,9 +1221,17 @@ static int test_fromdata_ec(void)
     if (OSSL_PARAM_BLD_push_utf8_string(bld, OSSL_PKEY_PARAM_GROUP_NAME,
                                         curve, 0) <= 0)
         goto err;
+    /*
+     * We intentionally provide the input point in compressed format,
+     * and avoid setting `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT`.
+     *
+     * Later on we check what format is used when exporting the
+     * `OSSL_PKEY_PARAM_PUB_KEY` and expect to default to uncompressed
+     * format.
+     */
     if (OSSL_PARAM_BLD_push_octet_string(bld, OSSL_PKEY_PARAM_PUB_KEY,
-                                         ec_pub_keydata,
-                                         sizeof(ec_pub_keydata)) <= 0)
+                                         ec_pub_keydata_compressed,
+                                         sizeof(ec_pub_keydata_compressed)) <= 0)
         goto err;
     if (OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY, ec_priv_bn) <= 0)
         goto err;
@@ -1287,9 +1302,17 @@ static int test_fromdata_ec(void)
             || !TEST_str_eq(out_curve_name, curve)
             || !EVP_PKEY_get_octet_string_param(pk, OSSL_PKEY_PARAM_PUB_KEY,
                                             out_pub, sizeof(out_pub), &len)
-            || !TEST_true(out_pub[0] == (POINT_CONVERSION_COMPRESSED + 1))
+
+            /*
+             * Our providers use uncompressed format by default if
+             * `OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT` was not
+             * explicitly set, irrespective of the format used for the
+             * input point given as a param to create this key.
+             */
+            || !TEST_true(out_pub[0] == POINT_CONVERSION_UNCOMPRESSED)
             || !TEST_mem_eq(out_pub + 1, len - 1,
-                            ec_pub_keydata + 1, compressed_sz - 1)
+                            ec_pub_keydata + 1, sizeof(ec_pub_keydata) - 1)
+
             || !TEST_true(EVP_PKEY_get_bn_param(pk, OSSL_PKEY_PARAM_PRIV_KEY,
                                                 &bn_priv))
             || !TEST_BN_eq(ec_priv_bn, bn_priv))