Skip to content

Commit 3f7f005

Browse files
authored
tool-openssl: pkcs8 error output on decrypt (#2883)
### Description of changes: Align error output of "openssl pkcs8" when handling encrypted keys. ### Testing: Added new test to prevent regression By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 92ff0c5 commit 3f7f005

File tree

2 files changed

+110
-2
lines changed

2 files changed

+110
-2
lines changed

tool-openssl/pkcs8.cc

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,45 @@ static bssl::UniquePtr<EVP_PKEY> read_private_der(BIO *in_bio,
100100
return bssl::UniquePtr<EVP_PKEY>(d2i_PrivateKey_bio(in_bio, nullptr));
101101
}
102102

103+
// Returns 1 if PEM is encrypted, 0 if not, -1 on error
104+
static int is_pem_encrypted(BIO *bio) {
105+
char *name_ptr = nullptr;
106+
char *header_ptr = nullptr;
107+
unsigned char *data_ptr = nullptr;
108+
long len = 0;
109+
110+
// Read the PEM block
111+
if (!PEM_read_bio(bio, &name_ptr, &header_ptr, &data_ptr, &len)) {
112+
return -1; // Error reading PEM
113+
}
114+
115+
// We are responsible for freeing these
116+
bssl::UniquePtr<char> name(name_ptr);
117+
bssl::UniquePtr<char> header(header_ptr);
118+
bssl::UniquePtr<uint8_t> data(data_ptr);
119+
120+
int is_encrypted = 0;
121+
122+
// Check if there's a header with encryption info
123+
if (name && strcmp(name.get(), "ENCRYPTED PRIVATE KEY") == 0) {
124+
is_encrypted = 1;
125+
}
126+
// Check for traditional PEM encryption (by header)
127+
else if (header && header.get()[0] != '\0') {
128+
EVP_CIPHER_INFO cipher;
129+
if (PEM_get_EVP_CIPHER_INFO(header.get(), &cipher)) {
130+
is_encrypted = (cipher.cipher != nullptr) ? 1 : 0;
131+
}
132+
}
133+
134+
// Rewind buffer so it can be parsed to obtain a private key
135+
if (BIO_seek(bio, 0) >= 0) {
136+
return is_encrypted;
137+
}
138+
139+
return -1;
140+
}
141+
103142
static const argument_t kArguments[] = {
104143
{"-help", kBooleanArgument, "Display option summary"},
105144
{"-in", kOptionalArgument, "Input file"},
@@ -133,7 +172,7 @@ bool pkcs8Tool(const args_list_t &args) {
133172
bssl::UniquePtr<EVP_PKEY> pkey;
134173
const EVP_CIPHER *cipher = nullptr;
135174
bssl::UniquePtr<PKCS8_PRIV_KEY_INFO> p8inf;
136-
175+
bool input_is_encrypted = false;
137176

138177
if (!ParseOrderedKeyValueArguments(parsed_args, extra_args, args,
139178
kArguments)) {
@@ -193,6 +232,18 @@ bool pkcs8Tool(const args_list_t &args) {
193232
if (!in) {
194233
fprintf(stderr, "Cannot open input file\n");
195234
return false;
235+
} else if (inform == "PEM") {
236+
switch(is_pem_encrypted(in.get())) {
237+
case 0:
238+
input_is_encrypted = false;
239+
break;
240+
case 1:
241+
input_is_encrypted = true;
242+
break;
243+
default:
244+
fprintf(stderr, "Unable to load PEM file\n");
245+
return false;
246+
}
196247
}
197248
if (!validate_bio_size(in.get())) {
198249
return false;
@@ -218,7 +269,11 @@ bool pkcs8Tool(const args_list_t &args) {
218269
in.get(), passin_arg->empty() ? nullptr : passin_arg->c_str())
219270
.release());
220271
if (!pkey) {
221-
fprintf(stderr, "Unable to load private key\n");
272+
if (input_is_encrypted) {
273+
fprintf(stderr, "Error decrypting key\n");
274+
} else {
275+
fprintf(stderr, "Unable to load private key\n");
276+
}
222277
return false;
223278
}
224279

tool-openssl/pkcs8_test.cc

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,59 @@ TEST_F(PKCS8Test, PKCS8ToolEncryptionTest) {
116116
ASSERT_TRUE(result);
117117
}
118118

119+
// Verify failure output contains "Error decrypting key"
120+
TEST_F(PKCS8Test, PKCS8ErrorDecryptingKey) {
121+
{
122+
const char* passwd = "test1234";
123+
bssl::UniquePtr<BIO> pass_bio(BIO_new_file(pass_path, "wb"));
124+
BIO_write(pass_bio.get(), passwd, strlen(passwd));
125+
BIO_flush(pass_bio.get());
126+
}
127+
128+
std::string passfile = std::string("file:") + pass_path;
129+
130+
// Phase 1: Encrypt the key
131+
args_list_t args_encrypt = {
132+
"-passin", "pass:''",
133+
"-inform", "PEM",
134+
"-in", in_path,
135+
"-topk8",
136+
"-v2", "aes-256-cbc",
137+
"-passout", passfile.c_str(),
138+
"-outform", "PEM",
139+
"-out", out_path
140+
};
141+
142+
ASSERT_TRUE(pkcs8Tool(args_encrypt));
143+
144+
// Phase 2: Try to decrypt with wrong password (should fail)
145+
args_list_t args_verify = {
146+
"-passin", "pass:''",
147+
"-inform", "PEM",
148+
"-in", out_path,
149+
"-outform", "PEM",
150+
};
151+
152+
// Capture stderr to verify the error message
153+
testing::internal::CaptureStderr();
154+
bool verify_result = pkcs8Tool(args_verify);
155+
std::string captured_stderr = testing::internal::GetCapturedStderr();
156+
157+
ASSERT_FALSE(verify_result) << "Expected decryption to fail with wrong password";
158+
EXPECT_TRUE(captured_stderr.find("Error decrypting key") != std::string::npos)
159+
<< "Expected 'Error decrypting key' in stderr, but got: " << captured_stderr;
160+
161+
// Phase 3: Decrypt with correct password (should succeed)
162+
args_list_t args_decrypt = {
163+
"-passin", passfile.c_str(),
164+
"-inform", "PEM",
165+
"-in", out_path,
166+
"-outform", "PEM",
167+
};
168+
169+
ASSERT_TRUE(pkcs8Tool(args_decrypt));
170+
}
171+
119172
// Test with a direct password rather than using environment variables
120173
TEST_F(PKCS8Test, PKCS8ToolEnvVarPasswordTest) {
121174
// Phase 1: Create an unencrypted PKCS8 file first

0 commit comments

Comments
 (0)