Remove X509_REQ_set_extension_nids and document related functions.

PKCS#10 CSRs don't contain extensions but attributes, which are kind of
like extensions, but defined separately. There is an attribute type from
PKCS#9 to embed a list of X.509 extensions inside an attribute, as well
as a Microsoft variant.

X509_REQ_set_extension_nids allowed callers globally reconfigure the set
of attributes recognized as aliases of this extensions attribute.  This
is not used by anyone and not thread-safe. Remove it and only support
the two default attribute types.

From there, document the remaining functions.

Update-Note: This removes a pair of unused functions.
Change-Id: Ic1fc41163996c0c980ba8320b417e444d484aa39
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46326
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/x509/x509_req.c

@@ -157,62 +157,37 @@ int X509_REQ_check_private_key(X509_REQ *x, EVP_PKEY *k)
     return (ok);
 }
 
-/*
- * It seems several organisations had the same idea of including a list of
- * extensions in a certificate request. There are at least two OIDs that are
- * used and there may be more: so the list is configurable.
- */
-
-static const int ext_nid_list[] = { NID_ext_req, NID_ms_ext_req, NID_undef };
-
-static const int *ext_nids = ext_nid_list;
-
 int X509_REQ_extension_nid(int req_nid)
 {
-    int i, nid;
-    for (i = 0;; i++) {
-        nid = ext_nids[i];
-        if (nid == NID_undef)
-            return 0;
-        else if (req_nid == nid)
-            return 1;
-    }
+    return req_nid == NID_ext_req || req_nid == NID_ms_ext_req;
 }
 
-const int *X509_REQ_get_extension_nids(void)
+STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req)
 {
-    return ext_nids;
+    if (req == NULL || req->req_info == NULL) {
+        return NULL;
     }
 
-void X509_REQ_set_extension_nids(const int *nids)
-{
-    ext_nids = nids;
+    int idx = X509_REQ_get_attr_by_NID(req, NID_ext_req, -1);
+    if (idx == -1) {
+        idx = X509_REQ_get_attr_by_NID(req, NID_ms_ext_req, -1);
+    }
+    if (idx == -1) {
+        return NULL;
     }
 
-STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req)
-{
-    X509_ATTRIBUTE *attr;
+    X509_ATTRIBUTE *attr = X509_REQ_get_attr(req, idx);
     ASN1_TYPE *ext = NULL;
-    int idx;
-    const int *pnid;
-    const unsigned char *p;
-
-    if ((req == NULL) || (req->req_info == NULL) || !ext_nids)
-        return (NULL);
-    for (pnid = ext_nids; *pnid != NID_undef; pnid++) {
-        idx = X509_REQ_get_attr_by_NID(req, *pnid, -1);
-        if (idx == -1)
-            continue;
-        attr = X509_REQ_get_attr(req, idx);
-        if (attr->single)
+    if (attr->single) {
         ext = attr->value.single;
-        else if (sk_ASN1_TYPE_num(attr->value.set))
+    } else if (sk_ASN1_TYPE_num(attr->value.set)) {
         ext = sk_ASN1_TYPE_value(attr->value.set, 0);
-        break;
     }
-    if (!ext || (ext->type != V_ASN1_SEQUENCE))
+
+    if (!ext || ext->type != V_ASN1_SEQUENCE) {
         return NULL;
-    p = ext->value.sequence->data;
+    }
+    const unsigned char *p = ext->value.sequence->data;
     return (STACK_OF(X509_EXTENSION) *)
         ASN1_item_d2i(NULL, &p, ext->value.sequence->length,
                       ASN1_ITEM_rptr(X509_EXTENSIONS));
@@ -223,8 +198,8 @@ STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req)
  * in case we want to create a non standard one.
  */
 
-int X509_REQ_add_extensions_nid(X509_REQ *req, STACK_OF(X509_EXTENSION) *exts,
-                                int nid)
+int X509_REQ_add_extensions_nid(X509_REQ *req,
+                                const STACK_OF(X509_EXTENSION) *exts, int nid)
 {
     ASN1_TYPE *at = NULL;
     X509_ATTRIBUTE *attr = NULL;
@@ -260,7 +235,8 @@ int X509_REQ_add_extensions_nid(X509_REQ *req, STACK_OF(X509_EXTENSION) *exts,
 }
 
 /* This is the normal usage: use the "official" OID */
-int X509_REQ_add_extensions(X509_REQ *req, STACK_OF(X509_EXTENSION) *exts)
+int X509_REQ_add_extensions(X509_REQ *req,
+                            const STACK_OF(X509_EXTENSION) *exts)
 {
     return X509_REQ_add_extensions_nid(req, exts, NID_ext_req);
 }

include/openssl/x509.h

@@ -1239,15 +1239,31 @@ OPENSSL_EXPORT int X509_REQ_set_pubkey(X509_REQ *req, EVP_PKEY *pkey);
 // |EVP_PKEY_free| when done.
 OPENSSL_EXPORT EVP_PKEY *X509_REQ_get_pubkey(X509_REQ *req);
 
+// X509_REQ_extension_nid returns one if |nid| is a supported CSR attribute type
+// for carrying extensions and zero otherwise. The supported types are
+// |NID_ext_req| (pkcs-9-at-extensionRequest from RFC2985) and |NID_ms_ext_req|
+// (a Microsoft szOID_CERT_EXTENSIONS variant).
 OPENSSL_EXPORT int X509_REQ_extension_nid(int nid);
-OPENSSL_EXPORT const int *X509_REQ_get_extension_nids(void);
-OPENSSL_EXPORT void X509_REQ_set_extension_nids(const int *nids);
+
+// X509_REQ_get_extensions decodes the list of requested extensions in |req| and
+// returns a newly-allocated |STACK_OF(X509_EXTENSION)| containing the result.
+// It returns NULL on error, or if |req| did not request extensions.
+//
+// This function supports both pkcs-9-at-extensionRequest from RFC2985 and the
+// Microsoft szOID_CERT_EXTENSIONS variant.
 OPENSSL_EXPORT STACK_OF(X509_EXTENSION) *X509_REQ_get_extensions(X509_REQ *req);
-OPENSSL_EXPORT int X509_REQ_add_extensions_nid(X509_REQ *req,
-                                               STACK_OF(X509_EXTENSION) *exts,
-                                               int nid);
-OPENSSL_EXPORT int X509_REQ_add_extensions(X509_REQ *req,
-                                           STACK_OF(X509_EXTENSION) *exts);
+
+// X509_REQ_add_extensions_nid adds an attribute to |req| of type |nid|, to
+// request the certificate extensions in |exts|. It returns one on success and
+// zero on error. |nid| should be |NID_ext_req| or |NID_ms_ext_req|.
+OPENSSL_EXPORT int X509_REQ_add_extensions_nid(
+    X509_REQ *req, const STACK_OF(X509_EXTENSION) *exts, int nid);
+
+// X509_REQ_add_extensions behaves like |X509_REQ_add_extensions_nid|, using the
+// standard |NID_ext_req| for the attribute type.
+OPENSSL_EXPORT int X509_REQ_add_extensions(
+    X509_REQ *req, const STACK_OF(X509_EXTENSION) *exts);
+
 OPENSSL_EXPORT int X509_REQ_get_attr_count(const X509_REQ *req);
 OPENSSL_EXPORT int X509_REQ_get_attr_by_NID(const X509_REQ *req, int nid,
                                             int lastpos);