Skip to content

Commit

Permalink
add support for PEM_write_bio_PrivateKey_traditional (#1845)
Browse files Browse the repository at this point in the history
The main difference between `PEM_write_bio_PrivateKey_traditional` and
the modern `PEM_write_bio_PrivateKey` is that `PEM_write_bio_PrivateKey`
writes a PKCS#8 encoding, while the traditional one writes with DER
encoding.
OpenSSL's `PEM_write_bio_PrivateKey_traditional` calls into
`i2d_PrivateKey` which calls into another additional `old_priv_encode`
EVP_PKEY_ASN1_METHOD hook. We also need to make the call into
`i2d_PrivateKey`, but luckily for us upstream removed the hook and let
the call point to the corresponding `i2d_RSA/DSA/ECPrivateKey` methods
instead. We can built on this directly to reimplement
`PEM_write_bio_PrivateKey_traditional`.

`PEM_read_bio_PrivateKey` already has logic to parse the "traditional
form" of these PEM files. I've also added tests to verify.

### Testing:
Tests for each type writable/readable. A single test for DH, which
shouldn't be writable.

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.
  • Loading branch information
samuel40791765 authored Sep 18, 2024
1 parent efa9d6c commit 265e3aa
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 1 deletion.
19 changes: 19 additions & 0 deletions crypto/pem/pem_pkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,25 @@ int PEM_write_bio_Parameters(BIO *bio, EVP_PKEY *pkey) {
}
}

static int i2d_PrivateKey_void(const void *key, uint8_t **out) {
return i2d_PrivateKey((const EVP_PKEY *)key, out);
}

int PEM_write_bio_PrivateKey_traditional(BIO *bp, EVP_PKEY *x,
const EVP_CIPHER *enc,
unsigned char *kstr, int klen,
pem_password_cb *cb, void *u) {
if (bp == NULL || x == NULL || x->ameth == NULL ||
x->ameth->pem_str == NULL) {
OPENSSL_PUT_ERROR(PEM, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
char pem_str[80];
BIO_snprintf(pem_str, sizeof(pem_str), "%s PRIVATE KEY", x->ameth->pem_str);
return PEM_ASN1_write_bio(i2d_PrivateKey_void, pem_str, bp, x, enc, kstr,
klen, cb, u);
}

EVP_PKEY *PEM_read_PrivateKey(FILE *fp, EVP_PKEY **x, pem_password_cb *cb,
void *u) {
BIO *b = BIO_new_fp(fp, BIO_NOCLOSE);
Expand Down
102 changes: 101 additions & 1 deletion crypto/pem/pem_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
#include <openssl/cipher.h>
#include <openssl/err.h>
#include <openssl/evp.h>
#include <openssl/rand.h>
#include <openssl/rsa.h>


#include "../test/test_util.h"
#include "openssl/rand.h"

const char* SECRET = "test";

Expand Down Expand Up @@ -366,3 +366,103 @@ TEST(ParametersTest, RubyDHFile) {
EXPECT_TRUE(dh);
EXPECT_EQ(DH_num_bits(dh.get()), 2048u);
}

TEST(PEMTest, WriteReadTraditionalPem) {
// Test |PEM_write_bio_PrivateKey_traditional| with |EC_KEY|.
bssl::UniquePtr<EC_KEY> ec_key(EC_KEY_new());
ASSERT_TRUE(ec_key);
bssl::UniquePtr<EC_GROUP> ec_group(EC_GROUP_new_by_curve_name(NID_secp256k1));
ASSERT_TRUE(ec_group);
ASSERT_TRUE(EC_KEY_set_group(ec_key.get(), ec_group.get()));
ASSERT_TRUE(EC_KEY_generate_key(ec_key.get()));

bssl::UniquePtr<BIO> write_bio(BIO_new(BIO_s_mem()));
bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
ASSERT_TRUE(EVP_PKEY_set1_EC_KEY(pkey.get(), ec_key.get()));
EXPECT_TRUE(PEM_write_bio_PrivateKey_traditional(
write_bio.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));

const uint8_t *content;
size_t content_len;
BIO_mem_contents(write_bio.get(), &content, &content_len);

bssl::UniquePtr<BIO> read_bio(BIO_new_mem_buf(content, content_len));
ASSERT_TRUE(read_bio);
bssl::UniquePtr<EVP_PKEY> pkey_read(
PEM_read_bio_PrivateKey(read_bio.get(), nullptr, nullptr, nullptr));
ASSERT_TRUE(pkey_read);

EC_KEY *pkey_eckey = EVP_PKEY_get0_EC_KEY(pkey.get());
const BIGNUM *orig_priv_key = EC_KEY_get0_private_key(ec_key.get());
const BIGNUM *read_priv_key = EC_KEY_get0_private_key(pkey_eckey);
ASSERT_EQ(0, BN_cmp(orig_priv_key, read_priv_key));

// Test |PEM_write_bio_PrivateKey_traditional| with |RSA|.
bssl::UniquePtr<BIGNUM> e(BN_new());
ASSERT_TRUE(e);
ASSERT_TRUE(BN_set_word(e.get(), RSA_F4));
bssl::UniquePtr<RSA> rsa(RSA_new());
ASSERT_TRUE(rsa);
ASSERT_TRUE(RSA_generate_key_ex(rsa.get(), 1024, e.get(), nullptr));

write_bio.reset(BIO_new(BIO_s_mem()));
pkey.reset(EVP_PKEY_new());
ASSERT_TRUE(EVP_PKEY_set1_RSA(pkey.get(), rsa.get()));
EXPECT_TRUE(PEM_write_bio_PrivateKey_traditional(
write_bio.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));

BIO_mem_contents(write_bio.get(), &content, &content_len);
read_bio.reset(BIO_new_mem_buf(content, content_len));
ASSERT_TRUE(read_bio);
pkey_read.reset(
PEM_read_bio_PrivateKey(read_bio.get(), nullptr, nullptr, nullptr));
ASSERT_TRUE(pkey_read);

RSA *pkey_rsa = EVP_PKEY_get0_RSA(pkey.get());
EXPECT_EQ(0, BN_cmp(RSA_get0_d(pkey_rsa), RSA_get0_d(rsa.get())));
EXPECT_EQ(0, BN_cmp(RSA_get0_d(pkey_rsa), RSA_get0_d(rsa.get())));

// Test |PEM_write_bio_PrivateKey_traditional| with |DSA|.
bssl::UniquePtr<DSA> dsa(DSA_new());
ASSERT_TRUE(dsa);
uint8_t seed[20];
ASSERT_TRUE(RAND_bytes(seed, sizeof(seed)));
ASSERT_TRUE(DSA_generate_parameters_ex(dsa.get(), 512, seed, sizeof(seed),
nullptr, nullptr, nullptr));
ASSERT_TRUE(DSA_generate_key(dsa.get()));

write_bio.reset(BIO_new(BIO_s_mem()));
pkey.reset(EVP_PKEY_new());
ASSERT_TRUE(EVP_PKEY_set1_DSA(pkey.get(), dsa.get()));
EXPECT_TRUE(PEM_write_bio_PrivateKey_traditional(
write_bio.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));

BIO_mem_contents(write_bio.get(), &content, &content_len);
read_bio.reset(BIO_new_mem_buf(content, content_len));
ASSERT_TRUE(read_bio);
pkey_read.reset(
PEM_read_bio_PrivateKey(read_bio.get(), nullptr, nullptr, nullptr));
ASSERT_TRUE(pkey_read);

DSA *pkey_dsa = EVP_PKEY_get0_DSA(pkey.get());
EXPECT_EQ(0,
BN_cmp(DSA_get0_priv_key(pkey_dsa), DSA_get0_priv_key(dsa.get())));
EXPECT_EQ(0,
BN_cmp(DSA_get0_priv_key(pkey_dsa), DSA_get0_priv_key(dsa.get())));

// Test |PEM_write_bio_PrivateKey_traditional| with |DH|. This should fail,
// since it's not supported by the API.
bssl::UniquePtr<BIGNUM> p(BN_get_rfc3526_prime_1536(nullptr));
ASSERT_TRUE(p);
bssl::UniquePtr<BIGNUM> g(BN_new());
ASSERT_TRUE(g);
ASSERT_TRUE(BN_set_u64(g.get(), 2));
bssl::UniquePtr<DH> dh(DH_new());
ASSERT_TRUE(dh);
ASSERT_TRUE(DH_set0_pqg(dh.get(), p.release(), nullptr, g.release()));
write_bio.reset(BIO_new(BIO_s_mem()));
pkey.reset(EVP_PKEY_new());
ASSERT_TRUE(EVP_PKEY_set1_DH(pkey.get(), dh.get()));
EXPECT_FALSE(PEM_write_bio_PrivateKey_traditional(
write_bio.get(), pkey.get(), nullptr, nullptr, 0, nullptr, nullptr));
}
7 changes: 7 additions & 0 deletions include/openssl/pem.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,13 @@ OPENSSL_EXPORT EC_GROUP *PEM_read_bio_ECPKParameters(BIO *bio,
OPENSSL_EXPORT int PEM_write_bio_ECPKParameters(BIO *out,
const EC_GROUP *group);

// PEM_write_bio_PrivateKey_traditional calls |PEM_ASN1_write_bio| to write
// out |x|'s private key in the "traditional" ASN1 format. Use
// |PEM_write_bio_PrivateKey| instead.
OPENSSL_EXPORT int PEM_write_bio_PrivateKey_traditional(
BIO *bp, EVP_PKEY *x, const EVP_CIPHER *enc, unsigned char *kstr, int klen,
pem_password_cb *cb, void *u);

#ifdef __cplusplus
} // extern "C"
#endif
Expand Down

0 comments on commit 265e3aa

Please sign in to comment.