Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVP_PKEY_eq() doesn't live up to its promise for provider keys #16704

Closed
levitte opened this issue Sep 29, 2021 · 5 comments
Closed

EVP_PKEY_eq() doesn't live up to its promise for provider keys #16704

levitte opened this issue Sep 29, 2021 · 5 comments
Labels
branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug

Comments

@levitte
Copy link
Member

levitte commented Sep 29, 2021

This was identified in another issue: #16267 (comment)

In EVP_PKEY_eq(3), the following is found:

DESCRIPTION
...
The function EVP_PKEY_eq() checks the public key components and parameters (if present) of keys a and b for equality.
...
NOTES
...
Since OpenSSL private keys contain public key components too the function EVP_PKEY_eq() can also be used to determine if a private key matches a public key.

So for all intents and purposes, was assumed that EVP_PKEY_eq() could compare any key, no matter what...

... except we decided that an EVP_PKEY doesn't necessarily have to hold a public key, and the assumption that EVP_PKEY_eq() can be used to compare private keys via their public key no longer holds.

Do note that for legacy EVP_PKEYs, the ever present public key is still a valid assumption (because existing implementations are built with the older assumption, and the method structure obligates it anyway), so this issue is only about EVP_PKEYs with provider backends.

@levitte levitte added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Sep 29, 2021
@baentsch
Copy link
Contributor

Referring to this comment and FWIW I completely agree that this text should be made more visible particularly in the context of provider keys:

=for comment One might think that a combination of bits means that all
the selected data subsets must be considered, but then you have to
consider that when comparing key objects (future function), an
implementation might opt to not compare the private key if it has
compared the public key, since a match of one half implies a match of
the other half.

The reverse statement (private key equality means public key equality) then also permits the implementation of key matching for OQS keys so far tentatively done: If no public key is present, but OSSL_KEYMGMT_SELECT_PUBLIC_KEY is requested, a private key compare is done instead: https://github.com/open-quantum-safe/oqs-provider/blob/6469b466475ea426d637dbbb31b46d88110a7c0c/oqsprov/oqs_kmgmt.c#L130-L137

@levitte
Copy link
Member Author

levitte commented Sep 29, 2021

If no public key is present, but OSSL_KEYMGMT_SELECT_PUBLIC_KEY is requested, a private key compare is done instead

It might be wise to check that any of OSSL_KEYMGMT_SELECT_KEYPAIR is present.

@levitte
Copy link
Member Author

levitte commented Mar 23, 2022

This should be closed, shouldn't it?

@baentsch
Copy link
Contributor

This should be closed, shouldn't it?

Yes. But it looks like I don't have permission to do so.

@levitte
Copy link
Member Author

levitte commented Mar 23, 2022

This should be closed, shouldn't it?

Yes. But it looks like I don't have permission to do so.

No problem, I'm doing it. Now

@levitte levitte closed this as completed Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants