Fix PEM error checking

runtime/bin/secure_socket.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "bin/secure_socket.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <stdio.h>
@@ skipping 490 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ScopedSSLStackType);
 };
 
 typedef ScopedSSLType<PKCS12, PKCS12_free> ScopedPKCS12;
 typedef ScopedSSLType<X509, X509_free> ScopedX509;
 
 typedef ScopedSSLStackType<STACK_OF(X509), X509, X509_free> ScopedX509Stack;
 typedef ScopedSSLStackType<STACK_OF(X509_NAME), X509_NAME, X509_NAME_free>
     ScopedX509NAMEStack;
 
+
+// We try reading data as PKCS12 only if reading as PEM was unsuccessful and
+// if there is no indication that the data is malformed PEM. We assume the data
+// is malformed PEM if it contains the start line, i.e. a line with ----- BEGIN.
+static bool TryPKCS12(bool pem_success) {
+  uint32_t last_error = ERR_peek_last_error();
+  return !pem_success &&
+      (ERR_GET_LIB(last_error) == ERR_LIB_PEM) &&
+      (ERR_GET_REASON(last_error) == PEM_R_NO_START_LINE);
+}
+
+
 static EVP_PKEY* GetPrivateKeyPKCS12(BIO* bio, const char* password) {
   ScopedPKCS12 p12(d2i_PKCS12_bio(bio, NULL));
   if (p12.get() == NULL) {
     return NULL;
   }
 
   EVP_PKEY* key = NULL;
   X509 *cert = NULL;
   STACK_OF(X509) *ca_certs = NULL;
   int status = PKCS12_parse(p12.get(), password, &key, &cert, &ca_certs);
   if (status == 0) {
     return NULL;
   }
 
   // We only care about the private key.
   ScopedX509 delete_cert(cert);
   ScopedX509Stack delete_ca_certs(ca_certs);
   return key;
 }
 
 
 static EVP_PKEY* GetPrivateKey(BIO* bio, const char* password) {
   EVP_PKEY *key = PEM_read_bio_PrivateKey(
       bio, NULL, PasswordCallback, const_cast<char*>(password));
-
-  // If the data doesn't contain the PEM start line, try reading as PKCS12.
-  uint32_t err = ERR_peek_last_error();
-  if ((key == NULL) &&
-      (ERR_GET_LIB(err) == ERR_LIB_PEM) &&
-      (ERR_GET_REASON(err) == PEM_R_NO_START_LINE)) {
+  if (TryPKCS12(key != NULL)) {
     // Reset the bio, and clear the error from trying to read as PEM.
     ERR_clear_error();
     BIO_reset(bio);
 
     // Try to decode as PKCS12
     key = GetPrivateKeyPKCS12(bio, password);
   }
   return key;
 }
 
@@ skipping 83 matching lines @@
   int status = 0;
   X509* cert = NULL;
   while ((cert = PEM_read_bio_X509(bio, NULL, NULL, NULL)) != NULL) {
     status = X509_STORE_add_cert(store, cert);
     if (status == 0) {
       X509_free(cert);
       return status;
     }
   }
 
+  // If bio does not contain PEM data, the first call to PEM_read_bio_X509 will
+  // return NULL, and the while-loop will exit while status is still 0.
   uint32_t err = ERR_peek_last_error();
   if ((ERR_GET_LIB(err) != ERR_LIB_PEM) ||
       (ERR_GET_REASON(err) != PEM_R_NO_START_LINE)) {
-    // Some real error happened.
+    // If bio contains data that is trying to be PEM but is malformed, then
+    // this case will be triggered.
     status = 0;
   }
 
   return status;
 }
 
 
 static int SetTrustedCertificatesBytes(SSL_CTX* context, BIO* bio) {
   int status = SetTrustedCertificatesBytesPEM(context, bio);
-  uint32_t err = ERR_peek_last_error();
-  if ((status == 0) &&
-      (ERR_GET_LIB(err) == ERR_LIB_PEM) &&
-      (ERR_GET_REASON(err) == PEM_R_NO_START_LINE)) {
+  if (TryPKCS12(status != 0)) {
     ERR_clear_error();
     BIO_reset(bio);
     status = SetTrustedCertificatesBytesPKCS12(context, bio);
   } else if (status != 0) {
     // The PEM file was successfully parsed.
     ERR_clear_error();
   }
   return status;
 }
 
@@ skipping 102 matching lines @@
     status = SSL_CTX_add0_chain_cert(context, ca);
     if (status == 0) {
       X509_free(ca);
       return status;
     }
     // Note that we must not free `ca` if it was successfully added to the
     // chain. We must free the main certificate x509, though since its reference
     // count is increased by SSL_CTX_use_certificate.
   }
 
+  // If bio does not contain PEM data, the first call to PEM_read_bio_X509 will
+  // return NULL, and the while-loop will exit while status is still 0.
   uint32_t err = ERR_peek_last_error();
   if ((ERR_GET_LIB(err) != ERR_LIB_PEM) ||
       (ERR_GET_REASON(err) != PEM_R_NO_START_LINE)) {
-    // Some real error happened.
+    // If bio contains data that is trying to be PEM but is malformed, then
+    // this case will be triggered.
     status = 0;
   }
 
   return status;
 }
 
 
 static int UseChainBytes(SSL_CTX* context, BIO* bio) {
   int status = UseChainBytesPEM(context, bio);
-  uint32_t err = ERR_peek_last_error();
-  if ((status == 0) &&
-      (ERR_GET_LIB(err) == ERR_LIB_PEM) &&
-      (ERR_GET_REASON(err) == PEM_R_NO_START_LINE)) {
+  if (TryPKCS12(status != 0)) {
     ERR_clear_error();
     BIO_reset(bio);
     status = UseChainBytesPKCS12(context, bio);
   } else if (status != 0) {
-    // The PEM file was successfully parsed.
+    // The PEM file was successfully read.
     ERR_clear_error();
   }
   return status;
 }
 
 
 void FUNCTION_NAME(SecurityContext_UseCertificateChainBytes)(
     Dart_NativeArguments args) {
   SSL_CTX* context = GetSecurityContext(args);
   int status;
@@ skipping 90 matching lines @@
     }
 
     // Duplicate the name to put it on the stack.
     x509_name = X509_NAME_dup(x509_name);
     if (x509_name == NULL) {
       return NULL;
     }
     sk_X509_NAME_push(result.get(), x509_name);
   }
 
+  if (sk_X509_NAME_num(result.get()) == 0) {
+    // The data was not PEM.
+    return NULL;
+  }
+
   uint32_t err = ERR_peek_last_error();
   if ((ERR_GET_LIB(err) != ERR_LIB_PEM) ||
       (ERR_GET_REASON(err) != PEM_R_NO_START_LINE)) {
-    // Some real error happened.
+    // The data was trying to be PEM, but was malformed.
     return NULL;
   }
 
   return result.release();
 }
 
 
 static STACK_OF(X509_NAME)* GetCertificateNames(BIO* bio) {
   STACK_OF(X509_NAME)* result = GetCertificateNamesPEM(bio);
-  uint32_t err = ERR_peek_last_error();
-  if ((result == NULL) &&
-      (ERR_GET_LIB(err) == ERR_LIB_PEM) &&
-      (ERR_GET_REASON(err) == PEM_R_NO_START_LINE)) {
+  if (TryPKCS12(result != NULL)) {
     ERR_clear_error();
     BIO_reset(bio);
     result = GetCertificateNamesPKCS12(bio);
   } else if (result != NULL) {
     // The PEM file was successfully parsed.
     ERR_clear_error();
   }
   return result;
 }
 
@@ skipping 633 matching lines @@
     } else {
       if (SSL_LOG_DATA) Log::Print(
           "WriteEncrypted  BIO_read wrote %d bytes\n", bytes_processed);
     }
   }
   return bytes_processed;
 }
 
 }  // namespace bin
 }  // namespace dart