Add support for PEM Parameters without ASN1 hooks (#1831)

This reverts part of  commit 32fdc51.

commit 32fdc51 removed the function
pointers for `param_decode` and `param_encode` and removed
`PEM_read_bio_Parameters` and `PEM_write_bio_Parameters` along with it
(which was the only API consuming these hooks). This reverts part of the
commit to add these back.

I ultimately decided to leave the ASN1 logic in
`PEM_read/write_bio_Parameters` which allows us to continue decoupling
the pem logic from `EVP_PKEY`. These correspond to the historical
`param_decode` and `param_encode` `EVP_PKEY_ASN1_METHOD` hooks in
OpenSSL.


### Testing:
1. Round trip serde tests for each possible `EVP_PKEY` type
2. Sample test DH Parameter file from Ruby 3.1.

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.

crypto/pem/pem_lib.c

@@ -71,6 +71,7 @@
 #include <openssl/x509.h>
 
 #include "../internal.h"
+#include "../fipsmodule/evp/internal.h"
 
 
 #define MIN_LENGTH 4
@@ -146,6 +147,13 @@ static int check_pem(const char *nm, const char *name) {
            !strcmp(nm, PEM_STRING_DSA);
   }
 
+  // These correspond with the PEM strings that have "PARAMETERS".
+  if (!strcmp(name, PEM_STRING_PARAMETERS)) {
+    return !strcmp(nm, PEM_STRING_ECPARAMETERS) ||
+           !strcmp(nm, PEM_STRING_DSAPARAMS) ||
+           !strcmp(nm, PEM_STRING_DHPARAMS);
+  }
+
   // Permit older strings
 
   if (!strcmp(nm, PEM_STRING_X509_OLD) && !strcmp(name, PEM_STRING_X509)) {

crypto/pem/pem_pkey.c

@@ -67,6 +67,7 @@
 #include <openssl/pkcs8.h>
 #include <openssl/rand.h>
 #include <openssl/x509.h>
+#include "../fipsmodule/evp/internal.h"
 
 EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp, EVP_PKEY **x, pem_password_cb *cb,
                                   void *u) {
@@ -156,6 +157,109 @@ int PEM_write_bio_PrivateKey(BIO *bp, EVP_PKEY *x, const EVP_CIPHER *enc,
   return PEM_write_bio_PKCS8PrivateKey(bp, x, enc, (char *)kstr, klen, cb, u);
 }
 
+EVP_PKEY *PEM_read_bio_Parameters(BIO *bio, EVP_PKEY **pkey) {
+  if (bio == NULL) {
+    OPENSSL_PUT_ERROR(PEM, ERR_R_PASSED_NULL_PARAMETER);
+    return NULL;
+  }
+
+  char *nm = NULL;
+  unsigned char *data = NULL;
+  long len;
+  if (!PEM_bytes_read_bio(&data, &len, &nm, PEM_STRING_PARAMETERS, bio, 0,
+                          NULL)) {
+    return NULL;
+  }
+  const unsigned char *data_const = data;
+
+  // Implementing the ASN1 logic here allows us to decouple the pem logic for
+  // |EVP_PKEY|. These correspond to the historical |param_decode|
+  // |EVP_PKEY_ASN1_METHOD| hooks in OpenSSL.
+  EVP_PKEY *ret = EVP_PKEY_new();
+  if (ret == NULL) {
+    goto err;
+  }
+  if (strcmp(nm, PEM_STRING_ECPARAMETERS) == 0) {
+    EC_KEY *ec_key = d2i_ECParameters(NULL, &data_const, len);
+    if (ec_key == NULL || !EVP_PKEY_assign_EC_KEY(ret, ec_key)) {
+      OPENSSL_PUT_ERROR(EVP, ERR_R_EC_LIB);
+      EC_KEY_free(ec_key);
+      goto err;
+    }
+  } else if (strcmp(nm, PEM_STRING_DSAPARAMS) == 0) {
+    DSA *dsa = d2i_DSAparams(NULL, &data_const, len);
+    if (dsa == NULL || !EVP_PKEY_assign_DSA(ret, dsa)) {
+      OPENSSL_PUT_ERROR(EVP, ERR_R_DSA_LIB);
+      DSA_free(dsa);
+      goto err;
+    }
+  } else if (strcmp(nm, PEM_STRING_DHPARAMS) == 0) {
+    DH *dh = d2i_DHparams(NULL, &data_const, len);
+    if (dh == NULL || !EVP_PKEY_assign_DH(ret, dh)) {
+      OPENSSL_PUT_ERROR(EVP, ERR_R_DH_LIB);
+      DH_free(dh);
+      goto err;
+    }
+  } else {
+    goto err;
+  }
+
+  if (pkey != NULL) {
+    EVP_PKEY_free(*pkey);
+    *pkey = ret;
+  }
+
+  OPENSSL_free(nm);
+  OPENSSL_free(data);
+  return ret;
+
+err:
+  EVP_PKEY_free(ret);
+  OPENSSL_free(nm);
+  OPENSSL_free(data);
+  return NULL;
+}
+
+static int i2d_ECParameters_void(const void *key, uint8_t **out) {
+  return i2d_ECParameters((EC_KEY *)key, out);
+}
+
+static int i2d_DSAparams_void(const void *key, uint8_t **out) {
+  return i2d_DSAparams((DSA *)key, out);
+}
+
+static int i2d_DHparams_void(const void *key, uint8_t **out) {
+  return i2d_DHparams((DH *)key, out);
+}
+
+int PEM_write_bio_Parameters(BIO *bio, EVP_PKEY *pkey) {
+  if (bio == NULL || pkey == NULL) {
+    OPENSSL_PUT_ERROR(PEM, ERR_R_PASSED_NULL_PARAMETER);
+    return 0;
+  }
+
+  // Implementing the ASN1 logic here allows us to decouple the pem logic for
+  // |EVP_PKEY|. These correspond to the historical |param_encode|
+  // |EVP_PKEY_ASN1_METHOD| hooks in OpenSSL.
+  char pem_str[80];
+  switch (pkey->type) {
+    case EVP_PKEY_EC:
+      BIO_snprintf(pem_str, 80, PEM_STRING_ECPARAMETERS);
+      return PEM_ASN1_write_bio(i2d_ECParameters_void, pem_str, bio,
+                                pkey->pkey.ec, NULL, NULL, 0, 0, NULL);
+    case EVP_PKEY_DSA:
+      BIO_snprintf(pem_str, 80, PEM_STRING_DSAPARAMS);
+      return PEM_ASN1_write_bio(i2d_DSAparams_void, pem_str, bio,
+                                pkey->pkey.dsa, NULL, NULL, 0, 0, NULL);
+    case EVP_PKEY_DH:
+      BIO_snprintf(pem_str, 80, PEM_STRING_DHPARAMS);
+      return PEM_ASN1_write_bio(i2d_DHparams_void, pem_str, bio, pkey->pkey.dh,
+                                NULL, NULL, 0, 0, NULL);
+    default:
+      return 0;
+  }
+}
+
 EVP_PKEY *PEM_read_PrivateKey(FILE *fp, EVP_PKEY **x, pem_password_cb *cb,
                               void *u) {
   BIO *b = BIO_new_fp(fp, BIO_NOCLOSE);

crypto/pem/pem_test.cc

@@ -25,6 +25,7 @@
 
 
 #include "../test/test_util.h"
+#include "openssl/rand.h"
 
 const char* SECRET = "test";
 
@@ -265,3 +266,103 @@ TEST(PEMTest, WriteReadECPKPem) {
   ASSERT_TRUE(read_group);
   ASSERT_EQ(EC_GROUP_cmp(EC_group_p256(), read_group.get(), nullptr), 0);
 }
+
+TEST(ParametersTest, PEMReadwrite) {
+  // Test |PEM_read/write_bio_Parameters| 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_secp384r1));
+  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_Parameters(write_bio.get(), pkey.get()));
+
+  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_Parameters(read_bio.get(), nullptr));
+  ASSERT_TRUE(pkey_read);
+
+  EC_KEY *pkey_eckey = EVP_PKEY_get0_EC_KEY(pkey.get());
+  const EC_GROUP *orig_params = EC_KEY_get0_group(pkey_eckey);
+  const EC_GROUP *read_params = EC_KEY_get0_group(pkey_eckey);
+  ASSERT_EQ(0, EC_GROUP_cmp(orig_params, read_params, nullptr));
+
+  // Test |PEM_read/write_bio_Parameters| with |DH|.
+  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_TRUE(PEM_write_bio_Parameters(write_bio.get(), pkey.get()));
+
+  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_Parameters(read_bio.get(), nullptr));
+  ASSERT_TRUE(pkey_read);
+
+  DH *pkey_dh = EVP_PKEY_get0_DH(pkey.get());
+  EXPECT_EQ(0, BN_cmp(DH_get0_p(pkey_dh), DH_get0_p(dh.get())));
+  EXPECT_EQ(0, BN_cmp(DH_get0_g(pkey_dh), DH_get0_g(dh.get())));
+
+  // Test |PEM_read/write_bio_Parameters| 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_Parameters(write_bio.get(), pkey.get()));
+
+  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_Parameters(read_bio.get(), nullptr));
+  ASSERT_TRUE(pkey_read);
+
+  DSA *pkey_dsa = EVP_PKEY_get0_DSA(pkey.get());
+  EXPECT_EQ(0, BN_cmp(DSA_get0_p(pkey_dsa), DSA_get0_p(dsa.get())));
+  EXPECT_EQ(0, BN_cmp(DSA_get0_g(pkey_dsa), DSA_get0_g(dsa.get())));
+}
+
+const char *kRubyPemDHPARAMETERS =
+    "-----BEGIN DH PARAMETERS-----\n"
+    "MIIBCAKCAQEA7E6kBrYiyvmKAMzQ7i8WvwVk9Y/+f8S7sCTN712KkK3cqd1jhJDY"
+    "JbrYeNV3kUIKhPxWHhObHKpD1R84UpL+s2b55+iMd6GmL7OYmNIT/FccKhTcveab"
+    "VBmZT86BZKYyf45hUF9FOuUM9xPzuK3Vd8oJQvfYMCd7LPC0taAEljQLR4Edf8E6"
+    "YoaOffgTf5qxiwkjnlVZQc3whgnEt9FpVMvQ9eknyeGB5KHfayAc3+hUAvI3/Cr3"
+    "1bNveX5wInh5GDx1FGhKBZ+s1H+aedudCm7sCgRwv8lKWYGiHzObSma8A86KG+MD"
+    "7Lo5JquQ3DlBodj3IDyPrxIv96lvRPFtAwIBAg==\n"
+    "-----END DH PARAMETERS-----\n";
+
+TEST(ParametersTest, RubyDHFile) {
+  bssl::UniquePtr<BIO> read_bio(
+      BIO_new_mem_buf(kRubyPemDHPARAMETERS, strlen(kRubyPemDHPARAMETERS)));
+  ASSERT_TRUE(read_bio);
+  bssl::UniquePtr<EVP_PKEY> pkey_read(
+      PEM_read_bio_Parameters(read_bio.get(), nullptr));
+  ASSERT_TRUE(pkey_read);
+  bssl::UniquePtr<DH> dh(EVP_PKEY_get1_DH(pkey_read.get()));
+  EXPECT_TRUE(dh);
+  EXPECT_EQ(DH_num_bits(dh.get()), 2048u);
+}

include/openssl/pem.h

@@ -107,6 +107,7 @@ extern "C" {
 #define PEM_STRING_ECDSA_PUBLIC "ECDSA PUBLIC KEY"
 #define PEM_STRING_ECPARAMETERS "EC PARAMETERS"
 #define PEM_STRING_ECPRIVATEKEY "EC PRIVATE KEY"
+#define PEM_STRING_PARAMETERS "PARAMETERS"
 #define PEM_STRING_CMS "CMS"
 
 // enc_type is one off
@@ -343,9 +344,22 @@ OPENSSL_EXPORT int PEM_read_bio(BIO *bp, char **name, char **header,
 OPENSSL_EXPORT int PEM_write_bio(BIO *bp, const char *name, const char *hdr,
                                  const unsigned char *data, long len);
 
+// PEM_bytes_read_bio reads PEM-formatted data from |bp| for the data type given
+// in |name|. If a PEM block is found, it returns one and sets |*pnm| and
+// |*pdata| to newly-allocated buffers containing the PEM type and the decoded
+// data, respectively. |*pnm| is a NUL-terminated C string, while |*pdata| has
+// |*plen| bytes. The caller must release each of |*pnm| and |*pdata| with
+// |OPENSSL_free| when done. If no PEM block is found, this function returns
+// zero and pushes |PEM_R_NO_START_LINE| to the error queue. If one is found,
+// but there is an error decoding it, it returns zero and pushes some other
+// error to the error queue. |cb| is the callback to use when querying for
+// pass phrase used for encrypted PEM structures (normally only private keys)
+// and |u| is interpreted as the null terminated string to use as the
+// passphrase.
 OPENSSL_EXPORT int PEM_bytes_read_bio(unsigned char **pdata, long *plen,
                                       char **pnm, const char *name, BIO *bp,
                                       pem_password_cb *cb, void *u);
+
 OPENSSL_EXPORT void *PEM_ASN1_read_bio(d2i_of_void *d2i, const char *name,
                                        BIO *bp, void **x, pem_password_cb *cb,
                                        void *u);
@@ -473,6 +487,21 @@ OPENSSL_EXPORT int PEM_write_PKCS8PrivateKey(FILE *fp, const EVP_PKEY *x,
                                              int klen, pem_password_cb *cd,
                                              void *u);
 
+// PEM_read_bio_Parameters is a generic PEM deserialization function that
+// parses the public "parameters" in |bio| and returns a corresponding
+// |EVP_PKEY|. If |*pkey| is non-null, the original |*pkey| is freed and the
+// returned |EVP_PKEY| is also written to |*pkey|. |*pkey| must be either NULL
+// or an allocated value, passing in an uninitialized pointer is undefined
+// behavior. This is only supported with |EVP_PKEY_EC|, |EVP_PKEY_DH|, and
+// |EVP_PKEY_DSA|.
+OPENSSL_EXPORT EVP_PKEY *PEM_read_bio_Parameters(BIO *bio, EVP_PKEY **pkey);
+
+// PEM_write_bio_Parameters is a generic PEM serialization function that parses
+// the public "parameters" of |pkey| to |bio|. It returns 1 on success or 0 on
+// failure. This is only supported with |EVP_PKEY_EC|, |EVP_PKEY_DH|, and
+// |EVP_PKEY_DSA|.
+OPENSSL_EXPORT int PEM_write_bio_Parameters(BIO *bio, EVP_PKEY *pkey);
+
 // PEM_read_bio_ECPKParameters deserializes the PEM file written in |bio|
 // according to |ECPKParameters| in RFC 3279. It returns the |EC_GROUP|
 // corresponding to deserialized output and also writes it to |out_group|. Only