PR comments; documentation, checks, and fix CFI CI

crypto/pem/pem_pkey.c

@@ -158,6 +158,11 @@ int PEM_write_bio_PrivateKey(BIO *bp, EVP_PKEY *x, const EVP_CIPHER *enc,
 }
 
 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;
@@ -171,6 +176,9 @@ EVP_PKEY *PEM_read_bio_Parameters(BIO *bio, EVP_PKEY **pkey) {
   // |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)) {
@@ -212,8 +220,21 @@ EVP_PKEY *PEM_read_bio_Parameters(BIO *bio, EVP_PKEY **pkey) {
   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 || pkey->ameth == NULL) {
+  if (bio == NULL || pkey == NULL) {
+    OPENSSL_PUT_ERROR(PEM, ERR_R_PASSED_NULL_PARAMETER);
     return 0;
   }
 
@@ -224,16 +245,16 @@ int PEM_write_bio_Parameters(BIO *bio, EVP_PKEY *pkey) {
   switch (pkey->type) {
     case EVP_PKEY_EC:
       BIO_snprintf(pem_str, 80, PEM_STRING_ECPARAMETERS);
-      return PEM_ASN1_write_bio((i2d_of_void *)i2d_ECParameters, pem_str, bio,
+      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_of_void *)i2d_DSAparams, pem_str, bio,
+      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_of_void *)i2d_DHparams, pem_str, bio,
-                                pkey->pkey.dh, NULL, NULL, 0, 0, NULL);
+      return PEM_ASN1_write_bio(i2d_DHparams_void, pem_str, bio, pkey->pkey.dh,
+                                NULL, NULL, 0, 0, NULL);
     default:
       return 0;
   }

include/openssl/pem.h

@@ -344,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);
@@ -477,8 +490,10 @@ OPENSSL_EXPORT int PEM_write_PKCS8PrivateKey(FILE *fp, const EVP_PKEY *x,
 // 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|. This is only supported with
-// |EVP_PKEY_EC|, |EVP_PKEY_DH|, and |EVP_PKEY_DSA|.
+// 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