EVP_PKEY_(get_)id not returning an OID (for provider PKEYs)

[baentsch]

The masterdocumentation for EVP_PKEY_get_id (and thus for the old API EVP_PKEY_id) is documented to return an OID:

EVP_PKEY_get_id() returns the actual OID associated with pkey.

Is this really correct? It rather seems to return a NID (or doesn't "OID" in this context refer to an X.509 OID but rather to an "OpenSSL internal object ID"?). In that case some clarification wrt "OID" may be in order (Apologies if I missed this somewhere).

The result is always used as a NID anyway. The one use that causes acute problems is here:

The issue is that a provider-based PKEY always causes EVP_PKEY(_get)_id to return -1 -- which is rather "suboptimal" for code interested in outputting the (SN) name of the algorithm.

Or asked another way: How should an OpenSSL consumer (like OpenVPN) obtain the name (and/or NID) of the algorithm underlying a PKEY if that PKEY is provider-based? Ideally, it shouldn't know/have to worry about this "OpenSSL internal implementation detail", right?

In the OpenVPN use case, the problem is made worse by OpenSSL setting an error state due to the API call sequence EVP_PKEY_get_id-> OBJ_nid2sn: This causes the whole OpenVPN system to fail as OpenSSL reported an error:

OpenSSL: error:04000065:object identifier routines::unknown nid

but only when logging is set too high (trying to obtain the algorithm name as per the above). Really surprising to users. Does an error state have to be set if a "provider-based" PKEY is examined?

[mattcaswell]

Is this really correct?

No, it's a NID. Usually there's a one-to-one correspondence from NID to OID (OBJ_nid2oid), so I guess that's where the confusion comes from.

Or asked another way: How should an OpenSSL consumer (like OpenVPN) obtain the name (and/or NID) of the algorithm underlying a PKEY if that PKEY is provider-based? Ideally, it shouldn't know/have to worry about this "OpenSSL internal implementation detail", right?

If we're talking about a name for human consumption, does EVP_PKEY_get0_description do the trick?

[mattcaswell]

There is also EVP_PKEY_get0_type_name()

[baentsch]

No, it's a NID.

OK, then I'd suggest a documentation update (will do so if no-one jumps at this). I'd also add the correction that the retval from that function may actually be -1 (not just NID_undef or positive NID as currently documented) thus indicating a provided key. Would it be sensible to then also directly point to EVP_PKEY_get0_type_name for that case?

does EVP_PKEY_get0_description do the trick?

No, that also returns NULL (but that might arguably be changed by the provider implementation). But it would not solve the issue for OpenVPN: Are you saying the code there must be updated to support providers? Adding a call to EVP_PKEY_get0_type_name would be sufficient if EVP_PKEY_(get_)id returns -1.

If so, do you recommend a "standard way" how to utilize OpenSSLv3 APIs (such as EVP_PKEY_get0_type_name) in "consumer code" (like OpenVPN)? Something like #if OPENSSL_VERSION >= 0x30000000L?

[mattcaswell]

may actually be -1 (not just NID_undef or positive NID as currently documented)

That is really unfortunate. Arguably that's a bug. We should either fix it, or fix the documentation. I worry whether it might be too late to actually fix it though??? Maybe applications already rely on this. Not sure.

Are you saying the code there must be updated to support providers?

Looks like it. It should still be ok if its a provider supplied sig alg that OpenSSL already knows about - but if it is completely unknown to OpenSSL its just not going to work.

Adding a call to EVP_PKEY_get0_type_name would be sufficient if EVP_PKEY_(get_)id returns -1.

Shouldn't you be able to just always call EVP_PKEY_get0_type_name in OpenSSL 3.0 (and not call EVP_PKEY_get_id at all)? That should work even for legacy keys.

OPENSSL_VERSION_NUMBER can be checked for conditional compilation where support for older APIs is required. See:

https://www.openssl.org/docs/man3.0/man3/OPENSSL_VERSION_NUMBER.html

Note that page describes numerous macros for checking the version number - but many of those were introduced in 3.0. OPENSSL_VERSION_NUMBER can reliably be used for older versions.

[baentsch]

That is really unfortunate. Arguably that's a bug. We should either fix it, or fix the documentation. I worry whether it might be too late to actually fix it though??? Maybe applications already rely on this.

Their fault if they do. It's clearly documented. I'd suggest fixing the code instead. It would also solve the "downstream" OpenVPN problem: The whole code causing the problem would not trigger again if the retval of EVP_PKEY_get_id were 0 (or a real NID): https://github.com/OpenVPN/openvpn/blob/838474145933199a62d1f59fbc2df14e4fbd57f3/src/openvpn/ssl_openssl.c#L2083-L2107

As simple as

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 554fad927c..6477594017 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -982,7 +982,7 @@ int EVP_PKEY_type(int type)
 
 int EVP_PKEY_get_id(const EVP_PKEY *pkey)
 {
-    return pkey->type;
+    return pkey->type > 0 ? pkey->type : NID_undef;
 }

OK to PR?

[mattcaswell]

Let's have a PR for it. We can gather more opinion there.