commit 07a1e6236ee89238b4b70af306211fbe5d063f9c
author David Benjamin <davidben@google.com> Fri Apr 18 14:29:42 2025 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Apr 18 11:46:16 2025 -0700
tree d7d1e3fb78f158a3f0b6d467d0acdb212a99f864
parent 999dd48f4d6c52b7c79253c76cd0160591a2c25b

Fix v2i_AUTHORITY_INFO_ACCESS

Regressed in
https://boringssl-review.googlesource.com/c/boringssl/+/78687 because I
forgot to account for the use-after-move (clang-tidy would sure be
nice). Fix by just waiting for the object to be finished before adding
it to the stack, which is tidier anyway.

Looked through that CL and previous scoper CLs and I think this is the
only instance.

Fixed: 411460548
Change-Id: Ib1ad77127dc1bd611ea971188c4cb078547b596f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78727
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>

crypto/x509/v3_info.cc

diff --git a/crypto/x509/v3_info.cc b/crypto/x509/v3_info.cc
index 842e34b..4f6c69d 100644
--- a/crypto/x509/v3_info.cc
+++ b/crypto/x509/v3_info.cc
@@ -129,7 +129,7 @@
   for (size_t i = 0; i < sk_CONF_VALUE_num(nval); i++) {
     const CONF_VALUE *cnf = sk_CONF_VALUE_value(nval, i);
     bssl::UniquePtr<ACCESS_DESCRIPTION> acc(ACCESS_DESCRIPTION_new());
-    if (acc == nullptr || !bssl::PushToStack(ainfo.get(), std::move(acc))) {
+    if (acc == nullptr) {
       return nullptr;
     }
     char *ptmp = strchr(cnf->name, ';');
@@ -153,6 +153,9 @@
       ERR_add_error_data(2, "value=", objtmp.get());
       return nullptr;
     }
+    if (!bssl::PushToStack(ainfo.get(), std::move(acc))) {
+      return nullptr;
+    }
   }
   return ainfo.release();
 }

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 4bed472..b7e9fbf 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -6463,6 +6463,31 @@
       {"policyMappings", "invalid_oid:2.2.2.2", nullptr, {}},
       {"policyMappings", "1.1.1.1:invalid_oid", nullptr, {}},
 
+      // authorityInfoAccess is a comma-separated list of
+      // accessMethod;accessLocation tuples, where the latter specifies a
+      // GeneralName.
+      {"authorityInfoAccess",
+       "caIssuers;URI:http://example.com/1, "
+       "caIssuers;URI:http://example.com/2, OCSP;URI:http://example.com/3, "
+       "OCSP;DNS:non-uri-does-not-make-sense-but-is-allowed.test",
+       nullptr,
+       {0x30, 0x81, 0xb3, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, 0x01,
+        0x01, 0x04, 0x81, 0xa6, 0x30, 0x81, 0xa3, 0x30, 0x20, 0x06, 0x08, 0x2b,
+        0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x02, 0x86, 0x14, 0x68, 0x74, 0x74,
+        0x70, 0x3a, 0x2f, 0x2f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e,
+        0x63, 0x6f, 0x6d, 0x2f, 0x31, 0x30, 0x20, 0x06, 0x08, 0x2b, 0x06, 0x01,
+        0x05, 0x05, 0x07, 0x30, 0x02, 0x86, 0x14, 0x68, 0x74, 0x74, 0x70, 0x3a,
+        0x2f, 0x2f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f,
+        0x6d, 0x2f, 0x32, 0x30, 0x20, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05,
+        0x07, 0x30, 0x01, 0x86, 0x14, 0x68, 0x74, 0x74, 0x70, 0x3a, 0x2f, 0x2f,
+        0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x2f,
+        0x33, 0x30, 0x3b, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30,
+        0x01, 0x82, 0x2f, 0x6e, 0x6f, 0x6e, 0x2d, 0x75, 0x72, 0x69, 0x2d, 0x64,
+        0x6f, 0x65, 0x73, 0x2d, 0x6e, 0x6f, 0x74, 0x2d, 0x6d, 0x61, 0x6b, 0x65,
+        0x2d, 0x73, 0x65, 0x6e, 0x73, 0x65, 0x2d, 0x62, 0x75, 0x74, 0x2d, 0x69,
+        0x73, 0x2d, 0x61, 0x6c, 0x6c, 0x6f, 0x77, 0x65, 0x64, 0x2e, 0x74, 0x65,
+        0x73, 0x74}},
+
       // The "DER:" prefix just specifies an arbitrary byte string. Colons
       // separators are ignored.
       {kTestOID, "DER:0001020304", nullptr, {0x30, 0x15, 0x06, 0x0c, 0x2a, 0x86,