Document and tidy up X509_find_by_*.

I put them under convenience functions because they're just wrappers
over existing getters and comparison functions. Used very occasionally,
but probably not important enough to put in the front of the header.

I const-corrected all parameters except X509_NAME. X509_NAME is still a
little tricky const-wise. (X509_NAME_cmp actually does take const names,
so it would compile, but it's misleading because it would actually
mutate the names.)

While here, I tidied it up a little. X509_issuer_and_serial_cmp isn't
really pulling its weight here and is forcing
X509_find_by_issuer_and_serial to stack-allocate a fake, mostly
uninitialized X509 object. The NULL check is also redundant because
STACK_OF(T) treats NULL as the empty list anyway.

With that, X509_issuer_and_serial_cmp is unused (I found no external
callers), so remove it. It's not a particularly problematic function, so
we can easily put it back, but if unused, one less to document.

Update-Note: Removed X509_issuer_and_serial_cmp as it's unused.
Bug: 426
Change-Id: I8785dea9b96265c1fea0c3c7b59e2979e223d819
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54386
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>

crypto/x509/x509_cmp.c

@@ -71,19 +71,6 @@
 #include "internal.h"
 
 
-int X509_issuer_and_serial_cmp(const X509 *a, const X509 *b) {
-  int i;
-  X509_CINF *ai, *bi;
-
-  ai = a->cert_info;
-  bi = b->cert_info;
-  i = ASN1_INTEGER_cmp(ai->serialNumber, bi->serialNumber);
-  if (i) {
-    return i;
-  }
-  return (X509_NAME_cmp(ai->issuer, bi->issuer));
-}
-
 int X509_issuer_name_cmp(const X509 *a, const X509 *b) {
   return (X509_NAME_cmp(a->cert_info->issuer, b->cert_info->issuer));
 }
@@ -221,40 +208,25 @@ unsigned long X509_NAME_hash_old(X509_NAME *x) {
   return ret;
 }
 
-// Search a stack of X509 for a match
-X509 *X509_find_by_issuer_and_serial(STACK_OF(X509) *sk, X509_NAME *name,
-                                     ASN1_INTEGER *serial) {
-  size_t i;
-  X509_CINF cinf;
-  X509 x, *x509 = NULL;
-
-  if (!sk) {
-    return NULL;
-  }
-
+X509 *X509_find_by_issuer_and_serial(const STACK_OF(X509) *sk, X509_NAME *name,
+                                     const ASN1_INTEGER *serial) {
   if (serial->type != V_ASN1_INTEGER) {
     return NULL;
   }
 
-  x.cert_info = &cinf;
-  cinf.serialNumber = serial;
-  cinf.issuer = name;
-
-  for (i = 0; i < sk_X509_num(sk); i++) {
-    x509 = sk_X509_value(sk, i);
-    if (X509_issuer_and_serial_cmp(x509, &x) == 0) {
+  for (size_t i = 0; i < sk_X509_num(sk); i++) {
+    X509 *x509 = sk_X509_value(sk, i);
+    if (ASN1_INTEGER_cmp(X509_get0_serialNumber(x509), serial) == 0 &&
+        X509_NAME_cmp(X509_get_issuer_name(x509), name) == 0) {
       return x509;
     }
   }
   return NULL;
 }
 
-X509 *X509_find_by_subject(STACK_OF(X509) *sk, X509_NAME *name) {
-  X509 *x509;
-  size_t i;
-
-  for (i = 0; i < sk_X509_num(sk); i++) {
-    x509 = sk_X509_value(sk, i);
+X509 *X509_find_by_subject(const STACK_OF(X509) *sk, X509_NAME *name) {
+  for (size_t i = 0; i < sk_X509_num(sk); i++) {
+    X509 *x509 = sk_X509_value(sk, i);
     if (X509_NAME_cmp(X509_get_subject_name(x509), name) == 0) {
       return x509;
     }

include/openssl/x509.h

@@ -1444,6 +1444,18 @@ OPENSSL_EXPORT int i2d_PKCS8PrivateKeyInfo_fp(FILE *fp, EVP_PKEY *key);
 OPENSSL_EXPORT int i2d_PrivateKey_fp(FILE *fp, EVP_PKEY *pkey);
 OPENSSL_EXPORT int i2d_PUBKEY_fp(FILE *fp, EVP_PKEY *pkey);
 
+// X509_find_by_issuer_and_serial returns the first |X509| in |sk| whose issuer
+// and serial are |name| and |serial|, respectively. If no match is found, it
+// returns NULL.
+OPENSSL_EXPORT X509 *X509_find_by_issuer_and_serial(const STACK_OF(X509) *sk,
+                                                    X509_NAME *name,
+                                                    const ASN1_INTEGER *serial);
+
+// X509_find_by_subject returns the first |X509| in |sk| whose subject is
+// |name|. If no match is found, it returns NULL.
+OPENSSL_EXPORT X509 *X509_find_by_subject(const STACK_OF(X509) *sk,
+                                          X509_NAME *name);
+
 
 // ex_data functions.
 //
@@ -2043,8 +2055,6 @@ OPENSSL_EXPORT int X509_REQ_check_private_key(X509_REQ *x509, EVP_PKEY *pkey);
 
 OPENSSL_EXPORT int X509_check_private_key(X509 *x509, const EVP_PKEY *pkey);
 
-OPENSSL_EXPORT int X509_issuer_and_serial_cmp(const X509 *a, const X509 *b);
-
 OPENSSL_EXPORT int X509_issuer_name_cmp(const X509 *a, const X509 *b);
 OPENSSL_EXPORT unsigned long X509_issuer_name_hash(X509 *a);
 
@@ -2323,12 +2333,6 @@ OPENSSL_EXPORT ASN1_TYPE *X509_ATTRIBUTE_get0_type(X509_ATTRIBUTE *attr,
 
 OPENSSL_EXPORT int X509_verify_cert(X509_STORE_CTX *ctx);
 
-// lookup a cert from a X509 STACK
-OPENSSL_EXPORT X509 *X509_find_by_issuer_and_serial(STACK_OF(X509) *sk,
-                                                    X509_NAME *name,
-                                                    ASN1_INTEGER *serial);
-OPENSSL_EXPORT X509 *X509_find_by_subject(STACK_OF(X509) *sk, X509_NAME *name);
-
 // PKCS#8 utilities
 
 DECLARE_ASN1_FUNCTIONS_const(PKCS8_PRIV_KEY_INFO)