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.

Sign up for GitHub

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

Jump to bottom

ossl_pkey.c: Workaround: Decode with non-zero selections. #669

Merged
merged 2 commits into from Aug 25, 2023
Merged

ossl_pkey.c: Workaround: Decode with non-zero selections. #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d157ba1
Remove the pending logics by the pend_on_openssl_issue_21493.
junaruga Aug 22, 2023
db688fa
ossl_pkey.c: Workaround: Decode with non-zero selections.
junaruga Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 59 additions & 52 deletions ext/openssl/ossl_pkey.c
View file
Open in desktop
Expand Up @@ -82,30 +82,62 @@ ossl_pkey_new(EVP_PKEY *pkey)
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
# include <openssl/decoder.h>

EVP_PKEY *
ossl_pkey_read_generic(BIO *bio, VALUE pass)
static EVP_PKEY *
ossl_pkey_read(BIO *bio, const char *input_type, int selection, VALUE pass)
junaruga marked this conversation as resolved.
Show resolved Hide resolved
{
void *ppass = (void *)pass;
OSSL_DECODER_CTX *dctx;
EVP_PKEY *pkey = NULL;
int pos = 0, pos2;

dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, NULL, 0, NULL, NULL);
dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, input_type, NULL, NULL,
selection, NULL, NULL);
if (!dctx)
goto out;
if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1)
goto out;

/* First check DER */
if (OSSL_DECODER_from_bio(dctx, bio) == 1)
if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb,
ppass) != 1)
goto out;
while (1) {
if (OSSL_DECODER_from_bio(dctx, bio) == 1)
goto out;
if (BIO_eof(bio))
break;
pos2 = BIO_tell(bio);
if (pos2 < 0 || pos2 <= pos)
break;
ossl_clear_error();
pos = pos2;
}
out:
OSSL_BIO_reset(bio);
OSSL_DECODER_CTX_free(dctx);
return pkey;
}

EVP_PKEY *
ossl_pkey_read_generic(BIO *bio, VALUE pass)
{
EVP_PKEY *pkey = NULL;
/* First check DER, then check PEM. */
const char *input_types[] = {"DER", "PEM"};
int input_type_num = (int)(sizeof(input_types) / sizeof(char *));
/*
* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed.
* Non-zero selections to try to decode.
*
* See EVP_PKEY_fromdata(3) - Selections to see all the selections.
*
* First check for private key formats. This is to keep compatibility with
* ruby/openssl < 3.0 which decoded the following as a private key.
* This is a workaround for the decoder failing to decode or returning
* bogus keys with selection 0, if a key management provider is different
* from a decoder provider. The workaround is to avoid using selection 0.
*
* Affected OpenSSL versions: >= 3.1.0, <= 3.1.2, or >= 3.0.0, <= 3.0.10
* Fixed OpenSSL versions: 3.2, next release of the 3.1.z and 3.0.z
*
* See https://github.com/openssl/openssl/pull/21519 for details.
*
* First check for private key formats (EVP_PKEY_KEYPAIR). This is to keep
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the (EVP_PKEY_KEYPAIR) here to explain why we need to check the EVP_PKEY_KEYPAIR case first.

* compatibility with ruby/openssl < 3.0 which decoded the following as a
* private key.
*
* $ openssl ecparam -name prime256v1 -genkey -outform PEM
* -----BEGIN EC PARAMETERS-----
Expand All @@ -126,50 +158,25 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass)
*
* Note that we need to create the OSSL_DECODER_CTX variable each time when
* we use the different selection as a workaround.
* https://github.com/openssl/openssl/issues/20657
* See https://github.com/openssl/openssl/issues/20657 for details.
*/
OSSL_DECODER_CTX_free(dctx);
dctx = NULL;
dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, NULL,
EVP_PKEY_KEYPAIR, NULL, NULL);
if (!dctx)
goto out;
if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1)
goto out;
while (1) {
if (OSSL_DECODER_from_bio(dctx, bio) == 1)
goto out;
if (BIO_eof(bio))
break;
pos2 = BIO_tell(bio);
if (pos2 < 0 || pos2 <= pos)
break;
ossl_clear_error();
pos = pos2;
}

OSSL_BIO_reset(bio);
OSSL_DECODER_CTX_free(dctx);
dctx = NULL;
dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, NULL, 0, NULL, NULL);
if (!dctx)
goto out;
if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1)
goto out;
while (1) {
if (OSSL_DECODER_from_bio(dctx, bio) == 1)
goto out;
if (BIO_eof(bio))
break;
pos2 = BIO_tell(bio);
if (pos2 < 0 || pos2 <= pos)
break;
ossl_clear_error();
pos = pos2;
int selections[] = {
EVP_PKEY_KEYPAIR,
EVP_PKEY_KEY_PARAMETERS,
EVP_PKEY_PUBLIC_KEY
};
int selection_num = (int)(sizeof(selections) / sizeof(int));
int i, j;

for (i = 0; i < input_type_num; i++) {
for (j = 0; j < selection_num; j++) {
pkey = ossl_pkey_read(bio, input_types[i], selections[j], pass);
if (pkey) {
goto out;
}
}
junaruga marked this conversation as resolved.
Show resolved Hide resolved
}

out:
OSSL_DECODER_CTX_free(dctx);
return pkey;
}
#else
Expand Down
6 changes: 0 additions & 6 deletions test/openssl/test_pkey.rb
View file
Open in desktop
Expand Up @@ -82,8 +82,6 @@ def test_hmac_sign_verify
end

def test_ed25519
pend_on_openssl_issue_21493

# Test vector from RFC 8032 Section 7.1 TEST 2
priv_pem = <<~EOF
-----BEGIN PRIVATE KEY-----
Expand Down Expand Up @@ -148,8 +146,6 @@ def test_ed25519
end

def test_x25519
pend_on_openssl_issue_21493

# Test vector from RFC 7748 Section 6.1
alice_pem = <<~EOF
-----BEGIN PRIVATE KEY-----
Expand Down Expand Up @@ -202,8 +198,6 @@ def raw_initialize
end

def test_compare?
pend_on_openssl_issue_21493

key1 = Fixtures.pkey("rsa1024")
key2 = Fixtures.pkey("rsa1024")
key3 = Fixtures.pkey("rsa2048")
Expand Down
16 changes: 0 additions & 16 deletions test/openssl/utils.rb
View file
Open in desktop
Expand Up @@ -144,22 +144,6 @@ def libressl?(major = nil, minor = nil, fix = nil)
return false unless version
!major || (version.map(&:to_i) <=> [major, minor, fix]) >= 0
end

# OpenSSL 3: x25519 a decode from and then encode to a pem file corrupts the
# key if fips+base provider is used
# This issue happens in OpenSSL between 3.0,0 and 3.0.10 or between 3.1.0 and
# 3.1.2.
# https://github.com/openssl/openssl/issues/21493
# https://github.com/openssl/openssl/pull/21519
def pend_on_openssl_issue_21493
if OpenSSL.fips_mode &&
(
(openssl?(3, 0, 0, 0) && !openssl?(3, 0, 0, 11)) ||
(openssl?(3, 1, 0, 0) && !openssl?(3, 1, 0, 3))
)
pend('See <https://github.com/openssl/openssl/issues/21493>')
end
end
end

class OpenSSL::TestCase < Test::Unit::TestCase
Expand Down