Const-correct X509_EXTENSION functions, as best we can.

Some of these were non-const because dup functions weren't
const-correct, but they are now. Once nuisance is the accessors. Ideally
they'd return non-const pointers, but that'll break OpenSSL consumers.

Bug: 407
Change-Id: I52b939a846b726d1d84dd2d5fdf71a7a7284d49e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53336
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

crypto/x509/t_req.c

@@ -208,13 +208,12 @@ int X509_REQ_print_ex(BIO *bio, X509_REQ *x, unsigned long nmflags,
     if (exts) {
       BIO_printf(bio, "%8sRequested Extensions:\n", "");
 
-      size_t i;
-      for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
-        X509_EXTENSION *ex = sk_X509_EXTENSION_value(exts, i);
+      for (size_t i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
+        const X509_EXTENSION *ex = sk_X509_EXTENSION_value(exts, i);
         if (BIO_printf(bio, "%12s", "") <= 0) {
           goto err;
         }
-        ASN1_OBJECT *obj = X509_EXTENSION_get_object(ex);
+        const ASN1_OBJECT *obj = X509_EXTENSION_get_object(ex);
         i2a_ASN1_OBJECT(bio, obj);
         const int is_critical = X509_EXTENSION_get_critical(ex);
         if (BIO_printf(bio, ": %s\n", is_critical ? "critical" : "") <= 0) {

crypto/x509/x509_ext.c

@@ -99,7 +99,7 @@ int X509_CRL_add1_ext_i2d(X509_CRL *x, int nid, void *value, int crit,
   return X509V3_add1_i2d(&x->crl->extensions, nid, value, crit, flags);
 }
 
-int X509_CRL_add_ext(X509_CRL *x, X509_EXTENSION *ex, int loc) {
+int X509_CRL_add_ext(X509_CRL *x, const X509_EXTENSION *ex, int loc) {
   return (X509v3_add_ext(&(x->crl->extensions), ex, loc) != NULL);
 }
 
@@ -127,7 +127,7 @@ X509_EXTENSION *X509_delete_ext(X509 *x, int loc) {
   return (X509v3_delete_ext(x->cert_info->extensions, loc));
 }
 
-int X509_add_ext(X509 *x, X509_EXTENSION *ex, int loc) {
+int X509_add_ext(X509 *x, const X509_EXTENSION *ex, int loc) {
   return (X509v3_add_ext(&(x->cert_info->extensions), ex, loc) != NULL);
 }
 
@@ -168,7 +168,7 @@ X509_EXTENSION *X509_REVOKED_delete_ext(X509_REVOKED *x, int loc) {
   return (X509v3_delete_ext(x->extensions, loc));
 }
 
-int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex, int loc) {
+int X509_REVOKED_add_ext(X509_REVOKED *x, const X509_EXTENSION *ex, int loc) {
   return (X509v3_add_ext(&(x->extensions), ex, loc) != NULL);
 }
 

crypto/x509/x509_test.cc

@@ -4402,10 +4402,10 @@ TEST(X509Test, AddExt) {
     ASSERT_EQ(static_cast<size_t>(X509_get_ext_count(x509.get())), exts.size());
     for (size_t i = 0; i < exts.size(); i++) {
       SCOPED_TRACE(i);
-      X509_EXTENSION *ext = X509_get_ext(x509.get(), static_cast<int>(i));
+      const X509_EXTENSION *ext = X509_get_ext(x509.get(), static_cast<int>(i));
       EXPECT_EQ(OBJ_obj2nid(X509_EXTENSION_get_object(ext)), exts[i].nid);
       EXPECT_EQ(X509_EXTENSION_get_critical(ext), exts[i].critical ? 1 : 0);
-      ASN1_OCTET_STRING *data = X509_EXTENSION_get_data(ext);
+      const ASN1_OCTET_STRING *data = X509_EXTENSION_get_data(ext);
       EXPECT_EQ(Bytes(ASN1_STRING_get0_data(data), ASN1_STRING_length(data)),
                 Bytes(exts[i].data));
     }

crypto/x509/x509_v3.c

@@ -144,7 +144,7 @@ X509_EXTENSION *X509v3_delete_ext(STACK_OF(X509_EXTENSION) *x, int loc) {
 }
 
 STACK_OF(X509_EXTENSION) *X509v3_add_ext(STACK_OF(X509_EXTENSION) **x,
-                                         X509_EXTENSION *ex, int loc) {
+                                         const X509_EXTENSION *ex, int loc) {
   X509_EXTENSION *new_ex = NULL;
   int n;
   STACK_OF(X509_EXTENSION) *sk = NULL;
@@ -267,14 +267,14 @@ int X509_EXTENSION_set_data(X509_EXTENSION *ex, const ASN1_OCTET_STRING *data) {
   return 1;
 }
 
-ASN1_OBJECT *X509_EXTENSION_get_object(X509_EXTENSION *ex) {
+ASN1_OBJECT *X509_EXTENSION_get_object(const X509_EXTENSION *ex) {
   if (ex == NULL) {
     return NULL;
   }
   return ex->object;
 }
 
-ASN1_OCTET_STRING *X509_EXTENSION_get_data(X509_EXTENSION *ex) {
+ASN1_OCTET_STRING *X509_EXTENSION_get_data(const X509_EXTENSION *ex) {
   if (ex == NULL) {
     return NULL;
   }

crypto/x509/x509_vfy.c

@@ -1097,7 +1097,7 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
 // both present or both absent. If both present all fields must be identical.
 
 static int crl_extension_match(X509_CRL *a, X509_CRL *b, int nid) {
-  ASN1_OCTET_STRING *exta, *extb;
+  const ASN1_OCTET_STRING *exta, *extb;
   int i;
   i = X509_CRL_get_ext_by_NID(a, nid, -1);
   if (i >= 0) {
@@ -2025,8 +2025,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer, EVP_PKEY *skey,
   // number to correct value too.
 
   for (i = 0; i < X509_CRL_get_ext_count(newer); i++) {
-    X509_EXTENSION *ext;
-    ext = X509_CRL_get_ext(newer, i);
+    const X509_EXTENSION *ext = X509_CRL_get_ext(newer, i);
     if (!X509_CRL_add_ext(crl, ext, -1)) {
       goto memerr;
     }

crypto/x509v3/v3_akey.c

@@ -143,7 +143,6 @@ static void *v2i_AUTHORITY_KEYID(const X509V3_EXT_METHOD *method,
   GENERAL_NAMES *gens = NULL;
   GENERAL_NAME *gen = NULL;
   ASN1_INTEGER *serial = NULL;
-  X509_EXTENSION *ext;
   X509 *cert;
   AUTHORITY_KEYID *akeyid;
 
@@ -178,6 +177,7 @@ static void *v2i_AUTHORITY_KEYID(const X509V3_EXT_METHOD *method,
 
   if (keyid) {
     j = X509_get_ext_by_NID(cert, NID_subject_key_identifier, -1);
+    const X509_EXTENSION *ext;
     if ((j >= 0) && (ext = X509_get_ext(cert, j))) {
       ikeyid = X509V3_EXT_d2i(ext);
     }

crypto/x509v3/v3_prn.c

@@ -66,15 +66,13 @@
 
 // Extension printing routines
 
-static int unknown_ext_print(BIO *out, X509_EXTENSION *ext, unsigned long flag,
-                             int indent, int supported);
+static int unknown_ext_print(BIO *out, const X509_EXTENSION *ext,
+                             unsigned long flag, int indent, int supported);
 
 // Print out a name+value stack
 
-void X509V3_EXT_val_prn(BIO *out, STACK_OF(CONF_VALUE) *val, int indent,
+void X509V3_EXT_val_prn(BIO *out, const STACK_OF(CONF_VALUE) *val, int indent,
                         int ml) {
-  size_t i;
-  CONF_VALUE *nval;
   if (!val) {
     return;
   }
@@ -84,13 +82,13 @@ void X509V3_EXT_val_prn(BIO *out, STACK_OF(CONF_VALUE) *val, int indent,
       BIO_puts(out, "<EMPTY>\n");
     }
   }
-  for (i = 0; i < sk_CONF_VALUE_num(val); i++) {
+  for (size_t i = 0; i < sk_CONF_VALUE_num(val); i++) {
     if (ml) {
       BIO_printf(out, "%*s", indent, "");
     } else if (i > 0) {
       BIO_printf(out, ", ");
     }
-    nval = sk_CONF_VALUE_value(val, i);
+    const CONF_VALUE *nval = sk_CONF_VALUE_value(val, i);
     if (!nval->name) {
       BIO_puts(out, nval->value);
     } else if (!nval->value) {
@@ -106,7 +104,7 @@ void X509V3_EXT_val_prn(BIO *out, STACK_OF(CONF_VALUE) *val, int indent,
 
 // Main routine: print out a general extension
 
-int X509V3_EXT_print(BIO *out, X509_EXTENSION *ext, unsigned long flag,
+int X509V3_EXT_print(BIO *out, const X509_EXTENSION *ext, unsigned long flag,
                      int indent) {
   void *ext_str = NULL;
   char *value = NULL;
@@ -180,13 +178,11 @@ int X509V3_extensions_print(BIO *bp, const char *title,
   }
 
   for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
-    ASN1_OBJECT *obj;
-    X509_EXTENSION *ex;
-    ex = sk_X509_EXTENSION_value(exts, i);
+    const X509_EXTENSION *ex = sk_X509_EXTENSION_value(exts, i);
     if (indent && BIO_printf(bp, "%*s", indent, "") <= 0) {
       return 0;
     }
-    obj = X509_EXTENSION_get_object(ex);
+    const ASN1_OBJECT *obj = X509_EXTENSION_get_object(ex);
     i2a_ASN1_OBJECT(bp, obj);
     j = X509_EXTENSION_get_critical(ex);
     if (BIO_printf(bp, ": %s\n", j ? "critical" : "") <= 0) {
@@ -203,8 +199,8 @@ int X509V3_extensions_print(BIO *bp, const char *title,
   return 1;
 }
 
-static int unknown_ext_print(BIO *out, X509_EXTENSION *ext, unsigned long flag,
-                             int indent, int supported) {
+static int unknown_ext_print(BIO *out, const X509_EXTENSION *ext,
+                             unsigned long flag, int indent, int supported) {
   switch (flag & X509V3_EXT_UNKNOWN_MASK) {
     case X509V3_EXT_DEFAULT:
       return 0;
@@ -229,7 +225,8 @@ static int unknown_ext_print(BIO *out, X509_EXTENSION *ext, unsigned long flag,
   }
 }
 
-int X509V3_EXT_print_fp(FILE *fp, X509_EXTENSION *ext, int flag, int indent) {
+int X509V3_EXT_print_fp(FILE *fp, const X509_EXTENSION *ext, int flag,
+                        int indent) {
   BIO *bio_tmp;
   int ret;
   if (!(bio_tmp = BIO_new_fp(fp, BIO_NOCLOSE))) {

crypto/x509v3/v3_purp.c

@@ -320,7 +320,7 @@ static int nid_cmp(const void *void_a, const void *void_b) {
   return *a - *b;
 }
 
-int X509_supported_extension(X509_EXTENSION *ex) {
+int X509_supported_extension(const X509_EXTENSION *ex) {
   // This table is a list of the NIDs of supported extensions: that is
   // those which are used by the verify process. If an extension is
   // critical and doesn't appear in this list then the verify process will
@@ -405,7 +405,6 @@ int x509v3_cache_extensions(X509 *x) {
   ASN1_BIT_STRING *usage;
   ASN1_BIT_STRING *ns;
   EXTENDED_KEY_USAGE *extusage;
-  X509_EXTENSION *ex;
   size_t i;
   int j;
 
@@ -576,7 +575,7 @@ int x509v3_cache_extensions(X509 *x) {
   }
 
   for (j = 0; j < X509_get_ext_count(x); j++) {
-    ex = X509_get_ext(x, j);
+    const X509_EXTENSION *ex = X509_get_ext(x, j);
     if (OBJ_obj2nid(X509_EXTENSION_get_object(ex)) == NID_freshest_crl) {
       x->ex_flags |= EXFLAG_FRESHEST;
     }
@@ -768,7 +767,7 @@ static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x,
   // Extended Key Usage MUST be critical
   i_ext = X509_get_ext_by_NID((X509 *)x, NID_ext_key_usage, -1);
   if (i_ext >= 0) {
-    X509_EXTENSION *ext = X509_get_ext((X509 *)x, i_ext);
+    const X509_EXTENSION *ext = X509_get_ext((X509 *)x, i_ext);
     if (!X509_EXTENSION_get_critical(ext)) {
       return 0;
     }

include/openssl/x509.h

@@ -1889,7 +1889,8 @@ OPENSSL_EXPORT int X509v3_get_ext_by_critical(const STACK_OF(X509_EXTENSION) *x,
                                               int crit, int lastpos);
 
 // X509v3_get_ext returns the extension in |x| at index |loc|, or NULL if |loc|
-// is out of bounds.
+// is out of bounds. This function returns a non-const pointer for OpenSSL
+// compatibility, but callers should not mutate the result.
 OPENSSL_EXPORT X509_EXTENSION *X509v3_get_ext(const STACK_OF(X509_EXTENSION) *x,
                                               int loc);
 
@@ -1908,7 +1909,7 @@ OPENSSL_EXPORT X509_EXTENSION *X509v3_delete_ext(STACK_OF(X509_EXTENSION) *x,
 // right. If |loc| is -1 or out of bounds, the new extension is appended to the
 // list.
 OPENSSL_EXPORT STACK_OF(X509_EXTENSION) *X509v3_add_ext(
-    STACK_OF(X509_EXTENSION) **x, X509_EXTENSION *ex, int loc);
+    STACK_OF(X509_EXTENSION) **x, const X509_EXTENSION *ex, int loc);
 
 // X509_get_ext_count returns the number of extensions in |x|.
 OPENSSL_EXPORT int X509_get_ext_count(const X509 *x);
@@ -1928,7 +1929,8 @@ OPENSSL_EXPORT int X509_get_ext_by_critical(const X509 *x, int crit,
                                             int lastpos);
 
 // X509_get_ext returns the extension in |x| at index |loc|, or NULL if |loc| is
-// out of bounds.
+// out of bounds. This function returns a non-const pointer for OpenSSL
+// compatibility, but callers should not mutate the result.
 OPENSSL_EXPORT X509_EXTENSION *X509_get_ext(const X509 *x, int loc);
 
 // X509_delete_ext removes the extension in |x| at index |loc| and returns the
@@ -1944,7 +1946,7 @@ OPENSSL_EXPORT X509_EXTENSION *X509_delete_ext(X509 *x, int loc);
 // The new extension is inserted at index |loc|, shifting extensions to the
 // right. If |loc| is -1 or out of bounds, the new extension is appended to the
 // list.
-OPENSSL_EXPORT int X509_add_ext(X509 *x, X509_EXTENSION *ex, int loc);
+OPENSSL_EXPORT int X509_add_ext(X509 *x, const X509_EXTENSION *ex, int loc);
 
 // X509_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the extension in
 // |x509|'s extension list.
@@ -1982,7 +1984,8 @@ OPENSSL_EXPORT int X509_CRL_get_ext_by_critical(const X509_CRL *x, int crit,
                                                 int lastpos);
 
 // X509_CRL_get_ext returns the extension in |x| at index |loc|, or NULL if
-// |loc| is out of bounds.
+// |loc| is out of bounds. This function returns a non-const pointer for OpenSSL
+// compatibility, but callers should not mutate the result.
 OPENSSL_EXPORT X509_EXTENSION *X509_CRL_get_ext(const X509_CRL *x, int loc);
 
 // X509_CRL_delete_ext removes the extension in |x| at index |loc| and returns
@@ -1998,7 +2001,8 @@ OPENSSL_EXPORT X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc);
 // The new extension is inserted at index |loc|, shifting extensions to the
 // right. If |loc| is -1 or out of bounds, the new extension is appended to the
 // list.
-OPENSSL_EXPORT int X509_CRL_add_ext(X509_CRL *x, X509_EXTENSION *ex, int loc);
+OPENSSL_EXPORT int X509_CRL_add_ext(X509_CRL *x, const X509_EXTENSION *ex,
+                                    int loc);
 
 // X509_CRL_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the
 // extension in |crl|'s extension list.
@@ -2037,7 +2041,8 @@ OPENSSL_EXPORT int X509_REVOKED_get_ext_by_critical(const X509_REVOKED *x,
                                                     int crit, int lastpos);
 
 // X509_REVOKED_get_ext returns the extension in |x| at index |loc|, or NULL if
-// |loc| is out of bounds.
+// |loc| is out of bounds. This function returns a non-const pointer for OpenSSL
+// compatibility, but callers should not mutate the result.
 OPENSSL_EXPORT X509_EXTENSION *X509_REVOKED_get_ext(const X509_REVOKED *x,
                                                     int loc);
 
@@ -2056,8 +2061,8 @@ OPENSSL_EXPORT X509_EXTENSION *X509_REVOKED_delete_ext(X509_REVOKED *x,
 // The new extension is inserted at index |loc|, shifting extensions to the
 // right. If |loc| is -1 or out of bounds, the new extension is appended to the
 // list.
-OPENSSL_EXPORT int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex,
-                                        int loc);
+OPENSSL_EXPORT int X509_REVOKED_add_ext(X509_REVOKED *x,
+                                        const X509_EXTENSION *ex, int loc);
 
 // X509_REVOKED_get_ext_d2i behaves like |X509V3_get_d2i| but looks for the
 // extension in |revoked|'s extension list.
@@ -2109,11 +2114,16 @@ OPENSSL_EXPORT int X509_EXTENSION_set_critical(X509_EXTENSION *ex, int crit);
 OPENSSL_EXPORT int X509_EXTENSION_set_data(X509_EXTENSION *ex,
                                            const ASN1_OCTET_STRING *data);
 
-// X509_EXTENSION_get_object returns |ex|'s extension type.
-OPENSSL_EXPORT ASN1_OBJECT *X509_EXTENSION_get_object(X509_EXTENSION *ex);
-
-// X509_EXTENSION_get_data returns |ne|'s extension value.
-OPENSSL_EXPORT ASN1_OCTET_STRING *X509_EXTENSION_get_data(X509_EXTENSION *ne);
+// X509_EXTENSION_get_object returns |ex|'s extension type. This function
+// returns a non-const pointer for OpenSSL compatibility, but callers should not
+// mutate the result.
+OPENSSL_EXPORT ASN1_OBJECT *X509_EXTENSION_get_object(const X509_EXTENSION *ex);
+
+// X509_EXTENSION_get_data returns |ne|'s extension value. This function returns
+// a non-const pointer for OpenSSL compatibility, but callers should not mutate
+// the result.
+OPENSSL_EXPORT ASN1_OCTET_STRING *X509_EXTENSION_get_data(
+    const X509_EXTENSION *ne);
 
 // X509_EXTENSION_get_critical returns one if |ex| is critical and zero
 // otherwise.

include/openssl/x509v3.h

@@ -787,12 +787,13 @@ OPENSSL_EXPORT int X509V3_add1_i2d(STACK_OF(X509_EXTENSION) **x, int nid,
 // hexdump.
 #define X509V3_EXT_DUMP_UNKNOWN (3L << 16)
 
-OPENSSL_EXPORT void X509V3_EXT_val_prn(BIO *out, STACK_OF(CONF_VALUE) *val,
+OPENSSL_EXPORT void X509V3_EXT_val_prn(BIO *out,
+                                       const STACK_OF(CONF_VALUE) *val,
                                        int indent, int ml);
-OPENSSL_EXPORT int X509V3_EXT_print(BIO *out, X509_EXTENSION *ext,
+OPENSSL_EXPORT int X509V3_EXT_print(BIO *out, const X509_EXTENSION *ext,
                                     unsigned long flag, int indent);
-OPENSSL_EXPORT int X509V3_EXT_print_fp(FILE *out, X509_EXTENSION *ext, int flag,
-                                       int indent);
+OPENSSL_EXPORT int X509V3_EXT_print_fp(FILE *out, const X509_EXTENSION *ext,
+                                       int flag, int indent);
 
 // X509V3_extensions_print prints |title|, followed by a human-readable
 // representation of |exts| to |out|. It returns one on success and zero on
@@ -805,7 +806,7 @@ OPENSSL_EXPORT int X509V3_extensions_print(BIO *out, const char *title,
 
 OPENSSL_EXPORT int X509_check_ca(X509 *x);
 OPENSSL_EXPORT int X509_check_purpose(X509 *x, int id, int ca);
-OPENSSL_EXPORT int X509_supported_extension(X509_EXTENSION *ex);
+OPENSSL_EXPORT int X509_supported_extension(const X509_EXTENSION *ex);
 OPENSSL_EXPORT int X509_PURPOSE_set(int *p, int purpose);
 OPENSSL_EXPORT int X509_check_issued(X509 *issuer, X509 *subject);
 OPENSSL_EXPORT int X509_check_akid(X509 *issuer, AUTHORITY_KEYID *akid);