Skip to content

Commit 402cd56

Browse files
rheniumrhenium
authored
Merge pull request #931 from rhenium/ky/pkey-read-fix-repeated-prompts
pkey: fix repeated passphrase prompts in OpenSSL::PKey.read
2 parents d377a34 + 985ba27 commit 402cd56

File tree

2 files changed

+142
-15
lines changed

2 files changed

+142
-15
lines changed

‎ext/openssl/ossl_pkey.c‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -83,41 +83,48 @@ ossl_pkey_wrap(EVP_PKEY *pkey)
8383
# include <openssl/decoder.h>
8484

8585
static EVP_PKEY *
86-
ossl_pkey_read(BIO *bio, const char *input_type, int selection, VALUE pass)
86+
ossl_pkey_read(BIO *bio, const char *input_type, int selection, VALUE pass,
87+
int *retryable)
8788
{
8889
void *ppass = (void *)pass;
8990
OSSL_DECODER_CTX *dctx;
9091
EVP_PKEY *pkey = NULL;
9192
int pos = 0, pos2;
9293

94+
*retryable = 0;
9395
dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, input_type, NULL, NULL,
9496
selection, NULL, NULL);
9597
if (!dctx)
9698
goto out;
97-
if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb,
99+
if (selection == EVP_PKEY_KEYPAIR &&
100+
OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb,
98101
ppass) != 1)
99102
goto out;
100103
while (1) {
101104
if (OSSL_DECODER_from_bio(dctx, bio) == 1)
102-
goto out;
103-
if (BIO_eof(bio))
104105
break;
106+
if (ERR_GET_REASON(ERR_peek_error()) != ERR_R_UNSUPPORTED)
107+
break;
108+
if (BIO_eof(bio) == 1) {
109+
*retryable = 1;
110+
break;
111+
}
105112
pos2 = BIO_tell(bio);
106-
if (pos2 < 0 || pos2 <= pos)
113+
if (pos2 < 0 || pos2 <= pos) {
114+
*retryable = 1;
107115
break;
116+
}
108117
ossl_clear_error();
109118
pos = pos2;
110119
}
111120
out:
112-
OSSL_BIO_reset(bio);
113121
OSSL_DECODER_CTX_free(dctx);
114122
return pkey;
115123
}
116124

117125
EVP_PKEY *
118126
ossl_pkey_read_generic(BIO *bio, VALUE pass)
119127
{
120-
EVP_PKEY *pkey = NULL;
121128
/* First check DER, then check PEM. */
122129
const char *input_types[] = {"DER", "PEM"};
123130
int input_type_num = (int)(sizeof(input_types) / sizeof(char *));
@@ -166,18 +173,22 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass)
166173
EVP_PKEY_PUBLIC_KEY
167174
};
168175
int selection_num = (int)(sizeof(selections) / sizeof(int));
169-
int i, j;
170176

171-
for (i = 0; i < input_type_num; i++) {
172-
for (j = 0; j < selection_num; j++) {
173-
pkey = ossl_pkey_read(bio, input_types[i], selections[j], pass);
174-
if (pkey) {
175-
goto out;
177+
for (int i = 0; i < input_type_num; i++) {
178+
for (int j = 0; j < selection_num; j++) {
179+
if (i || j) {
180+
ossl_clear_error();
181+
BIO_reset(bio);
176182
}
183+
184+
int retryable;
185+
EVP_PKEY *pkey = ossl_pkey_read(bio, input_types[i], selections[j],
186+
pass, &retryable);
187+
if (pkey || !retryable)
188+
return pkey;
177189
}
178190
}
179-
out:
180-
return pkey;
191+
return NULL;
181192
}
182193
#else
183194
EVP_PKEY *

‎test/openssl/test_pkey.rb‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,122 @@ def test_s_generate_key
6060
assert_not_equal nil, pkey.private_key
6161
end
6262

63+
def test_s_read_pem_unknown_block
64+
# A PEM-encoded certificate and a PEM-encoded private key are combined.
65+
# Check that OSSL_STORE doesn't stop after the first PEM block.
66+
orig = Fixtures.pkey("rsa-1")
67+
subject = OpenSSL::X509::Name.new([["CN", "test"]])
68+
cert = issue_cert(subject, orig, 1, [], nil, nil)
69+
70+
input = cert.to_text + cert.to_pem + orig.to_text + orig.private_to_pem
71+
pkey = OpenSSL::PKey.read(input)
72+
assert_equal(orig.private_to_der, pkey.private_to_der)
73+
end
74+
75+
def test_s_read_der_then_pem
76+
# If the input is valid as both DER and PEM (which allows garbage data
77+
# before and after the block), it is read as DER
78+
#
79+
# TODO: Garbage data after DER should not be allowed, but it is currently
80+
# ignored
81+
orig1 = Fixtures.pkey("rsa-1")
82+
orig2 = Fixtures.pkey("rsa-2")
83+
pkey = OpenSSL::PKey.read(orig1.public_to_der + orig2.private_to_pem)
84+
assert_equal(orig1.public_to_der, pkey.public_to_der)
85+
assert_not_predicate(pkey, :private?)
86+
end
87+
88+
def test_s_read_passphrase
89+
orig = Fixtures.pkey("rsa-1")
90+
encrypted_pem = orig.private_to_pem("AES-256-CBC", "correct_passphrase")
91+
assert_match(/\A-----BEGIN ENCRYPTED PRIVATE KEY-----/, encrypted_pem)
92+
93+
# Correct passphrase passed as the second argument
94+
pkey1 = OpenSSL::PKey.read(encrypted_pem, "correct_passphrase")
95+
assert_equal(orig.private_to_der, pkey1.private_to_der)
96+
97+
# Correct passphrase returned by the block. The block gets false
98+
called = 0
99+
flag = nil
100+
pkey2 = OpenSSL::PKey.read(encrypted_pem) { |f|
101+
called += 1
102+
flag = f
103+
"correct_passphrase"
104+
}
105+
assert_equal(orig.private_to_der, pkey2.private_to_der)
106+
assert_equal(1, called)
107+
assert_false(flag)
108+
109+
# Incorrect passphrase passed. The block is not called
110+
called = 0
111+
assert_raise(OpenSSL::PKey::PKeyError) {
112+
OpenSSL::PKey.read(encrypted_pem, "incorrect_passphrase") {
113+
called += 1
114+
}
115+
}
116+
assert_equal(0, called)
117+
118+
# Incorrect passphrase returned by the block. The block is called only once
119+
called = 0
120+
assert_raise(OpenSSL::PKey::PKeyError) {
121+
OpenSSL::PKey.read(encrypted_pem) {
122+
called += 1
123+
"incorrect_passphrase"
124+
}
125+
}
126+
assert_equal(1, called)
127+
128+
# Incorrect passphrase returned by the block. The input contains two PEM
129+
# blocks.
130+
called = 0
131+
assert_raise(OpenSSL::PKey::PKeyError) {
132+
OpenSSL::PKey.read(encrypted_pem + encrypted_pem) {
133+
called += 1
134+
"incorrect_passphrase"
135+
}
136+
}
137+
assert_equal(1, called)
138+
end
139+
140+
def test_s_read_passphrase_tty
141+
omit "https://github.com/aws/aws-lc/pull/2555" if aws_lc?
142+
143+
orig = Fixtures.pkey("rsa-1")
144+
encrypted_pem = orig.private_to_pem("AES-256-CBC", "correct_passphrase")
145+
146+
# Correct passphrase passed to OpenSSL's prompt
147+
script = <<~"end;"
148+
require "openssl"
149+
Process.setsid
150+
OpenSSL::PKey.read(#{encrypted_pem.dump})
151+
puts "ok"
152+
end;
153+
assert_in_out_err([*$:.map { |l| "-I#{l}" }, "-e#{script}"],
154+
"correct_passphrase\n") { |stdout, stderr|
155+
assert_equal(["Enter PEM pass phrase:"], stderr)
156+
assert_equal(["ok"], stdout)
157+
}
158+
159+
# Incorrect passphrase passed to OpenSSL's prompt
160+
script = <<~"end;"
161+
require "openssl"
162+
Process.setsid
163+
begin
164+
OpenSSL::PKey.read(#{encrypted_pem.dump})
165+
rescue OpenSSL::PKey::PKeyError
166+
puts "ok"
167+
else
168+
puts "expected OpenSSL::PKey::PKeyError"
169+
end
170+
end;
171+
stdin = "incorrect_passphrase\n" * 5
172+
assert_in_out_err([*$:.map { |l| "-I#{l}" }, "-e#{script}"],
173+
stdin) { |stdout, stderr|
174+
assert_equal(1, stderr.count("Enter PEM pass phrase:"))
175+
assert_equal(["ok"], stdout)
176+
}
177+
end if ENV["OSSL_TEST_ALL"] == "1" && Process.respond_to?(:setsid)
178+
63179
def test_hmac_sign_verify
64180
pkey = OpenSSL::PKey.generate_key("HMAC", { "key" => "abcd" })
65181

0 commit comments

Comments
 (0)