openssl
openssl copied to clipboard
openssl
Unexpected success from EVP_PKEY_fromdata
The function EVP_PKEY_fromdata will return success even in instances where it has failed to import a key successfully.
In this email to openssl-users the OP reports an instance where EVP_PKEY_fromdata indicates success when importing a public key: https://mta.openssl.org/pipermail/openssl-users/2021-October/014479.html
The same report goes on to describe a subsequent crash which is fixed by #16911.
The problem with the EVP_PKEY_fromdata call is that the OP attempted to import a public key using OSSL_PKEY_PARAM_EC_PUB_X and OSSL_PKEY_PARAM_EC_PUB_Y which are only supported as "getters", i.e. not for import. Since the OP explicitly indicates EVP_PKEY_PUBLIC_KEY in the EVP_PKEY_fromdata call I would expect it to fail since a public key was not imported. However this is not the case. Instead it simply imports the parameters, ignores the unknown public key params and returns success, having created a parameters only key.
This seems quite unexpected.
However fixing this is quite tricky. In order to indicate an attempt to import a private key you might be expected to use EVP_PKEY_KEYPAIR as the selection parameter (there is no EVP_PKEY_PRIVATE_KEY - although if it did exist it would be the same as OSSL_KEYMGMT_SELECT_PRIVATE_KEY). However EVP_PKEY_KEYPAIR actually indicates both the public and private key are present - but in some key types (including EC) the import is deliberately tolerant of a private key without a public key. So it doesn't seem to be as simple as saying "if EVP_PKEY_PUBLIC_KEY is specified and no public key is present then fail" since in the EVP_PKEY_KEYPAIR case, EVP_PKEY_PUBLIC_KEY is incorporated but it is valid to have no public key.
IMO if EVP_PKEY_KEYPAIR is specified - you should import at least public or private key part. Basically the selection should be the maximum that is possibly being imported. However importing no part of the KEYPAIR should be an error.
IMO if EVP_PKEY_KEYPAIR is specified - you should import at least public or private key part. Basically the selection should be the maximum that is possibly being imported. However importing no part of the KEYPAIR should be an error.
Should we actually say if EVP_PKEY_KEYPAIR is specified then it must at least have the private key, and the public is optional (dependant on the key type)? Otherwise if you really want a private key, then you will get success back even if you failed to specify the private key parameters successfully but did manage to specify the public params. However if you specify EVP_PKEY_PUBLIC_KEY then the the public key must always be present?
Yeah this semantics also would make sense. It does not matter much which one is chosen but it should be documented.
My interpretation is my preferred approach. Otherwise if you attempt to import a private key you have no way of knowing whether it actually worked and you ended up with a private key.
Unfortunately, implementing this might break things. It at least breaks libssl due to this:
https://github.com/openssl/openssl/blob/b387274d0fb3097d3a252d397226b86b8f98f30d/ssl/statem/statem_clnt.c#L2045-L2065
In this code we specify EVP_PKEY_KEYPAIR but only supply the public portion :-(
This problem is eerily similar to what #16705 is trying to solve, no? Sure, there it's about key matching, but, the way to think around KEYPAIR is similar.
My reading of the documentation is that selection describes a limit on what to import, but does not require that anything not in the supplied params has to be imported. If selection says private key and params is, say, NULL, the call could still be a success though nothing is imported.
Otherwise EVP_PKEY_PUBLIC_KEY will have to be interpreted as public key + parameters, KEYPAIR as everything leaving no option for importing private key without the rest. And custom providers will have to respect these too as EVP_PKEY_fromdata() could end up in keymgmt_import() of any provider. In other words, putting requirements on a call like EVP_PKEY_fromdata() imposes conditions on all providers.
A custom provider could have other requirements like certain selections are not allowed or importing private key requires public key etc., and can trigger an error and document those.
It sounds like core the issue is the OSSL_PARAM semantic of ignoring unused parameters. That seems not the right design for precisely this issue. If you mis-specify an optional parameter, you won't notice.
Another example is "rsa-factorN". If the caller specifies a multi-prime RSA key, but the provider only supports two-prime RSA, the extra factors will get silently chopped off. Since OpenSSL 3.0 also did not set the precedent of validating keys on import (see #13615), it won't get caught that way either.
Most programming languages with keyword arguments similarly don't allow unused parameters, likely for much the same reason:
$ cat /tmp/foo.py
def import_ec_key(group, pub=None, priv=None):
# pub and priv are optional, if you want to make a params-only object, or
# whatver.
pass
import_ec_key(group="p256", x=1234, y=5678)
$ python3 /tmp/foo.py
Traceback (most recent call last):
File "/tmp/foo.py", line 6, in <module>
import_ec_key(group="p256", x=1234, y=5678)
TypeError: import_ec_key() got an unexpected keyword argument 'x'
Regarding private-only EC keys, you can compute the public key from the private key, so if OpenSSL wants to support private-only import, it's probably simplest to just compute it, rather than have all subsequent codepaths worry about this extra state. But, in this case, the public key was specified, just OpenSSL silently ignored it.
This problem is eerily similar to what #16705 is trying to solve, no? Sure, there it's about key matching, but, the way to think around KEYPAIR is similar.
IMO, #16705 takes a wrong approach. Will elaborate there.
Regarding private-only EC keys, you can compute the public key from the private key, so if OpenSSL wants to support private-only import, it's probably simplest to just compute it, rather than have all subsequent codepaths worry about this extra state.
There are two objections to "simply computing the public key" a) It involves substantial computational effort which, in most cases of practical interest, is entirely wasted. b) Unnecessary computation involving the private key, exposes this closely guarded secret to possible side-channel attack.
But, in this case, the public key was specified, just OpenSSL silently ignored it.
The silence is regrettable, but the real issue here is the incomplete mechanism for specifying EC public keys, which topic is addressed in #16270.
This function seems to be part of the problem that @mattcaswell reports... as far as I can tell from reading the code, it assumes that it's got something and therefore doesn't check if it got nothing:
https://github.com/openssl/openssl/blob/b387274d0fb3097d3a252d397226b86b8f98f30d/crypto/ec/ec_backend.c#L391-L492
In the beginning, these functions were only called by the export/import dance for legacy keys, so the selection was a declaration of what was found in those, not something that was passed directly by the user.
Fix in #17408 however do we really want the change and if so, also on 3.0 branch?
From #17408 it appears the decision was made to document the behavior rather than change it. Will someone open a PR for this? If not it seems it could be closed
no response, marking as inactive, to be closed at the end of 3.4 dev barring further input
I think we should consider doing a docfix for this. It is fairly surprising behavior so it should be at least mentioned in the documentation.