[Openvpn-devel] Implement ED448 and ED25519 support in xkey_provider

OpenSSL's implementation of ED448 and ED25519 has a few idiosyncrasies.
Instead of belonging to the eliptic curve type or to a common Edwards
curve type, ED448 and ED25519 have each their own type.

Also, OpenSSL excepts singatures using these curves to be done with the
EVP_DigestSign API instead of the EVP_Sign API but using md=NULL.

This has been tested using a "fake" external key that used a normal
software key instead of a hardware implementation but that makes no
difference from the perspective of xkey_provider/managment interface.
---
 src/openvpn/xkey_helper.c                |   4 +
 src/openvpn/xkey_provider.c              | 121 +++++++++++++++++++++--
 tests/unit_tests/openvpn/test_provider.c |  21 ++--
 3 files changed, 134 insertions(+), 12 deletions(-)

Hi,

On Tue, May 10, 2022 at 7:32 AM Arne Schwabe <arne@rfc2549.org> wrote:

> OpenSSL's implementation of ED448 and ED25519 has a few idiosyncrasies.
> Instead of belonging to the eliptic curve type or to a common Edwards
> curve type, ED448 and ED25519 have each their own type.
>
> Also, OpenSSL excepts singatures using these curves to be done with the
>

expects
signatures

(good to know I'm not the only one afflicted by transposition errors)


> EVP_DigestSign API instead of the EVP_Sign API but using md=NULL.

This has been tested using a "fake" external key that used a normal
> software key instead of a hardware implementation but that makes no
> difference from the perspective of xkey_provider/managment interface.
>

management


> ---
>  src/openvpn/xkey_helper.c                |   4 +
>  src/openvpn/xkey_provider.c              | 121 +++++++++++++++++++++--
>  tests/unit_tests/openvpn/test_provider.c |  21 ++--
>  3 files changed, 134 insertions(+), 12 deletions(-)
>
> diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
> index ecc7b1204..d59b16858 100644
> --- a/src/openvpn/xkey_helper.c
> +++ b/src/openvpn/xkey_helper.c
> @@ -206,6 +206,10 @@ xkey_management_sign(void *unused, unsigned char
> *sig, size_t *siglen,
>              openvpn_snprintf(alg_str, sizeof(alg_str),
> "ECDSA,hashalg=%s", alg.mdname);
>          }
>      }
> +    else if(!strcmp(alg.keytype, "ED448") || !strcmp(alg.keytype,
> "ED25519"))
> +    {
> +        strncpynt(alg_str, alg.keytype, sizeof(alg_str));
> +    }
>      /* else assume RSA key */
>      else if (!strcmp(alg.padmode, "pkcs1") && (flags &
> MF_EXTERNAL_KEY_PKCS1PAD))
>      {
> diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c
> index 46e57e0fe..e8ac3904b 100644
> --- a/src/openvpn/xkey_provider.c
> +++ b/src/openvpn/xkey_provider.c
> @@ -99,12 +99,28 @@ typedef struct
>      int refcount;                /**< reference count */
>  } XKEY_KEYDATA;
>
> -static int
> -KEYTYPE(const XKEY_KEYDATA *key)
> +static inline const char *
> +get_keytype(const XKEY_KEYDATA *key)
>  {
> -    return key->pubkey ? EVP_PKEY_get_id(key->pubkey) : 0;
> +    int keytype = key->pubkey ? EVP_PKEY_get_id(key->pubkey) : 0;
> +
> +    switch (keytype)
> +    {
> +        case EVP_PKEY_RSA:
> +            return "RSA";
> +
> +        case EVP_PKEY_ED448:
> +            return "ED448";
> +
> +        case EVP_PKEY_ED25519:
> +            return "ED25519";
> +
> +        default:
> +            return "EC";

+    }
>  }
>
> +
>  static int
>  KEYSIZE(const XKEY_KEYDATA *key)
>  {
> @@ -310,6 +326,22 @@ ec_keymgmt_import(void *keydata, int selection, const
> OSSL_PARAM params[])
>      return keymgmt_import(keydata, selection, params, "EC");
>  }
>
> +static int
> +ed448_keymgmt_import(void *keydata, int selection, const OSSL_PARAM
> params[])
> +{
> +    xkey_dmsg(D_XKEY, "entry");
> +
> +    return keymgmt_import(keydata, selection, params, "ED448");
> +}
> +
> +static int
> +ed25519_keymgmt_import(void *keydata, int selection, const OSSL_PARAM
> params[])
> +{
> +    xkey_dmsg(D_XKEY, "entry");
> +
> +    return keymgmt_import(keydata, selection, params, "ED25519");
> +}
> +
>  /* This function has to exist for key import to work
>   * though we do not support import of individual params
>   * like n or e. We simply return an empty list here for
> @@ -449,7 +481,7 @@ keymgmt_import_helper(XKEY_KEYDATA *key, const
> OSSL_PARAM *params)
>          ASSERT(pkey);
>
>          int id = EVP_PKEY_get_id(pkey);
> -        if (id != EVP_PKEY_RSA && id != EVP_PKEY_EC)
> +        if (id != EVP_PKEY_RSA && id != EVP_PKEY_EC && id !=
> EVP_PKEY_ED25519 && id != EVP_PKEY_ED448)
>          {
>              msg(M_WARN, "Error: xkey keymgmt_import: unknown key type
> (%d)", id);
>              return 0;
> @@ -556,6 +588,40 @@ ec_keymgmt_name(int id)
>      return "EC";
>  }
>
> +static const char *
> +ed448_keymgmt_name(int id)
> +{
> +    xkey_dmsg(D_XKEY, "entry");
> +
> +    /* though we do not implement keyexch we could be queried for
> +     * keyexch mechanism supported by EC keys
> +     */
> +    if (id == OSSL_OP_SIGNATURE || id == OSSL_OP_KEYEXCH)
> +    {
> +        return "ED448";

+    }
> +
> +    msg(D_XKEY, "xkey ed448_keymgmt_name called with op_id != SIGNATURE
> or KEYEXCH id=%d", id);
> +    return "EC";
>

I think this should be either "ED448" or NULL. The default provider does
not define the query_operation function for this key type in the dispatch
table --- likely because the name will then default to the keymgmt name
(ED448). Also, I just realized that in the default provider, the
query_operation_name (when defined) returns NULL for ids not recognized.
That leads to triggering an error message in the library which is good.

We could either return NULL here or "ED448" and then convert the above
debug msg() to a warning. Or leave out this function altogether and remove
it from the dispatch table.

+}
> +
> +static const char *
> +ed25519_keymgmt_name(int id)
> +{
> +    xkey_dmsg(D_XKEY, "entry");
> +
> +    /* though we do not implement keyexch we could be queried for
> +     * keyexch mechanism supported by EC keys
> +     */
> +    if (id == OSSL_OP_SIGNATURE || id == OSSL_OP_KEYEXCH)
> +    {
> +        return "ED25519";
> +    }

+
> +    msg(D_XKEY, "xkey ed448_keymgmt_name called with op_id != SIGNATURE
> or KEYEXCH id=%d", id);
> +    return "EC";
>

same comment as above with ED448 --> ED25519.


> +}
> +
>  static const OSSL_DISPATCH rsa_keymgmt_functions[] = {
>      {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},
>      {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},
> @@ -588,10 +654,45 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = {
>      {0, NULL }
>  };
>
> +static const OSSL_DISPATCH ed448_keymgmt_functions[] = {
> +    {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},
> +    {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},
> +    {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void))keymgmt_load},
> +    {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void))keymgmt_has},
> +    {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void))keymgmt_match},
> +    {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void))ed448_keymgmt_import},
> +    {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void
> (*)(void))keymgmt_import_types},
> +    {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void
> (*)(void))keymgmt_gettable_params},
> +    {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*)(void))keymgmt_get_params},
> +    {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*)(void))keymgmt_set_params},
> +    {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void
> (*)(void))keymgmt_gettable_params},       /* same as gettable */
> +    {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void
> (*)(void))ed448_keymgmt_name},
>

see above


> +    {0, NULL }
> +};
> +
> +static const OSSL_DISPATCH ed25519_keymgmt_functions[] = {
> +    {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},
> +    {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},
> +    {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void))keymgmt_load},
> +    {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void))keymgmt_has},
> +    {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void))keymgmt_match},
> +    {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void))ed25519_keymgmt_import},
> +    {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void
> (*)(void))keymgmt_import_types},
> +    {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void
> (*)(void))keymgmt_gettable_params},
> +    {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*)(void))keymgmt_get_params},
> +    {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*)(void))keymgmt_set_params},
> +    {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void
> (*)(void))keymgmt_gettable_params},       /* same as gettable */
> +    {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void
> (*)(void))ed25519_keymgmt_name},
>



> +    {0, NULL }
> +};
> +
> +
>  const OSSL_ALGORITHM keymgmts[] = {
>      {"RSA:rsaEncryption", XKEY_PROV_PROPS, rsa_keymgmt_functions,
> "OpenVPN xkey RSA Key Manager"},
>      {"RSA-PSS:RSASSA-PSS", XKEY_PROV_PROPS, rsa_keymgmt_functions,
> "OpenVPN xkey RSA-PSS Key Manager"},
>      {"EC:id-ecPublicKey", XKEY_PROV_PROPS, ec_keymgmt_functions, "OpenVPN
> xkey EC Key Manager"},
> +    {"ED448", XKEY_PROV_PROPS, ed448_keymgmt_functions, "OpenVPN xkey
> ED448 Key Manager"},
> +    {"ED25519", XKEY_PROV_PROPS, ed25519_keymgmt_functions, "OpenVPN xkey
> ED25519 Key Manager"},
>      {NULL, NULL, NULL, NULL}
>  };
>
> @@ -835,7 +936,7 @@ signature_sign_init(void *ctx, void *provkey, const
> OSSL_PARAM params[])
>      }
>      sctx->keydata = provkey;
>      sctx->keydata->refcount++; /* we are keeping a copy */
> -    sctx->sigalg.keytype = KEYTYPE(sctx->keydata) == EVP_PKEY_RSA ? "RSA"
> : "EC";
> +    sctx->sigalg.keytype = get_keytype(sctx->keydata);
>
>      signature_set_ctx_params(sctx, params);
>
> @@ -929,13 +1030,19 @@ signature_digest_sign_init(void *ctx, const char
> *mdname,
>      }
>      sctx->keydata = provkey; /* used by digest_sign */
>      sctx->keydata->refcount++;
> -    sctx->sigalg.keytype = KEYTYPE(sctx->keydata) == EVP_PKEY_RSA ? "RSA"
> : "EC";
> +    sctx->sigalg.keytype = get_keytype(sctx->keydata);
>
>      signature_set_ctx_params(ctx, params);
>      if (mdname)
>      {
>          sctx->sigalg.mdname = xkey_mdname(mdname); /* get a string
> literal pointer */
>      }
> +    else if (!strcmp(sctx->sigalg.keytype, "ED448") ||
> !strcmp(sctx->sigalg.keytype, "ED25519"))
> +    {
> +        /* EdDSA requires NULL as digest for the DigestSign API instead
> +         * of using the normal Sign API */
> +        sctx->sigalg.mdname = "none";
> +    }
>

Instead of this order, we have to assert that mdname must be NULL if
keytype is ED25519 or ED448. OpenSSL does not impose this when the key is
in provider, but docs for EVP_DigestSignInit() says mdname should be NULL
for such keys. So I think we should return an error if mdname is not NULL.

In fact we do call EVP_DigestSignInit() in our unit test with mdname=SHA256
and Ed25519 key (after this patch) which exposes this issue (see below).


>      else
>      {
>          msg(M_WARN, "xkey digest_sign_init: mdname is NULL.");
> @@ -1073,6 +1180,8 @@ static const OSSL_DISPATCH signature_functions[] = {
>  const OSSL_ALGORITHM signatures[] = {
>      {"RSA:rsaEncryption", XKEY_PROV_PROPS, signature_functions, "OpenVPN
> xkey RSA Signature"},
>      {"ECDSA", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey ECDSA
> Signature"},
> +    {"ED448", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey Ed448
> Signature"},
> +    {"ED25519", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey
> Ed25519 Signature"},
>      {NULL, NULL, NULL, NULL}
>  };
>
> diff --git a/tests/unit_tests/openvpn/test_provider.c
> b/tests/unit_tests/openvpn/test_provider.c
> index 0b0952ee2..e29252838 100644
> --- a/tests/unit_tests/openvpn/test_provider.c
> +++ b/tests/unit_tests/openvpn/test_provider.c
> @@ -66,7 +66,11 @@ static const char *const pubkey2 = "-----BEGIN PUBLIC
> KEY-----\n"
>
> "u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
>                                     "-----END PUBLIC KEY-----\n";
>
> -static const char *pubkeys[] = {pubkey1, pubkey2};
> +static const char *const pubkey3 = "-----BEGIN PUBLIC KEY-----\n"
> +
>  "MCowBQYDK2VwAyEA+q5xjF5hGyyqYZidJdz/0saEQabL3N4wIZJBxNGbgJE=\n"
> +                                   "-----END PUBLIC KEY-----";
> +
> +static const char *pubkeys[] = {pubkey1, pubkey2, pubkey3};
>
>  static const char *prov_name = "ovpn.xkey";
>
> @@ -158,12 +162,17 @@ management_query_pk_sig(struct management *man,
> const char *b64_data,
>      if (strstr(algorithm, "data=message"))
>      {
>          expected_tbs = test_msg_b64;
> -        assert_non_null(strstr(algorithm, "hashalg=SHA256"));
> +        /* ED25519 does not have a hash algorithm even though it goes via

+         * the DigestSign path (data=message) */
> +        if (!strstr(algorithm, "ED25519"))
> +        {
> +            assert_non_null(strstr(algorithm, "hashalg=SHA256"));
> +        }

     }
>      assert_string_equal(b64_data, expected_tbs); Though it still uses
> digestSign()
>
> -    /* We test using ECDSA or PSS with saltlen = digest */
> -    if (!strstr(algorithm, "ECDSA"))
> +    /* We test using ED25519, ECDSA or PSS with saltlen = digest */
> +    if (!strstr(algorithm, "ECDSA") && !strstr(algorithm, "ED25519"))
>      {
>          assert_non_null(strstr(algorithm,
> "RSA_PKCS1_PSS_PADDING,hashalg=SHA256,saltlen=digest"));
>      }
> @@ -328,13 +337,13 @@ xkey_sign(void *handle, unsigned char *sig, size_t
> *siglen,
>          assert_memory_equal(tbs, test_digest, sizeof(test_digest));
>      }
>
> -    /* For the test use sha256 and PSS padding for RSA */
> +    /* For the test use sha256 and PSS padding for RSA and none for EDDSA
> */
>      assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);
>

This works now because we wrongly call Ed25519 signature with mdname=sha256
in the test. I think we want this to be something like

 if (!strcmp(s.keytype, "ED25519"))
 {
     assert_string_equal(s.mdname, "none");
 }
else
{
    assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);
}

And, in digest_sign() we want to set mdname=NULL for Ed25519 key.. Like:

diff --git a/tests/unit_tests/openvpn/test_provider.c
b/tests/unit_tests/openvpn/test_provider.c
index e2925283..aa2ac7b9 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -237,6 +237,10 @@ digest_sign(EVP_PKEY *pkey)
         params[3] =
OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_MGF1_DIGEST, (char
*)saltlen, 0);
         params[4] = OSSL_PARAM_construct_end();
     }
+    else if (EVP_PKEY_get_id(pkey) == EVP_PKEY_ED25519)
+    {
+        mdname = NULL;
+    }


     if (!strcmp(s.keytype, "RSA"))
>      {
>          assert_string_equal(s.padmode, "pss"); /* we use PSS for the test
> */
>      }
> -    else if (strcmp(s.keytype, "EC"))
> +    else if (strcmp(s.keytype, "EC") && strcmp(s.keytype,"ED25519"))
>      {
>          fail_msg("Unknown keytype: %s", s.keytype);
>      }
>

Finally, xkey_management_sign() in xkey_helper.c should not call
xkey_digest() if mdname is "none". So line 182 in xkey_helper.c will have
to become something like:

if (!strcmp(alg.op, "DigestSign") && !(flags & MF_EXTERNAL_KEY_DIGEST) &&
strcmp(alg.mdname,"none"))

All look good otherwise.

Regards,

Selva
<div dir="ltr"><div>Hi,</div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 10, 2022 at 7:32 AM Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">OpenSSL&#39;s implementation of ED448 and ED25519 has a few idiosyncrasies.<br>
Instead of belonging to the eliptic curve type or to a common Edwards<br>
curve type, ED448 and ED25519 have each their own type.<br>
<br>
Also, OpenSSL excepts singatures using these curves to be done with the<br></blockquote><div><br></div><div>expects</div><div>signatures</div><div><br></div><div>(good to know I&#39;m not the only one afflicted by transposition errors)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
EVP_DigestSign API instead of the EVP_Sign API but using md=NULL. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This has been tested using a &quot;fake&quot; external key that used a normal<br>
software key instead of a hardware implementation but that makes no<br>
difference from the perspective of xkey_provider/managment interface.<br></blockquote><div><br></div><div>management</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
 src/openvpn/xkey_helper.c                |   4 +<br>
 src/openvpn/xkey_provider.c              | 121 +++++++++++++++++++++--<br>
 tests/unit_tests/openvpn/test_provider.c |  21 ++--<br>
 3 files changed, 134 insertions(+), 12 deletions(-)<br>
<br>
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c<br>
index ecc7b1204..d59b16858 100644<br>
--- a/src/openvpn/xkey_helper.c<br>
+++ b/src/openvpn/xkey_helper.c<br>
@@ -206,6 +206,10 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,<br>
             openvpn_snprintf(alg_str, sizeof(alg_str), &quot;ECDSA,hashalg=%s&quot;, alg.mdname);<br>
         }<br>
     }<br>
+    else if(!strcmp(alg.keytype, &quot;ED448&quot;) || !strcmp(alg.keytype, &quot;ED25519&quot;))<br>
+    {<br>
+        strncpynt(alg_str, alg.keytype, sizeof(alg_str));<br>
+    }<br>
     /* else assume RSA key */<br>
     else if (!strcmp(alg.padmode, &quot;pkcs1&quot;) &amp;&amp; (flags &amp; MF_EXTERNAL_KEY_PKCS1PAD))<br>
     {<br>
diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c<br>
index 46e57e0fe..e8ac3904b 100644<br>
--- a/src/openvpn/xkey_provider.c<br>
+++ b/src/openvpn/xkey_provider.c<br>
@@ -99,12 +99,28 @@ typedef struct<br>
     int refcount;                /**&lt; reference count */<br>
 } XKEY_KEYDATA;<br>
<br>
-static int<br>
-KEYTYPE(const XKEY_KEYDATA *key)<br>
+static inline const char *<br>
+get_keytype(const XKEY_KEYDATA *key)<br>
 {<br>
-    return key-&gt;pubkey ? EVP_PKEY_get_id(key-&gt;pubkey) : 0;<br>
+    int keytype = key-&gt;pubkey ? EVP_PKEY_get_id(key-&gt;pubkey) : 0;<br>
+<br>
+    switch (keytype)<br>
+    {<br>
+        case EVP_PKEY_RSA:<br>
+            return &quot;RSA&quot;;<br>
+<br>
+        case EVP_PKEY_ED448:<br>
+            return &quot;ED448&quot;;<br>
+<br>
+        case EVP_PKEY_ED25519:<br>
+            return &quot;ED25519&quot;;<br>
+<br>
+        default:<br>
+            return &quot;EC&quot;;</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    }<br>
 }<br>
<br>
+<br>
 static int<br>
 KEYSIZE(const XKEY_KEYDATA *key)<br>
 {<br>
@@ -310,6 +326,22 @@ ec_keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[])<br>
     return keymgmt_import(keydata, selection, params, &quot;EC&quot;);<br>
 }<br>
<br>
+static int<br>
+ed448_keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[])<br>
+{<br>
+    xkey_dmsg(D_XKEY, &quot;entry&quot;);<br>
+<br>
+    return keymgmt_import(keydata, selection, params, &quot;ED448&quot;);<br>
+}<br>
+<br>
+static int<br>
+ed25519_keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[])<br>
+{<br>
+    xkey_dmsg(D_XKEY, &quot;entry&quot;);<br>
+<br>
+    return keymgmt_import(keydata, selection, params, &quot;ED25519&quot;);<br>
+}<br>
+<br>
 /* This function has to exist for key import to work<br>
  * though we do not support import of individual params<br>
  * like n or e. We simply return an empty list here for<br>
@@ -449,7 +481,7 @@ keymgmt_import_helper(XKEY_KEYDATA *key, const OSSL_PARAM *params)<br>
         ASSERT(pkey);<br>
<br>
         int id = EVP_PKEY_get_id(pkey);<br>
-        if (id != EVP_PKEY_RSA &amp;&amp; id != EVP_PKEY_EC)<br>
+        if (id != EVP_PKEY_RSA &amp;&amp; id != EVP_PKEY_EC &amp;&amp; id != EVP_PKEY_ED25519 &amp;&amp; id != EVP_PKEY_ED448)<br>
         {<br>
             msg(M_WARN, &quot;Error: xkey keymgmt_import: unknown key type (%d)&quot;, id);<br>
             return 0;<br>
@@ -556,6 +588,40 @@ ec_keymgmt_name(int id)<br>
     return &quot;EC&quot;;<br>
 }<br>
<br>
+static const char *<br>
+ed448_keymgmt_name(int id)<br>
+{<br>
+    xkey_dmsg(D_XKEY, &quot;entry&quot;);<br>
+<br>
+    /* though we do not implement keyexch we could be queried for<br>
+     * keyexch mechanism supported by EC keys<br>
+     */<br>
+    if (id == OSSL_OP_SIGNATURE || id == OSSL_OP_KEYEXCH)<br>
+    {<br>
+        return &quot;ED448&quot;;</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    }<br>
+<br>
+    msg(D_XKEY, &quot;xkey ed448_keymgmt_name called with op_id != SIGNATURE or KEYEXCH id=%d&quot;, id);<br>
+    return &quot;EC&quot;;<br></blockquote><div><br></div><div>I think this should be either &quot;ED448&quot; or NULL. The default provider does not define the query_operation function for this key type in the dispatch table --- likely because the name will then default to the keymgmt name (ED448). Also, I just realized that in the default provider, the query_operation_name (when defined) returns NULL for ids not recognized. That leads to triggering an error message in the library which is good. </div><div><br></div><div>We could either return NULL here or &quot;ED448&quot; and then convert the above debug msg() to a warning. Or leave out this function altogether and remove it from the dispatch table.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}<br>
+<br>
+static const char *<br>
+ed25519_keymgmt_name(int id)<br>
+{<br>
+    xkey_dmsg(D_XKEY, &quot;entry&quot;);<br>
+<br>
+    /* though we do not implement keyexch we could be queried for<br>
+     * keyexch mechanism supported by EC keys<br>
+     */<br>
+    if (id == OSSL_OP_SIGNATURE || id == OSSL_OP_KEYEXCH)<br>
+    {<br>
+        return &quot;ED25519&quot;;<br>
+    } </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+    msg(D_XKEY, &quot;xkey ed448_keymgmt_name called with op_id != SIGNATURE or KEYEXCH id=%d&quot;, id);<br>
+    return &quot;EC&quot;;<br></blockquote><div><br></div><div>same comment as above with ED448 --&gt; ED25519.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}<br>
+<br>
 static const OSSL_DISPATCH rsa_keymgmt_functions[] = {<br>
     {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},<br>
     {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},<br>
@@ -588,10 +654,45 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = {<br>
     {0, NULL }<br>
 };<br>
<br>
+static const OSSL_DISPATCH ed448_keymgmt_functions[] = {<br>
+    {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},<br>
+    {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},<br>
+    {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void))keymgmt_load},<br>
+    {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void))keymgmt_has},<br>
+    {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void))keymgmt_match},<br>
+    {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void))ed448_keymgmt_import},<br>
+    {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void (*)(void))keymgmt_import_types},<br>
+    {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void (*)(void))keymgmt_gettable_params},<br>
+    {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*)(void))keymgmt_get_params},<br>
+    {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*)(void))keymgmt_set_params},<br>
+    {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void (*)(void))keymgmt_gettable_params},       /* same as gettable */<br>
+    {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void (*)(void))ed448_keymgmt_name},<br></blockquote><div><br></div><div>see above</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    {0, NULL }<br>
+};<br>
+<br>
+static const OSSL_DISPATCH ed25519_keymgmt_functions[] = {<br>
+    {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void))keymgmt_new},<br>
+    {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void))keymgmt_free},<br>
+    {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void))keymgmt_load},<br>
+    {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void))keymgmt_has},<br>
+    {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void))keymgmt_match},<br>
+    {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void))ed25519_keymgmt_import},<br>
+    {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void (*)(void))keymgmt_import_types},<br>
+    {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void (*)(void))keymgmt_gettable_params},<br>
+    {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*)(void))keymgmt_get_params},<br>
+    {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*)(void))keymgmt_set_params},<br>
+    {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void (*)(void))keymgmt_gettable_params},       /* same as gettable */<br>
+    {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void (*)(void))ed25519_keymgmt_name},<br>
</blockquote><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+    {0, NULL }<br>
+};<br>
+<br>
+<br>
 const OSSL_ALGORITHM keymgmts[] = {<br>
     {&quot;RSA:rsaEncryption&quot;, XKEY_PROV_PROPS, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA Key Manager&quot;},<br>
     {&quot;RSA-PSS:RSASSA-PSS&quot;, XKEY_PROV_PROPS, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA-PSS Key Manager&quot;},<br>
     {&quot;EC:id-ecPublicKey&quot;, XKEY_PROV_PROPS, ec_keymgmt_functions, &quot;OpenVPN xkey EC Key Manager&quot;},<br>
+    {&quot;ED448&quot;, XKEY_PROV_PROPS, ed448_keymgmt_functions, &quot;OpenVPN xkey ED448 Key Manager&quot;},<br>
+    {&quot;ED25519&quot;, XKEY_PROV_PROPS, ed25519_keymgmt_functions, &quot;OpenVPN xkey ED25519 Key Manager&quot;},<br>
     {NULL, NULL, NULL, NULL}<br>
 };<br>
<br>
@@ -835,7 +936,7 @@ signature_sign_init(void *ctx, void *provkey, const OSSL_PARAM params[])<br>
     }<br>
     sctx-&gt;keydata = provkey;<br>
     sctx-&gt;keydata-&gt;refcount++; /* we are keeping a copy */<br>
-    sctx-&gt;sigalg.keytype = KEYTYPE(sctx-&gt;keydata) == EVP_PKEY_RSA ? &quot;RSA&quot; : &quot;EC&quot;;<br>
+    sctx-&gt;sigalg.keytype = get_keytype(sctx-&gt;keydata);<br>
<br>
     signature_set_ctx_params(sctx, params);<br>
<br>
@@ -929,13 +1030,19 @@ signature_digest_sign_init(void *ctx, const char *mdname,<br>
     }<br>
     sctx-&gt;keydata = provkey; /* used by digest_sign */<br>
     sctx-&gt;keydata-&gt;refcount++;<br>
-    sctx-&gt;sigalg.keytype = KEYTYPE(sctx-&gt;keydata) == EVP_PKEY_RSA ? &quot;RSA&quot; : &quot;EC&quot;;<br>
+    sctx-&gt;sigalg.keytype = get_keytype(sctx-&gt;keydata);<br>
<br>
     signature_set_ctx_params(ctx, params);<br>
     if (mdname)<br>
     {<br>
         sctx-&gt;sigalg.mdname = xkey_mdname(mdname); /* get a string literal pointer */<br>
     }<br>
+    else if (!strcmp(sctx-&gt;sigalg.keytype, &quot;ED448&quot;) || !strcmp(sctx-&gt;sigalg.keytype, &quot;ED25519&quot;))<br>
+    {<br>
+        /* EdDSA requires NULL as digest for the DigestSign API instead<br>
+         * of using the normal Sign API */<br>
+        sctx-&gt;sigalg.mdname = &quot;none&quot;;<br>
+    }<br></blockquote><div><br></div><div>Instead of this order, we have to assert that mdname must be NULL if keytype is ED25519 or ED448. OpenSSL does not impose this when the key is in provider, but docs for EVP_DigestSignInit() says mdname should be NULL for such keys. So I think we should return an error if mdname is not NULL.</div><div><br></div><div>In fact we do call EVP_DigestSignInit() in our unit test with mdname=SHA256 and Ed25519 key (after this patch) which exposes this issue (see below).</div><div>  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     else<br>
     {<br>
         msg(M_WARN, &quot;xkey digest_sign_init: mdname is NULL.&quot;);<br>
@@ -1073,6 +1180,8 @@ static const OSSL_DISPATCH signature_functions[] = {<br>
 const OSSL_ALGORITHM signatures[] = {<br>
     {&quot;RSA:rsaEncryption&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey RSA Signature&quot;},<br>
     {&quot;ECDSA&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey ECDSA Signature&quot;},<br>
+    {&quot;ED448&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey Ed448 Signature&quot;},<br>
+    {&quot;ED25519&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey Ed25519 Signature&quot;},<br>
     {NULL, NULL, NULL, NULL}<br>
 };<br>
<br>
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c<br>
index 0b0952ee2..e29252838 100644<br>
--- a/tests/unit_tests/openvpn/test_provider.c<br>
+++ b/tests/unit_tests/openvpn/test_provider.c<br>
@@ -66,7 +66,11 @@ static const char *const pubkey2 = &quot;-----BEGIN PUBLIC KEY-----\n&quot;<br>
                                    &quot;u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n&quot;<br>
                                    &quot;-----END PUBLIC KEY-----\n&quot;;<br>
<br>
-static const char *pubkeys[] = {pubkey1, pubkey2};<br>
+static const char *const pubkey3 = &quot;-----BEGIN PUBLIC KEY-----\n&quot;<br>
+                                   &quot;MCowBQYDK2VwAyEA+q5xjF5hGyyqYZidJdz/0saEQabL3N4wIZJBxNGbgJE=\n&quot;<br>
+                                   &quot;-----END PUBLIC KEY-----&quot;;<br>
+<br>
+static const char *pubkeys[] = {pubkey1, pubkey2, pubkey3};<br>
<br>
 static const char *prov_name = &quot;ovpn.xkey&quot;;<br>
<br>
@@ -158,12 +162,17 @@ management_query_pk_sig(struct management *man, const char *b64_data,<br>
     if (strstr(algorithm, &quot;data=message&quot;))<br>
     {<br>
         expected_tbs = test_msg_b64;<br>
-        assert_non_null(strstr(algorithm, &quot;hashalg=SHA256&quot;));<br>
+        /* ED25519 does not have a hash algorithm even though it goes via</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+         * the DigestSign path (data=message) */<br>
+        if (!strstr(algorithm, &quot;ED25519&quot;))<br>
+        {<br>
+            assert_non_null(strstr(algorithm, &quot;hashalg=SHA256&quot;));<br>
+        } </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     }<br>
     assert_string_equal(b64_data, expected_tbs); Though it still uses digestSign()<br>
<br>
-    /* We test using ECDSA or PSS with saltlen = digest */<br>
-    if (!strstr(algorithm, &quot;ECDSA&quot;))<br>
+    /* We test using ED25519, ECDSA or PSS with saltlen = digest */<br>
+    if (!strstr(algorithm, &quot;ECDSA&quot;) &amp;&amp; !strstr(algorithm, &quot;ED25519&quot;))<br>
     {<br>
         assert_non_null(strstr(algorithm, &quot;RSA_PKCS1_PSS_PADDING,hashalg=SHA256,saltlen=digest&quot;));<br>
     }<br>
@@ -328,13 +337,13 @@ xkey_sign(void *handle, unsigned char *sig, size_t *siglen,<br>
         assert_memory_equal(tbs, test_digest, sizeof(test_digest));<br>
     }<br>
<br>
-    /* For the test use sha256 and PSS padding for RSA */<br>
+    /* For the test use sha256 and PSS padding for RSA and none for EDDSA */<br>
     assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);<br></blockquote><div><br></div><div>This works now because we wrongly call Ed25519 signature with mdname=sha256 in the test. I think we want this to be something like</div><div><br></div><div> if (!strcmp(s.keytype, &quot;ED25519&quot;))</div> {</div><div class="gmail_quote">     assert_string_equal(s.mdname, &quot;none&quot;);<br> }<br>else</div><div class="gmail_quote">{<br>    assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);<br>}</div><div class="gmail_quote"><br></div><div class="gmail_quote">And, in digest_sign() we want to set mdname=NULL for Ed25519 key.. Like:</div><div class="gmail_quote"><br></div><div class="gmail_quote">diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c<br>index e2925283..aa2ac7b9 100644<br>--- a/tests/unit_tests/openvpn/test_provider.c<br>+++ b/tests/unit_tests/openvpn/test_provider.c<br>@@ -237,6 +237,10 @@ digest_sign(EVP_PKEY *pkey)<br>         params[3] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_MGF1_DIGEST, (char *)saltlen, 0);<br>         params[4] = OSSL_PARAM_construct_end();<br>     }<br>+    else if (EVP_PKEY_get_id(pkey) == EVP_PKEY_ED25519)<br>+    {<br>+        mdname = NULL;<br>+    }<br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     if (!strcmp(s.keytype, &quot;RSA&quot;))<br>
     {<br>
         assert_string_equal(s.padmode, &quot;pss&quot;); /* we use PSS for the test */<br>
     }<br>
-    else if (strcmp(s.keytype, &quot;EC&quot;))<br>
+    else if (strcmp(s.keytype, &quot;EC&quot;) &amp;&amp; strcmp(s.keytype,&quot;ED25519&quot;))<br>
     {<br>
         fail_msg(&quot;Unknown keytype: %s&quot;, s.keytype);<br>
     }<br></blockquote><div> <br></div><div>Finally, xkey_management_sign() in xkey_helper.c should not call xkey_digest() if mdname is &quot;none&quot;. So line 182 in xkey_helper.c will have to become something like:</div><div><br></div><div>if (!strcmp(alg.op, &quot;DigestSign&quot;) &amp;&amp; !(flags &amp; MF_EXTERNAL_KEY_DIGEST) &amp;&amp; strcmp(alg.mdname,&quot;none&quot;))<br></div><div><br></div><div>All look good otherwise.</div><div><br></div><div>Regards,</div><div><br></div><div>Selva </div></div></div></div>