Add support for parsing ECPKParameter PEM files (#1670)

Ruby depends on reading/writing PEM files that are defined as
`EcpkParameters ` (See RFC3279 for more details). `EcpkParameters`
actually just refer to potential different ways of serializing EC Groups and
we already have basic support for this. OpenSSL's implementation of
`d2i/i2d_ECParameters` actually calls `d2i/i2d_ECPKParameters`
internally.

* I've borrowed some of the logic for `d2i/i2d_ECParameters` and worked
them into the new functions. Apparently we only support parsing named
curves or explicitly encoded versions of them, so I've documented that
and followed the same pattern. No new parsing logic has been added for
`EC_GROUP`s.
* We do not support writing custom curves to any ASN.1 output and this
PR continues that trend.

### Call-outs:
* Code change in `crypto/pem/pem_lib.c` was taken from OpenSSL:
openssl/openssl@2059574.
It was worth taking, so the call to `PEM_write_bio` could be cleaner.

### Testing:
* Tests for named curves and a explicitly encoded versions of P256. The
same PEM file works fine with OpenSSL as well
* We don't support writing custom curves, so I have a test to make sure.

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/ec_extra/ec_asn1.c

@@ -484,9 +484,7 @@ EC_KEY *d2i_ECParameters(EC_KEY **out_key, const uint8_t **inp, long len) {
     return NULL;
   }
 
-  CBS cbs;
-  CBS_init(&cbs, *inp, (size_t)len);
-  const EC_GROUP *group = EC_KEY_parse_parameters(&cbs);
+  EC_GROUP *group = d2i_ECPKParameters(NULL, inp, len);
   if (group == NULL) {
     return NULL;
   }
@@ -501,19 +499,47 @@ EC_KEY *d2i_ECParameters(EC_KEY **out_key, const uint8_t **inp, long len) {
     EC_KEY_free(*out_key);
     *out_key = ret;
   }
-  *inp = CBS_data(&cbs);
   return ret;
 }
 
+EC_GROUP *d2i_ECPKParameters(EC_GROUP **out_group, const uint8_t **inp,
+                             long len) {
+  if (inp == NULL || len < 0) {
+    return NULL;
+  }
+
+  CBS cbs;
+  CBS_init(&cbs, *inp, (size_t)len);
+  EC_GROUP *group = EC_KEY_parse_parameters(&cbs);
+  if (group == NULL) {
+    return NULL;
+  }
+
+  if (out_group != NULL) {
+    EC_GROUP_free(*out_group);
+    *out_group = group;
+  }
+  *inp = CBS_data(&cbs);
+  return group;
+}
+
 int i2d_ECParameters(const EC_KEY *key, uint8_t **outp) {
   if (key == NULL || key->group == NULL) {
     OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
     return -1;
   }
 
+  return i2d_ECPKParameters(key->group, outp);
+}
+
+int i2d_ECPKParameters(const EC_GROUP *group, uint8_t **outp) {
+  if (group == NULL) {
+    OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
+    return -1;
+  }
+
   CBB cbb;
-  if (!CBB_init(&cbb, 0) ||
-      !EC_KEY_marshal_curve_name(&cbb, key->group)) {
+  if (!CBB_init(&cbb, 0) || !EC_KEY_marshal_curve_name(&cbb, group)) {
     CBB_cleanup(&cbb);
     return -1;
   }

crypto/fipsmodule/ec/internal.h

@@ -81,13 +81,14 @@
 extern "C" {
 #endif
 
-// ECDH_compute_shared_secret calculates the shared key between |pub_key| and |priv_key|.
-// This function is called internally by |ECDH_compute_key| and |ECDH_compute_key_fips|.
-// The shared secret is returned in |buf|, the value stored in |buflen| on entry is expected
-// to be EC_MAX_BYTES or the number of bytes of the field element of the underlying curve.
-// On exit, |buflen| is set to the actual number of bytes of the shared secret.
-int ECDH_compute_shared_secret(uint8_t *buf, size_t *buflen, const EC_POINT *pub_key,
-                               const EC_KEY *priv_key);
+// ECDH_compute_shared_secret calculates the shared key between |pub_key| and
+// |priv_key|. This function is called internally by |ECDH_compute_key| and
+// |ECDH_compute_key_fips|. The shared secret is returned in |buf|, the value
+// stored in |buflen| on entry is expected to be EC_MAX_BYTES or the number of
+// bytes of the field element of the underlying curve. On exit, |buflen| is set
+// to the actual number of bytes of the shared secret.
+int ECDH_compute_shared_secret(uint8_t *buf, size_t *buflen,
+                               const EC_POINT *pub_key, const EC_KEY *priv_key);
 
 // EC internals.
 
@@ -770,6 +771,16 @@ struct ec_key_st {
   CRYPTO_EX_DATA ex_data;
 } /* EC_KEY */;
 
+// d2i_ECPKParameters deserializes the |ECPKParameters| specified in RFC 3279
+// to an |EC_GROUP| from |inp|. Only deserialization of namedCurves or
+// explicitly-encoded versions of namedCurves are supported.
+EC_GROUP *d2i_ECPKParameters(EC_GROUP **out_group, const uint8_t **inp,
+                             long len);
+
+// i2d_ECPKParameters serializes an |EC_GROUP| from |outp| according to the
+// |ECPKParameters| specified in RFC 3279. Only serialization of namedCurves
+// are supported.
+int i2d_ECPKParameters(const EC_GROUP *group, uint8_t **outp);
 
 #if defined(__cplusplus)
 }  // extern C

crypto/pem/pem_all.c

@@ -116,6 +116,7 @@
 #include <openssl/pkcs7.h>
 #include <openssl/rsa.h>
 #include <openssl/x509.h>
+#include "../fipsmodule/ec/internal.h"
 
 static RSA *pkey_get_rsa(EVP_PKEY *key, RSA **rsa);
 static DSA *pkey_get_dsa(EVP_PKEY *key, DSA **dsa);
@@ -241,3 +242,41 @@ EC_KEY *PEM_read_ECPrivateKey(FILE *fp, EC_KEY **eckey, pem_password_cb *cb,
 IMPLEMENT_PEM_rw_const(DHparams, DH, PEM_STRING_DHPARAMS, DHparams)
 
 IMPLEMENT_PEM_rw(PUBKEY, EVP_PKEY, PEM_STRING_PUBLIC, PUBKEY)
+
+EC_GROUP *PEM_read_bio_ECPKParameters(BIO *bio, EC_GROUP **out_group,
+                                      pem_password_cb *cb, void *u) {
+  uint8_t *data = NULL;
+  long len;
+  if (!PEM_bytes_read_bio(&data, &len, NULL, PEM_STRING_ECPARAMETERS, bio, cb,
+                          u)) {
+    return NULL;
+  }
+
+  const uint8_t *data_in = data;
+  EC_GROUP *ret = d2i_ECPKParameters(out_group, &data_in, len);
+  if (ret == NULL) {
+    OPENSSL_PUT_ERROR(PEM, ERR_R_ASN1_LIB);
+  }
+  OPENSSL_free(data);
+  return ret;
+}
+
+int PEM_write_bio_ECPKParameters(BIO *out, const EC_GROUP *group) {
+  int ret = 0;
+  unsigned char *data = NULL;
+
+  int buf_len = i2d_ECPKParameters(group, &data);
+  if(data == NULL || buf_len < 0) {
+    OPENSSL_PUT_ERROR(PEM, ERR_R_ASN1_LIB);
+    goto err;
+  }
+
+  if (PEM_write_bio(out, PEM_STRING_ECPARAMETERS, NULL, data, buf_len) <= 0) {
+    goto err;
+  }
+
+  ret = 1;
+err:
+  OPENSSL_free(data);
+  return ret;
+}

crypto/pem/pem_lib.c

@@ -546,7 +546,7 @@ int PEM_write_bio(BIO *bp, const char *name, const char *header,
     goto err;
   }
 
-  i = strlen(header);
+  i = (header != NULL) ? strlen(header) : 0;
   if (i > 0) {
     if ((BIO_write(bp, header, i) != i) || (BIO_write(bp, "\n", 1) != 1)) {
       goto err;

crypto/pem/pem_test.cc

@@ -187,5 +187,81 @@ TEST(PEMTest, WriteReadECPem) {
   const BIGNUM* orig_priv_key = EC_KEY_get0_private_key(ec_key.get());
   const BIGNUM* read_priv_key = EC_KEY_get0_private_key(ec_key_read.get());
   ASSERT_EQ(0, BN_cmp(orig_priv_key, read_priv_key));
+}
+
+const char *kPemECPARAMETERS =
+    "-----BEGIN EC PARAMETERS-----\n"
+    "BgUrgQQAIw==\n"
+    "-----END EC PARAMETERS-----\n";
+
+const char *kPemExplictECPARAMETERS =
+    "-----BEGIN EC PARAMETERS-----\n"
+    "MIH3AgEBMCwGByqGSM49AQECIQD/////AAAAAQAAAAAAAAAAAAAAAP//////////"
+    "/////zBbBCD/////AAAAAQAAAAAAAAAAAAAAAP///////////////AQgWsY12Ko6"
+    "k+ez671VdpiGvGUdBrDMU7D2O848PifSYEsDFQDEnTYIhucEk2pmeOETnSa3gZ9+"
+    "kARBBGsX0fLhLEJH+Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT+NC4v4af5uO5+tK"
+    "fA+eFivOM1drMV7Oy7ZAaDe/UfUCIQD/////AAAAAP//////////vOb6racXnoTz"
+    "ucrC/GMlUQIBAQ==\n"
+    "-----END EC PARAMETERS-----\n";
+
+TEST(PEMTest, WriteReadECPKPem) {
+  // Check named curve can be outputted to a PEM file.
+  bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(NID_secp521r1));
+  ASSERT_TRUE(group);
+  bssl::UniquePtr<BIO> write_bio(BIO_new(BIO_s_mem()));
+  ASSERT_TRUE(write_bio);
+  ASSERT_TRUE(PEM_write_bio_ECPKParameters(write_bio.get(), group.get()));
+
+  const uint8_t *content;
+  size_t content_len;
+  BIO_mem_contents(write_bio.get(), &content, &content_len);
+  EXPECT_EQ(Bytes(content, content_len), Bytes(kPemECPARAMETERS));
+
+  // Check named curve of a PEM file can be parsed.
+  bssl::UniquePtr<BIO> read_bio(
+      BIO_new_mem_buf(kPemECPARAMETERS, strlen(kPemECPARAMETERS)));
+  bssl::UniquePtr<EC_GROUP> read_group(
+      PEM_read_bio_ECPKParameters(read_bio.get(), nullptr, nullptr, nullptr));
+  ASSERT_TRUE(read_group);
+  ASSERT_EQ(EC_GROUP_cmp(EC_group_p521(), read_group.get(), nullptr), 0);
+
+
+  // Make an arbitrary curve which is identical to P-256.
+  static const uint8_t kP[] = {
+      0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+  };
+  static const uint8_t kA[] = {
+      0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfc,
+  };
+  static const uint8_t kB[] = {
+      0x5a, 0xc6, 0x35, 0xd8, 0xaa, 0x3a, 0x93, 0xe7, 0xb3, 0xeb, 0xbd,
+      0x55, 0x76, 0x98, 0x86, 0xbc, 0x65, 0x1d, 0x06, 0xb0, 0xcc, 0x53,
+      0xb0, 0xf6, 0x3b, 0xce, 0x3c, 0x3e, 0x27, 0xd2, 0x60, 0x4b,
+  };
+  bssl::UniquePtr<BIGNUM> p(BN_bin2bn(kP, sizeof(kP), nullptr)),
+      a(BN_bin2bn(kA, sizeof(kA), nullptr)),
+      b(BN_bin2bn(kB, sizeof(kB), nullptr));
+  ASSERT_TRUE(p && a && b);
+
+  // Writing custom curves, even if the parameters are identical to a named
+  // curve, will result in an error
+  bssl::UniquePtr<EC_GROUP> custom_group(
+      EC_GROUP_new_curve_GFp(p.get(), a.get(), b.get(), nullptr));
+  write_bio.reset(BIO_new(BIO_s_mem()));
+  ASSERT_TRUE(write_bio);
+  EXPECT_FALSE(
+      PEM_write_bio_ECPKParameters(write_bio.get(), custom_group.get()));
 
+  // Check that explicitly-encoded versions of namedCurves can be correctly
+  // parsed from a PEM file.
+  read_bio.reset(BIO_new_mem_buf(
+      kPemExplictECPARAMETERS, strlen(kPemExplictECPARAMETERS)));
+  read_group.reset(
+      PEM_read_bio_ECPKParameters(read_bio.get(), nullptr, nullptr, nullptr));
+  ASSERT_TRUE(read_group);
+  ASSERT_EQ(EC_GROUP_cmp(EC_group_p256(), read_group.get(), nullptr), 0);
 }

include/openssl/ec_key.h

@@ -329,14 +329,17 @@ OPENSSL_EXPORT int i2d_ECPrivateKey(const EC_KEY *key, uint8_t **outp);
 // d2i_ECParameters parses a DER-encoded ECParameters structure (RFC 5480) from
 // |len| bytes at |*inp|, as described in |d2i_SAMPLE|.
 //
-// Use |EC_KEY_parse_parameters| or |EC_KEY_parse_curve_name| instead.
+// Use |EC_KEY_parse_parameters| or |EC_KEY_parse_curve_name| instead. Only
+// deserialization of namedCurves or explicitly-encoded versions of named curves
+// are supported.
 OPENSSL_EXPORT EC_KEY *d2i_ECParameters(EC_KEY **out_key, const uint8_t **inp,
                                         long len);
 
 // i2d_ECParameters marshals |key|'s parameters as a DER-encoded OBJECT
 // IDENTIFIER, as described in |i2d_SAMPLE|.
 //
-// Use |EC_KEY_marshal_curve_name| instead.
+// Use |EC_KEY_marshal_curve_name| instead. Only serialization of namedCurves
+// are supported.
 OPENSSL_EXPORT int i2d_ECParameters(const EC_KEY *key, uint8_t **outp);
 
 // o2i_ECPublicKey parses an EC point from |len| bytes at |*inp| into

include/openssl/pem.h

@@ -105,6 +105,7 @@ extern "C" {
 #define PEM_STRING_SSL_SESSION "SSL SESSION PARAMETERS"
 #define PEM_STRING_DSAPARAMS "DSA PARAMETERS"
 #define PEM_STRING_ECDSA_PUBLIC "ECDSA PUBLIC KEY"
+#define PEM_STRING_ECPARAMETERS "EC PARAMETERS"
 #define PEM_STRING_ECPRIVATEKEY "EC PRIVATE KEY"
 #define PEM_STRING_CMS "CMS"
 
@@ -472,6 +473,21 @@ OPENSSL_EXPORT int PEM_write_PKCS8PrivateKey(FILE *fp, const EVP_PKEY *x,
                                              int klen, pem_password_cb *cd,
                                              void *u);
 
+// 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
+// deserialization of namedCurves or explicitly-encoded versions of namedCurves
+// are supported.
+OPENSSL_EXPORT EC_GROUP *PEM_read_bio_ECPKParameters(BIO *bio,
+                                                     EC_GROUP **out_group,
+                                                     pem_password_cb *cb,
+                                                     void *u);
+
+// PEM_write_bio_ECPKParameters serializes |group| as a PEM file to |out|
+// according to |ECPKParameters| in RFC 3279. Only serialization of namedCurves
+// are supported.
+OPENSSL_EXPORT int PEM_write_bio_ECPKParameters(BIO *out,
+                                                const EC_GROUP *group);
 
 #ifdef __cplusplus
 }  // extern "C"