Skip to content

Commit 2f0f555

Browse files
Support stdin for openssl rsa tool (#2899)
There are users that expect the stdin format when using the `openssl rsa` tool. I also noticed a number of behavioral issues/differences while working on this and comparing it to Openssl's version. OpenSSL prioritizes the PKCS#8 SubjectPublicKeyInfo format first, rather than the raw format. This is particularly problematic with `stdin`, since `stdin` doesn't have a FILE rewind mechanism. This PR aligns AWS-LC behavior with that. ### Call-outs: N/A ### Testing: New RSA comparison test 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. --------- Co-authored-by: Justin Smith <justsmth@amazon.com>
1 parent 04b2db3 commit 2f0f555

File tree

2 files changed

+75
-39
lines changed

2 files changed

+75
-39
lines changed

tool-openssl/rsa.cc

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ static const argument_t kArguments[] = {
2626
{ "-pubout", kBooleanArgument, "Output only public components" },
2727
{ "-noout", kBooleanArgument, "Prevents output of the encoded version of the RSA key" },
2828
{ "-modulus", kBooleanArgument, "Prints out the value of the modulus of the RSA key" },
29+
{ "-RSAPublicKey_in", kOptionalArgument,
30+
"Pass in the RSAPublicKey format. (no-op, we fallback to this"
31+
" format if the initial parse is unsuccessful)" },
2932
{ "", kOptionalArgument, "" }
3033
};
3134

@@ -84,21 +87,22 @@ bool rsaTool(const args_list_t &args) {
8487
return true;
8588
}
8689

87-
if (!HasArgument(parsed_args, "-in")) {
88-
fprintf(stderr, "Missing value for required argument: -in\n");
89-
PrintUsage(kArguments);
90-
return false;
91-
}
92-
9390
if (pubin) {
9491
// If reading a public key, then we can only output a public key.
9592
pubout = true;
9693
}
9794

98-
// Check for required option -in
95+
// Read from stdin if no -in path provided
96+
bssl::UniquePtr<BIO> in_file;
9997
if (in_path.empty()) {
100-
fprintf(stderr, "Error: missing required argument '-in'\n");
101-
return false;
98+
in_file.reset(BIO_new_fp(stdin, BIO_NOCLOSE));
99+
} else {
100+
in_file.reset(BIO_new_file(in_path.c_str(), "rb"));
101+
if (!in_file) {
102+
fprintf(stderr, "Error: unable to load RSA key from '%s'\n",
103+
in_path.c_str());
104+
return false;
105+
}
102106
}
103107

104108
// Check input format
@@ -125,54 +129,52 @@ bool rsaTool(const args_list_t &args) {
125129
}
126130
}
127131

128-
bssl::UniquePtr<BIO> in_file(BIO_new_file(in_path.c_str(), "rb"));
129-
if (!in_file) {
130-
fprintf(stderr, "Error: unable to load RSA key from '%s'\n", in_path.c_str());
131-
return false;
132-
}
133-
134132
bssl::UniquePtr<RSA> rsa;
135133
// Load the key
136134
if (pubin) {
137135
if (input_format == FORMAT_DER || input_format == FORMAT_UNKNOWN) {
138-
// Try raw RSAPublicKey format first
139-
rsa.reset(d2i_RSAPublicKey_bio(in_file.get(), nullptr));
136+
// OpenSSL prioritizes PKCS#8 SubjectPublicKeyInfo format first.
137+
bssl::UniquePtr<EVP_PKEY> pkey(d2i_PUBKEY_bio(in_file.get(), nullptr));
138+
if (pkey) {
139+
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
140+
}
140141
if (!rsa && BIO_seek(in_file.get(), 0) == 0) {
141-
// Try PKCS#8 SubjectPublicKeyInfo format
142-
bssl::UniquePtr<EVP_PKEY> pkey(d2i_PUBKEY_bio(in_file.get(), nullptr));
143-
if (pkey) {
144-
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
145-
}
142+
// Try raw RSAPublicKey format.
143+
// This is the "RSAPublicKey_in" flag in OpenSSL, but we support
144+
// this behind a no-op flag and an automatic fallback.
145+
rsa.reset(d2i_RSAPublicKey_bio(in_file.get(), nullptr));
146146
}
147147
}
148148
if (input_format == FORMAT_PEM || (!rsa && input_format == FORMAT_UNKNOWN)) {
149-
rsa.reset(PEM_read_bio_RSA_PUBKEY(in_file.get(), nullptr, nullptr, nullptr));
149+
bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PUBKEY(in_file.get(), nullptr, nullptr, nullptr));
150+
if (pkey) {
151+
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
152+
}
150153
if (!rsa && BIO_seek(in_file.get(), 0) == 0) {
151-
bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PUBKEY(in_file.get(), nullptr, nullptr, nullptr));
152-
if (pkey) {
153-
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
154-
}
154+
rsa.reset(PEM_read_bio_RSA_PUBKEY(in_file.get(), nullptr, nullptr, nullptr));
155155
}
156156
}
157157
} else {
158158
if (input_format == FORMAT_DER || input_format == FORMAT_UNKNOWN) {
159-
// Try RSAPrivateKey format first
160-
rsa.reset(d2i_RSAPrivateKey_bio(in_file.get(), nullptr));
159+
// OpenSSL prioritizes PKCS#8 PrivateKeyInfo format first.
160+
bssl::UniquePtr<EVP_PKEY> pkey(d2i_PrivateKey_bio(in_file.get(), nullptr));
161+
if (pkey) {
162+
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
163+
}
161164
if (!rsa && BIO_seek(in_file.get(), 0) == 0) {
162-
// Try PKCS#8 PrivateKeyInfo format
163-
bssl::UniquePtr<EVP_PKEY> pkey(d2i_PrivateKey_bio(in_file.get(), nullptr));
164-
if (pkey) {
165-
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
166-
}
165+
// Try RSAPrivateKey format. OpenSSL's |d2i_PrivateKey_bio| automatically
166+
// falls back to PKCS1, if PKCS8 is unsuccessful. We have to do things
167+
// manually here.
168+
rsa.reset(d2i_RSAPrivateKey_bio(in_file.get(), nullptr));
167169
}
168170
}
169171
if (input_format == FORMAT_PEM || (!rsa && input_format == FORMAT_UNKNOWN)) {
170-
rsa.reset(PEM_read_bio_RSAPrivateKey(in_file.get(), nullptr, nullptr, nullptr));
172+
bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(in_file.get(), nullptr, nullptr, nullptr));
173+
if (pkey) {
174+
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
175+
}
171176
if (!rsa && BIO_seek(in_file.get(), 0) == 0) {
172-
bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(in_file.get(), nullptr, nullptr, nullptr));
173-
if (pkey) {
174-
rsa.reset(EVP_PKEY_get1_RSA(pkey.get()));
175-
}
177+
rsa.reset(PEM_read_bio_RSAPrivateKey(in_file.get(), nullptr, nullptr, nullptr));
176178
}
177179
}
178180
}

tool-openssl/rsa_test.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,40 @@ TEST_F(RSAComparisonTest, RSAToolCompareModulusOutOpenSSL) {
481481
ASSERT_TRUE(CheckBoundaries(openssl_output_str, MODULUS, RSA_END, MODULUS, END));
482482
}
483483

484+
// Test against OpenSSL output reading from stdin "openssl rsa -in"
485+
TEST_F(RSAComparisonTest, StdinRSA) {
486+
std::string tool_command = std::string(tool_executable_path) + " rsa -in "
487+
+ std::string(in_path) + " -pubout | " +
488+
std::string(tool_executable_path) +
489+
" rsa -pubin -inform PEM -outform DER > " + out_path_tool;
490+
std::string openssl_command = std::string(openssl_executable_path) + " rsa -in "
491+
+ std::string(in_path) + " -pubout | " +
492+
std::string(openssl_executable_path) +
493+
" rsa -pubin -inform PEM -outform DER > " + out_path_openssl;
494+
495+
RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
496+
out_path_openssl, tool_output_str,
497+
openssl_output_str);
498+
499+
ASSERT_EQ(tool_output_str, openssl_output_str);
500+
501+
tool_command = std::string(tool_executable_path) + " rsa -in "
502+
+ std::string(in_path) + " -pubout -outform DER | " +
503+
std::string(tool_executable_path) +
504+
" rsa -pubin -inform DER -outform PEM > " + out_path_tool;
505+
openssl_command = std::string(openssl_executable_path) + " rsa -in "
506+
+ std::string(in_path) + " -pubout -outform DER | " +
507+
std::string(openssl_executable_path) +
508+
" rsa -pubin -inform DER -outform PEM > " + out_path_openssl;
509+
510+
511+
RunCommandsAndCompareOutput(tool_command, openssl_command, out_path_tool,
512+
out_path_openssl, tool_output_str,
513+
openssl_output_str);
514+
515+
ASSERT_EQ(tool_output_str, openssl_output_str);
516+
}
517+
484518
// Test against OpenSSL output "openssl rsa -in file -modulus -out out_file -noout"
485519
// Only modulus is printed to output file
486520
TEST_F(RSAComparisonTest, RSAToolCompareModulusOutNooutOpenSSL) {

0 commit comments

Comments
 (0)