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

[WIP] Allow keys without available public key to be used for SSL #10954

Closed
wants to merge 1 commit into from

Conversation

dwmw2
Copy link
Contributor

@dwmw2 dwmw2 commented Jan 27, 2020

This is the continuation of #1551. This is actually against 1.1.1 not 3.0.0 as discussed, but it's basically the same and testing on 1.1.1 means I don't have to rebuild the whole world including the PKCS#11 engine.

It's a special case for EC for now; we can later extend the generic EVP_PKEY API to have a generic "here's a public key I checked matches this private key; you might want to remember it" method.

But first we need to deal with the fact that X509_check_private_key(), where we want to do this thing, now takes a 'const' private key, so it's not clear if we should be doing anything of that form at all.

From my notes in #1551;

crypto/x509/x509_cmp.c: In function 'check_key_signature':
crypto/x509/x509_cmp.c:280:42: warning: passing argument 1 of 'EVP_PKEY_CTX_new' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(priv, NULL);
                                          ^~~~
In file included from include/openssl/x509.h:18:0,
                 from crypto/x509/x509_cmp.c:14:
include/openssl/evp.h:1338:15: note: expected 'EVP_PKEY * {aka struct evp_pkey_st *}' but argument is of type 'const EVP_PKEY * {aka const struct evp_pkey_st *}'
 EVP_PKEY_CTX *EVP_PKEY_CTX_new(EVP_PKEY *pkey, ENGINE *e);
               ^~~~~~~~~~~~~~~~
crypto/x509/x509_cmp.c:304:28: warning: passing argument 1 of 'EVP_PKEY_CTX_new' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     ctx = EVP_PKEY_CTX_new(pub, NULL);
                            ^~~
In file included from include/openssl/x509.h:18:0,
                 from crypto/x509/x509_cmp.c:14:
include/openssl/evp.h:1338:15: note: expected 'EVP_PKEY * {aka struct evp_pkey_st *}' but argument is of type 'const EVP_PKEY * {aka const struct evp_pkey_st *}'
 EVP_PKEY_CTX *EVP_PKEY_CTX_new(EVP_PKEY *pkey, ENGINE *e);
               ^~~~~~~~~~~~~~~~

The correct way to fix that is not obvious. Is the first argument to EVP_PKEY_CTX_new() really not const? Casting to non-const seems wrong, and that's just for the first part. When we later come to copy the public key data, it gets even wronger...

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Jan 27, 2020
Fixes: 1532

I had previously made it stop segfaulting, which was a good start.
This makes it *work*.

It's a special case for EC for now; we can later extend the generic
EVP_PKEY API to have a generic "here's a public key I checked matches
this private key; you might want to remember it" method.

But first we need to deal with the fact that X509_check_private_key(),
where we want to do this thing, now takes a 'const' private key, so it's
not clear if we should be doing *anything* of that form at all.
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Jan 27, 2020
@levitte
Copy link
Member

levitte commented Jan 27, 2020

I believe that the sane thing is to consider making it a separate public functions, something like X509_check_signature, to be called separately fromin conjunction with, or maybe even instead of X509_check_private_key.

While you're at it, I suggest also adding X509_REQ_check_private_keyX509_REQ_check_signature, for API consistency.

@t8m t8m added branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature labels Jul 23, 2021
@t8m t8m added this to the Post 3.0.0 milestone Jul 23, 2021
@t8m t8m added the stalled: awaiting contributor response This pull request is awaiting a response by the contributor label Jul 23, 2021
@t8m t8m closed this Nov 20, 2023
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 stalled: awaiting contributor response This pull request is awaiting a response by the contributor triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants