Allow empty return values from PKCS7_get_*.

Right now we're inconsistent about it. If the OPTIONAL container is
missing, we report an error, but if the container is empty, we happily
return nothing. The latter behavior is more convenient for emulating
OpenSSL's PKCS#7 functions.

These are our own functions, so we have some leeway here. Looking
through callers, they appear to handle this fine.

Update-Note: This is a behavior change.
Change-Id: I1321025a64df3054d380003c90e57d9eb95e610f
Reviewed-on: https://boringssl-review.googlesource.com/29364
Reviewed-by: Adam Langley <agl@google.com>

crypto/pkcs7/pkcs7.c

@@ -87,20 +87,21 @@ int PKCS7_get_raw_certificates(STACK_OF(CRYPTO_BUFFER) *out_certs, CBS *cbs,
                                CRYPTO_BUFFER_POOL *pool) {
   CBS signed_data, certificates;
   uint8_t *der_bytes = NULL;
-  int ret = 0;
+  int ret = 0, has_certificates;
   const size_t initial_certs_len = sk_CRYPTO_BUFFER_num(out_certs);
 
-  if (!pkcs7_parse_header(&der_bytes, &signed_data, cbs)) {
-    return 0;
-  }
-
   // See https://tools.ietf.org/html/rfc2315#section-9.1
-  if (!CBS_get_asn1(&signed_data, &certificates,
-                    CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0)) {
-    OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_NO_CERTIFICATES_INCLUDED);
+  if (!pkcs7_parse_header(&der_bytes, &signed_data, cbs) ||
+      !CBS_get_optional_asn1(
+          &signed_data, &certificates, &has_certificates,
+          CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0)) {
     goto err;
   }
 
+  if (!has_certificates) {
+    CBS_init(&certificates, NULL, 0);
+  }
+
   while (CBS_len(&certificates) > 0) {
     CBS cert;
     if (!CBS_get_asn1_element(&certificates, &cert, CBS_ASN1_SEQUENCE)) {

crypto/pkcs7/pkcs7_x509.c

@@ -64,28 +64,24 @@ err:
 int PKCS7_get_CRLs(STACK_OF(X509_CRL) *out_crls, CBS *cbs) {
   CBS signed_data, crls;
   uint8_t *der_bytes = NULL;
-  int ret = 0;
+  int ret = 0, has_crls;
   const size_t initial_crls_len = sk_X509_CRL_num(out_crls);
 
-  if (!pkcs7_parse_header(&der_bytes, &signed_data, cbs)) {
-    return 0;
-  }
-
   // See https://tools.ietf.org/html/rfc2315#section-9.1
-
-  // Even if only CRLs are included, there may be an empty certificates block.
-  // OpenSSL does this, for example.
-  if (CBS_peek_asn1_tag(&signed_data,
-                        CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0) &&
-      !CBS_get_asn1(&signed_data, NULL /* certificates */,
-                    CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0)) {
+  if (!pkcs7_parse_header(&der_bytes, &signed_data, cbs) ||
+      // Even if only CRLs are included, there may be an empty certificates
+      // block. OpenSSL does this, for example.
+      !CBS_get_optional_asn1(
+          &signed_data, NULL, NULL,
+          CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0) ||
+      !CBS_get_optional_asn1(
+          &signed_data, &crls, &has_crls,
+          CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1)) {
     goto err;
   }
 
-  if (!CBS_get_asn1(&signed_data, &crls,
-                    CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1)) {
-    OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_NO_CRLS_INCLUDED);
-    goto err;
+  if (!has_crls) {
+    CBS_init(&crls, NULL, 0);
   }
 
   while (CBS_len(&crls) > 0) {

include/openssl/pkcs7.h

@@ -36,6 +36,9 @@ DECLARE_STACK_OF(X509_CRL)
 // PKCS7_get_raw_certificates parses a PKCS#7, SignedData structure from |cbs|
 // and appends the included certificates to |out_certs|. It returns one on
 // success and zero on error. |cbs| is advanced passed the structure.
+//
+// Note that a SignedData structure may contain no certificates, in which case
+// this function succeeds but does not append any certificates.
 OPENSSL_EXPORT int PKCS7_get_raw_certificates(
     STACK_OF(CRYPTO_BUFFER) *out_certs, CBS *cbs, CRYPTO_BUFFER_POOL *pool);
 
@@ -51,6 +54,9 @@ OPENSSL_EXPORT int PKCS7_bundle_certificates(
 // PKCS7_get_CRLs parses a PKCS#7, SignedData structure from |cbs| and appends
 // the included CRLs to |out_crls|. It returns one on success and zero on error.
 // |cbs| is advanced passed the structure.
+//
+// Note that a SignedData structure may contain no CRLs, in which case this
+// function succeeds but does not append any CRLs.
 OPENSSL_EXPORT int PKCS7_get_CRLs(STACK_OF(X509_CRL) *out_crls, CBS *cbs);
 
 // PKCS7_bundle_CRLs appends a PKCS#7, SignedData structure containing
@@ -60,12 +66,18 @@ OPENSSL_EXPORT int PKCS7_bundle_CRLs(CBB *out, const STACK_OF(X509_CRL) *crls);
 // PKCS7_get_PEM_certificates reads a PEM-encoded, PKCS#7, SignedData structure
 // from |pem_bio| and appends the included certificates to |out_certs|. It
 // returns one on success and zero on error.
+//
+// Note that a SignedData structure may contain no certificates, in which case
+// this function succeeds but does not append any certificates.
 OPENSSL_EXPORT int PKCS7_get_PEM_certificates(STACK_OF(X509) *out_certs,
                                               BIO *pem_bio);
 
 // PKCS7_get_PEM_CRLs reads a PEM-encoded, PKCS#7, SignedData structure from
 // |pem_bio| and appends the included CRLs to |out_crls|. It returns one on
 // success and zero on error.
+//
+// Note that a SignedData structure may contain no CRLs, in which case this
+// function succeeds but does not append any CRLs.
 OPENSSL_EXPORT int PKCS7_get_PEM_CRLs(STACK_OF(X509_CRL) *out_crls,
                                       BIO *pem_bio);