tls: load all CRLs from a PEM bundle

src/crypto/crypto_tls_certificates.cc

@@ -57,16 +57,33 @@ bool AddCRL(Environment* env,
             X509_STORE** cache) {
   if (!bio) return false;
 
-  DeleteFnPtr<X509_CRL, X509_CRL_free> crl(
-      PEM_read_bio_X509_CRL(bio.get(), nullptr, NoPasswordCallback, nullptr));
-  if (!crl) return false;
-
-  X509_STORE* cert_store = GetOrCreateOwnedCertStore(env, ctx, cache);
-  CHECK_EQ(1, X509_STORE_add_crl(cert_store, crl.get()));
-  CHECK_EQ(1,
-           X509_STORE_set_flags(
-               cert_store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL));
-  return true;
+  using CRLPointer = DeleteFnPtr<X509_CRL, X509_CRL_free>;
+
+  // So that ERR_peek_last_error() below only reports errors from this loop.
+  ERR_clear_error();
+
+  bool added = false;
+  while (CRLPointer crl = CRLPointer(PEM_read_bio_X509_CRL(
+             bio.get(), nullptr, NoPasswordCallback, nullptr))) {
+    X509_STORE* cert_store = GetOrCreateOwnedCertStore(env, ctx, cache);
+    CHECK_EQ(1, X509_STORE_add_crl(cert_store, crl.get()));
+    CHECK_EQ(
+        1,
+        X509_STORE_set_flags(
+            cert_store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL));
+    added = true;
+  }
+
+  // The loop stops either because the BIO is exhausted, which OpenSSL reports
+  // as PEM_R_NO_START_LINE, or because a CRL failed to parse. Only the former
+  // means every CRL in the bundle made it into the store.
+  unsigned long err = ERR_peek_last_error();  // NOLINT(runtime/int)
+  if (ERR_GET_LIB(err) != ERR_LIB_PEM ||
+      ERR_GET_REASON(err) != PEM_R_NO_START_LINE) {
+    return false;
+  }
+
+  return added;
 }
 
 PrivateKeyResult UsePrivateKey(SSL_CTX* ctx,

src/crypto/crypto_tls_certificates.h

@@ -30,7 +30,8 @@ size_t AddCACertificates(Environment* env,
                          const ncrypto::BIOPointer& bio,
                          X509_STORE** cache = nullptr);
 
-// Add one PEM CRL and enable CRL checking.
+// Add every PEM CRL in |bio| to the context's certificate store and enable CRL
+// checking. Returns false unless every CRL in the bundle was read.
 bool AddCRL(Environment* env,
             SSL_CTX* ctx,
             const ncrypto::BIOPointer& bio,

test/parallel/test-tls-crl-bundle.js

@@ -0,0 +1,42 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// Verify that every CRL in a concatenated PEM bundle is loaded, not just the
+// first one. agent3 is revoked by ca2-crl-agent3.pem, but not by ca2-crl.pem.
+
+const fixtures = require('../common/fixtures');
+const tls = require('tls');
+const {
+  assert, connect, keys
+} = require(fixtures.path('tls-connect'));
+
+const crl = fixtures.readKey('ca2-crl.pem') +
+            fixtures.readKey('ca2-crl-agent3.pem');
+
+connect({
+  client: {
+    servername: 'agent3',
+    ca: keys.agent3.ca,
+    crl,
+  },
+  server: {
+    cert: keys.agent3.cert,
+    key: keys.agent3.key,
+  },
+}, common.mustCall((err, pair, cleanup) => {
+  assert(err);
+  assert.strictEqual(err.code, 'CERT_REVOKED');
+  return cleanup();
+}));
+
+// A bundle whose second entry does not parse must throw rather than quietly
+// apply only the CRLs that were read.
+const lines = fixtures.readKey('ca2-crl-agent3.pem', 'utf8').split('\n');
+lines[2] = 'AAAA' + lines[2].slice(4);
+assert.throws(() => {
+  tls.createSecureContext({
+    crl: fixtures.readKey('ca2-crl.pem', 'utf8') + lines.join('\n'),
+  });
+}, { code: 'ERR_CRYPTO_OPERATION_FAILED' });