Allow empty passwords in PEM password callback
This aligns with https://github.com/openssl/openssl/pull/6173 from
upstream OpenSSL. As part of this, I had to fix PEM_def_callback (which
is different in us vs BoringSSL) to use -1 as the error value, not 0.
Otherwise errors get misinterpreted as empty strings.
As part of this, make sure all the functions being fixed are covered by
tests.
Fixed: 362788352
Change-Id: I2b5071534c77944d473580fda98d23ae3b54e2d5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70787
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c
index 38c1d3f..6b272cc 100644
--- a/crypto/pem/pem_lib.c
+++ b/crypto/pem/pem_lib.c
@@ -312,12 +312,11 @@
const unsigned iv_len = EVP_CIPHER_iv_length(enc);
if (pass == NULL) {
- pass_len = 0;
if (!callback) {
callback = PEM_def_callback;
}
pass_len = (*callback)(buf, PEM_BUFSIZE, 1, u);
- if (pass_len <= 0) {
+ if (pass_len < 0) {
OPENSSL_PUT_ERROR(PEM, PEM_R_READ_KEY);
goto err;
}
@@ -393,7 +392,7 @@
callback = PEM_def_callback;
}
pass_len = callback(buf, PEM_BUFSIZE, 0, u);
- if (pass_len <= 0) {
+ if (pass_len < 0) {
OPENSSL_PUT_ERROR(PEM, PEM_R_BAD_PASSWORD_READ);
return 0;
}
@@ -779,11 +778,11 @@
int PEM_def_callback(char *buf, int size, int rwflag, void *userdata) {
if (!buf || !userdata || size < 0) {
- return 0;
+ return -1;
}
size_t len = strlen((char *)userdata);
if (len >= (size_t)size) {
- return 0;
+ return -1;
}
OPENSSL_strlcpy(buf, userdata, (size_t)size);
return (int)len;
diff --git a/crypto/pem/pem_pk8.c b/crypto/pem/pem_pk8.c
index 9c6419b..2fc0673 100644
--- a/crypto/pem/pem_pk8.c
+++ b/crypto/pem/pem_pk8.c
@@ -113,12 +113,11 @@
}
if (enc || (nid != -1)) {
if (!pass) {
- pass_len = 0;
if (!cb) {
cb = PEM_def_callback;
}
pass_len = cb(buf, PEM_BUFSIZE, 1, u);
- if (pass_len <= 0) {
+ if (pass_len < 0) {
OPENSSL_PUT_ERROR(PEM, PEM_R_READ_KEY);
PKCS8_PRIV_KEY_INFO_free(p8inf);
return 0;
@@ -166,7 +165,7 @@
cb = PEM_def_callback;
}
pass_len = cb(psbuf, PEM_BUFSIZE, 0, u);
- if (pass_len <= 0) {
+ if (pass_len < 0) {
OPENSSL_PUT_ERROR(PEM, PEM_R_BAD_PASSWORD_READ);
X509_SIG_free(p8);
return NULL;
diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c
index 2fb450c..9349ac7 100644
--- a/crypto/pem/pem_pkey.c
+++ b/crypto/pem/pem_pkey.c
@@ -110,7 +110,7 @@
cb = PEM_def_callback;
}
pass_len = cb(psbuf, PEM_BUFSIZE, 0, u);
- if (pass_len <= 0) {
+ if (pass_len < 0) {
OPENSSL_PUT_ERROR(PEM, PEM_R_BAD_PASSWORD_READ);
X509_SIG_free(p8);
goto err;
diff --git a/crypto/pem/pem_test.cc b/crypto/pem/pem_test.cc
index 117e000..d3c8f14 100644
--- a/crypto/pem/pem_test.cc
+++ b/crypto/pem/pem_test.cc
@@ -14,9 +14,12 @@
#include <openssl/pem.h>
+#include <functional>
+
#include <gtest/gtest.h>
#include <openssl/bio.h>
+#include <openssl/cipher.h>
#include <openssl/err.h>
#include <openssl/rsa.h>
@@ -44,3 +47,269 @@
EXPECT_TRUE(
ErrorEquals(ERR_get_error(), ERR_LIB_PEM, PEM_R_UNSUPPORTED_ENCRYPTION));
}
+
+static std::vector<uint8_t> DecodePEMBytes(const char *pem) {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ char *name, *header;
+ uint8_t *data;
+ long len;
+ if (bio == nullptr ||
+ !PEM_read_bio(bio.get(), &name, &header, &data, &len)) {
+ return {};
+ }
+ bssl::UniquePtr<char> free_name(name), free_header(header);
+ bssl::UniquePtr<uint8_t> free_data(data);
+ return std::vector<uint8_t>(data, data + len);
+}
+
+TEST(PEMTest, DecryptPassword) {
+ // A private key encrypted with the password "password", encrypted at the
+ // PKCS#8 level.
+ static const char kEncryptedPEM[] = R"(
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAjnhMUlb9deeQICCAAw
+HQYJYIZIAWUDBAECBBAO8j5GA5VK8wjvNrzp/iVhBIGQyQKFfFKlFhxiDkFfyhUc
+nPLr0eboQOz8eIaTW1Rblo/qDkQwNtONyfYn909SoIP7iU8UehcBG1UQe41WvQpu
+yRKYQteoWSzFl+yzktL2Y/25K7Uc+f2NScjdonYMZ+9/m1HGmEzKO+Hz28cAsJL7
+rH2gQ0lkxr1GtW77m2rfMKKuGYhpkgjWUbzJwP9v3iq+
+-----END ENCRYPTED PRIVATE KEY-----
+)";
+ // The same key and password, but encrypted at the PEM level.
+ static const char kEncryptedPEM2[] = R"(
+-----BEGIN EC PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-128-CBC,B3B2988AECAE6EAB0D043105994C1123
+
+RK7DUIGDHWTFh2rpTX+dR88hUyC1PyDlIULiNCkuWFwHrJbc1gM6hMVOKmU196XC
+iITrIKmilFm9CPD6Tpfk/NhI/QPxyJlk1geIkxpvUZ2FCeMuYI1To14oYOUKv14q
+wr6JtaX2G+pOmwcSPymZC4u2TncAP7KHgS8UGcMw8CE=
+-----END EC PRIVATE KEY-----
+)";
+
+ for (const char *pem : {kEncryptedPEM, kEncryptedPEM2}) {
+ SCOPED_TRACE(pem);
+ // Decrypt with the correct password.
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(
+ bio.get(), nullptr, nullptr, const_cast<char *>("password")));
+ EXPECT_TRUE(pkey);
+ }
+
+ // Decrypt with the wrong password.
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(
+ bio.get(), nullptr, nullptr, const_cast<char *>("wrong")));
+ EXPECT_FALSE(pkey);
+ EXPECT_TRUE(
+ ErrorEquals(ERR_peek_error(), ERR_LIB_CIPHER, CIPHER_R_BAD_DECRYPT));
+ ERR_clear_error();
+ }
+
+ // If the caller did not pass in a password, we should not proceed to try to
+ // decrypt.
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(
+ PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr));
+ EXPECT_FALSE(pkey);
+ EXPECT_TRUE(
+ ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_BAD_PASSWORD_READ));
+ ERR_clear_error();
+ }
+
+ // If the password, with a NUL terminator, does not fit in the internal
+ // buffer used by the PEM library, the PEM library should notice.
+ {
+ std::string too_long(PEM_BUFSIZE, 'a');
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(
+ bio.get(), nullptr, nullptr, const_cast<char *>(too_long.c_str())));
+ EXPECT_FALSE(pkey);
+ EXPECT_TRUE(
+ ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_BAD_PASSWORD_READ));
+ ERR_clear_error();
+ }
+ }
+
+ // |d2i_PKCS8PrivateKey_bio| should also be able to manage the password
+ // callback correctly.
+ std::vector<uint8_t> bytes = DecodePEMBytes(kEncryptedPEM);
+ ASSERT_FALSE(bytes.empty());
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(bytes.data(), bytes.size()));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(d2i_PKCS8PrivateKey_bio(
+ bio.get(), nullptr, nullptr, const_cast<char *>("password")));
+ EXPECT_TRUE(pkey);
+ }
+
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(bytes.data(), bytes.size()));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(
+ d2i_PKCS8PrivateKey_bio(bio.get(), nullptr, nullptr, nullptr));
+ EXPECT_FALSE(pkey);
+ EXPECT_TRUE(
+ ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_BAD_PASSWORD_READ));
+ ERR_clear_error();
+ }
+
+ {
+ std::string too_long(PEM_BUFSIZE, 'a');
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(bytes.data(), bytes.size()));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(d2i_PKCS8PrivateKey_bio(
+ bio.get(), nullptr, nullptr, const_cast<char *>(too_long.c_str())));
+ EXPECT_FALSE(pkey);
+ EXPECT_TRUE(
+ ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_BAD_PASSWORD_READ));
+ ERR_clear_error();
+ }
+
+ // A private key encrypted with the empty password, encrypted at the PKCS#8
+ // level.
+ static const char kEncryptedPEMEmpty[] = R"(
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIH0MF8GCSqGSIb3DQEFDTBSMDEGCSqGSIb3DQEFDDAkBBAXiHC8iDcjzF0I+D2g
+zJOcAgIIADAMBggqhkiG9w0CCQUAMB0GCWCGSAFlAwQBAgQQwupOMi8DtEWiuXt5
+Odla9QSBkC37uJuG7HSCOyTVCEW76Kmf7GoH+Ou17bDAp6NGwm3KLxRfFoExki9g
+hyLzdarBnhRbPqwMixhaQ2AtkpoSmjristGzZ9U7Y+TM3NnCA4+bu1TckdBn0g+Q
+fvZI9eydS9buA0deGxCUytrMWrR3PxS1yoXBywMDJTom8u5hvvvkJ9WcNzUVRf0D
+6z5NHHiXsQ==
+-----END ENCRYPTED PRIVATE KEY-----
+)";
+ // THe same key and password, but encrypted at the PEM level.
+ static const char kEncryptedPEMEmpty2[] = R"(
+-----BEGIN EC PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-128-CBC,A9505A7DD5C3B51D8AACED18F5758256
+
+yfJKjep7Koj8hU/PtGC+NNXSNbItQ2zyeXDMVoazffraoDGMg6g1hFPPjg9reC+J
+iQQIf9uACF27zi9fpWwbszszimrxl0u6n0ddBXizcK6xzkTvk3PZ67Vz1KYmotwC
+XjgdgSEeixwKhDOuHKFdlFGP/7sw5GHlK3jPSpqi2gI=
+-----END EC PRIVATE KEY-----
+)";
+
+ for (const char *pem : {kEncryptedPEMEmpty, kEncryptedPEMEmpty2}) {
+ SCOPED_TRACE(pem);
+
+ // The empty password should be correctly interpreted as a password.
+ {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(PEM_read_bio_PrivateKey(
+ bio.get(), nullptr, nullptr, const_cast<char *>("")));
+ EXPECT_TRUE(pkey);
+ }
+ }
+
+ // |d2i_PKCS8PrivateKey_bio| should also be able to manage the password
+ // callback correctly.
+ bytes = DecodePEMBytes(kEncryptedPEMEmpty);
+ {
+ ASSERT_FALSE(bytes.empty());
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(bytes.data(), bytes.size()));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(d2i_PKCS8PrivateKey_bio(
+ bio.get(), nullptr, nullptr, const_cast<char *>("")));
+ EXPECT_TRUE(pkey);
+ }
+}
+
+TEST(PEMTest, EncryptPassword) {
+ static const char kKey[] = R"(
+-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgBw8IcnrUoEqc3VnJ
+TYlodwi1b8ldMHcO6NHJzgqLtGqhRANCAATmK2niv2Wfl74vHg2UikzVl2u3qR4N
+Rvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYaHPUdfvGULUvPciLB
+-----END PRIVATE KEY-----
+)";
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(kKey, -1));
+ ASSERT_TRUE(bio);
+ bssl::UniquePtr<EVP_PKEY> pkey(
+ PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr));
+ EXPECT_TRUE(pkey);
+
+ // There are many ways to encrypt a PEM blob with a password.
+ struct PasswordMethod {
+ const char *name;
+ std::function<bool(BIO *, const char *)> func;
+ bool is_callback;
+ };
+ const PasswordMethod kPasswordMethods[] = {
+ {"PKCS#8 encryption, password from param",
+ [&](BIO *out, const char *pass) -> bool {
+ return PEM_write_bio_PrivateKey(
+ out, pkey.get(), EVP_aes_128_cbc(),
+ reinterpret_cast<const unsigned char *>(pass),
+ pass == nullptr ? 0 : strlen(pass), nullptr, nullptr);
+ },
+ /*is_callback=*/false},
+ {"PKCS#8 encryption, password from callback",
+ [&](BIO *out, const char *pass) -> bool {
+ return PEM_write_bio_PrivateKey(out, pkey.get(), EVP_aes_128_cbc(),
+ nullptr, 0, nullptr,
+ const_cast<char *>(pass));
+ },
+ /*is_callback=*/true},
+ {"PEM-level encryption, password from param",
+ [&](BIO *out, const char *pass) -> bool {
+ return PEM_write_bio_ECPrivateKey(
+ out, EVP_PKEY_get0_EC_KEY(pkey.get()), EVP_aes_128_cbc(), nullptr,
+ 0, nullptr, const_cast<char *>(pass));
+ },
+ /*is_callback=*/false},
+ {"PKCS#8 encryption, password from callback",
+ [&](BIO *out, const char *pass) -> bool {
+ return PEM_write_bio_ECPrivateKey(
+ out, EVP_PKEY_get0_EC_KEY(pkey.get()), EVP_aes_128_cbc(), nullptr,
+ 0, nullptr, const_cast<char *>(pass));
+ },
+ /*is_callback=*/true},
+ };
+ for (const auto &p : kPasswordMethods) {
+ SCOPED_TRACE(p.name);
+
+ // Encrypting the private key with a password should work.
+ bio.reset(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+ ASSERT_TRUE(p.func(bio.get(), "password"));
+
+ // Check we can decrypt it.
+ bssl::UniquePtr<EVP_PKEY> pkey2(PEM_read_bio_PrivateKey(
+ bio.get(), nullptr, nullptr, const_cast<char *>("password")));
+ ASSERT_TRUE(pkey2);
+
+ // The empty string is a valid password.
+ bio.reset(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+ ASSERT_TRUE(p.func(bio.get(), ""));
+
+ // Check we can decrypt it.
+ pkey2.reset(PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr,
+ const_cast<char *>("")));
+ ASSERT_TRUE(pkey2);
+
+ // Check error-handling when the password is specified via the callback.
+ if (p.is_callback) {
+ bio.reset(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+ EXPECT_FALSE(p.func(bio.get(), nullptr));
+ EXPECT_TRUE(ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_READ_KEY));
+ ERR_clear_error();
+
+ std::string too_long(PEM_BUFSIZE, 'a');
+ bio.reset(BIO_new(BIO_s_mem()));
+ ASSERT_TRUE(bio);
+ EXPECT_FALSE(p.func(bio.get(), too_long.c_str()));
+ EXPECT_TRUE(ErrorEquals(ERR_peek_error(), ERR_LIB_PEM, PEM_R_READ_KEY));
+ ERR_clear_error();
+ }
+ }
+}
