correct handling of certificate chains

panda/src/downloadertools/multify.cxx

@@ -49,7 +49,6 @@ bool got_chdir_to = false;
 size_t scale_factor = 0;       // -F
 pset<string> dont_compress;    // -Z
 vector_string sign_params;     // -S
-pvector<Filename> cert_chain;  // -N
 
 // Default extensions not to compress.  May be overridden with -Z.
 string dont_compress_str = "jpg,png,mp3,ogg";
@@ -225,21 +224,14 @@ help() {
     "      generate slightly smaller files, but compression takes longer.  The\n"
     "      default is -" << default_compression_level << ".\n\n"
 
-    "  -N ca-chain.crt\n"
-    "      Adds the indicated certificate chain file to the multifile.  This must\n"
-    "      used in conjunction with -c or -u mode.  The indicated file must be\n"
-    "      a PEM-formatted collection of certificates, representing the chain of\n"
-    "      authentication from the certificate used to sign the multifile (-S,\n"
-    "      below), and a certificate authority.  This may be necessary for\n"
-    "      certain certificates whose signing authority is one or more steps\n"
-    "      removed from a well-known certificate authority.  You must add this\n"
-    "      file to the multifile before signing it (or at least at the same time).\n\n"
-
-    "  -S file.crt,file.key[,\"password\"]\n"
+    "  -S file.crt,[chain.crt],file.key[,\"password\"]\n"
     "      Sign the multifile.  The signing certificate should be in PEM form in\n"
     "      file.crt, with its private key in PEM form in file.key.  If the key\n"
     "      is encrypted on-disk, specify the decryption password as the third\n"
-    "      option.  PEM form is the form accepted by the Apache web server.  The\n"
+    "      option.  If a certificate chain is required, chain.crt should also\n"
+    "      be specified; note that the commas should be supplied even if this\n"
+    "      optional filename is omitted.\n"
+    "      PEM form is the form accepted by the Apache web server.  The\n"
     "      signature is written to the multifile to prove it is unchanged; any\n"
     "      subsequent change to the multifile will invalidate the signature.\n"
     "      This parameter may be repeated to sign the multifile with different\n"
@@ -297,12 +289,10 @@ get_compression_level(const Filename &subfile_name) {
 }
 
 bool
-do_add_files(Multifile *multifile, const pvector<Filename> &filenames,
-             bool cert_chain_flag);
+do_add_files(Multifile *multifile, const pvector<Filename> &filenames);
 
 bool
-do_add_directory(Multifile *multifile, const Filename &directory_name,
-                 bool cert_chain_flag) {
+do_add_directory(Multifile *multifile, const Filename &directory_name) {
   vector_string files;
   if (!directory_name.scan_directory(files)) {
     cerr << "Unable to scan directory " << directory_name << "\n";
@@ -317,19 +307,18 @@ do_add_directory(Multifile *multifile, const Filename &directory_name,
     filenames.push_back(subfile_name);
   }
 
-  return do_add_files(multifile, filenames, cert_chain_flag);
+  return do_add_files(multifile, filenames);
 }
 
 bool
-do_add_files(Multifile *multifile, const pvector<Filename> &filenames,
-             bool cert_chain_flag) {
+do_add_files(Multifile *multifile, const pvector<Filename> &filenames) {
   bool okflag = true;
   pvector<Filename>::const_iterator fi;
   for (fi = filenames.begin(); fi != filenames.end(); ++fi) {
     const Filename &subfile_name = (*fi);
 
     if (subfile_name.is_directory()) {
-      if (!do_add_directory(multifile, subfile_name, cert_chain_flag)) {
+      if (!do_add_directory(multifile, subfile_name)) {
         okflag = false;
       }
 
@@ -353,12 +342,6 @@ do_add_files(Multifile *multifile, const pvector<Filename> &filenames,
         if (verbose) {
           cout << new_subfile_name << "\n";
         }
-        if (cert_chain_flag) {
-          // Set the cert_chain flag on the file.
-          int index = multifile->find_subfile(new_subfile_name);
-          nassertr(index >= 0, false);
-          multifile->set_subfile_is_cert_chain(index, true);
-        }
       }
     }
   }
@@ -406,10 +389,7 @@ add_files(const vector_string &params) {
     filenames.push_back(subfile_name);
   }
 
-  bool okflag = do_add_files(multifile, filenames, false);
-  if (okflag && !cert_chain.empty()) {
-    okflag = do_add_files(multifile, cert_chain, true);
-  }
+  bool okflag = do_add_files(multifile, filenames);
 
   if (multifile->needs_repack()) {
     if (!multifile->repack()) {
@@ -551,22 +531,28 @@ sign_multifile() {
     const string &param = (*si);
     size_t comma1 = param.find(',');
     if (comma1 == string::npos) {
-      cerr << "Signing parameter requires a comma: " << param << "\n";
+      cerr << "Signing parameter requires two commas: " << param << "\n";
       return false;
     }
     size_t comma2 = param.find(',', comma1 + 1);
+    if (comma2 == string::npos) {
+      cerr << "Signing parameter requires two commas: " << param << "\n";
+      return false;
+    }
+    size_t comma3 = param.find(',', comma2 + 1);
 
     Filename certificate = Filename::from_os_specific(param.substr(0, comma1));
+    Filename chain = Filename::from_os_specific(param.substr(comma1 + 1, comma2 - comma1 - 1));
     Filename pkey;
     string password;
-    if (comma2 != string::npos) {
-      pkey = Filename::from_os_specific(param.substr(comma1 + 1, comma2 - comma1 - 1));
-      password = param.substr(comma2 + 1);
+    if (comma3 != string::npos) {
+      pkey = Filename::from_os_specific(param.substr(comma2 + 1, comma3 - comma2 - 1));
+      password = param.substr(comma3 + 1);
     } else {
-      pkey = Filename::from_os_specific(param.substr(comma1 + 1));
+      pkey = Filename::from_os_specific(param.substr(comma2 + 1));
     }
 
-    if (!multifile->add_signature(certificate, pkey, password)) {
+    if (!multifile->add_signature(certificate, chain, pkey, password)) {
       return false;
     }
   }    
@@ -629,10 +615,6 @@ list_files(const vector_string &params) {
         if (multifile->is_subfile_encrypted(i)) {
           encrypted_symbol = 'e';
         }
-        char cert_chain_symbol = ' ';
-        if (multifile->get_subfile_is_cert_chain(i)) {
-          cert_chain_symbol = 'N';
-        }
         if (multifile->is_subfile_compressed(i)) {
           size_t orig_length = multifile->get_subfile_length(i);
           size_t internal_length = multifile->get_subfile_internal_length(i);
@@ -641,25 +623,25 @@ list_files(const vector_string &params) {
             ratio = (double)internal_length / (double)orig_length;
           }
           if (ratio > 1.0) {
-            printf("%12d worse %c%c %s %s\n",
+            printf("%12d worse %c %s %s\n",
                    (int)multifile->get_subfile_length(i),
-                   encrypted_symbol, cert_chain_symbol,
+                   encrypted_symbol, 
                    format_timestamp(multifile->get_record_timestamp(),
                                     multifile->get_subfile_timestamp(i)),
                    subfile_name.c_str());
           } else {
-            printf("%12d  %3.0f%% %c%c %s %s\n",
+            printf("%12d  %3.0f%% %c %s %s\n",
                    (int)multifile->get_subfile_length(i),
                    100.0 - ratio * 100.0, 
-                   encrypted_symbol, cert_chain_symbol,
+                   encrypted_symbol, 
                    format_timestamp(multifile->get_record_timestamp(),
                                     multifile->get_subfile_timestamp(i)),
                    subfile_name.c_str());
           }
         } else {
-          printf("%12d       %c%c %s %s\n", 
+          printf("%12d       %c %s %s\n", 
                  (int)multifile->get_subfile_length(i),
-                 encrypted_symbol, cert_chain_symbol,
+                 encrypted_symbol, 
                  format_timestamp(multifile->get_record_timestamp(),
                                   multifile->get_subfile_timestamp(i)),
                  subfile_name.c_str());
@@ -691,7 +673,6 @@ list_files(const vector_string &params) {
 #ifdef HAVE_OPENSSL
   int num_signatures = multifile->get_num_signatures();
   if (num_signatures != 0) {
-    multifile->load_certificate_chains();
     cout << "\n";
     for (i = 0; i < num_signatures; ++i) {
       cout << "Signed by " << multifile->get_signature_common_name(i);
@@ -747,7 +728,7 @@ main(int argc, char *argv[]) {
 
   extern char *optarg;
   extern int optind;
-  static const char *optflags = "crutxkvz123456789Z:T:S:T:f:OC:ep:P:F:h";
+  static const char *optflags = "crutxkvz123456789Z:T:S:f:OC:ep:P:F:h";
   int flag = getopt(argc, argv, optflags);
   Filename rel_path;
   while (flag != EOF) {
@@ -815,9 +796,6 @@ main(int argc, char *argv[]) {
     case 'Z':
       dont_compress_str = optarg;
       break;
-    case 'N':
-      cert_chain.push_back(Filename::from_os_specific(optarg));
-      break;
     case 'S':
       sign_params.push_back(optarg);
       break;

panda/src/express/multifile.I

@@ -520,3 +520,37 @@ get_last_byte_pos() const {
   return max(_index_start + (streampos)_index_length, 
              _data_start + (streampos)_data_length) - (streampos)1;
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: Multifile::CertRecord::Constructor
+//       Access: Public
+//  Description: Ownership of the X509 object is passed into the
+//               CertRecord; it will be freed when the CertRecord
+//               destructs.
+////////////////////////////////////////////////////////////////////
+INLINE Multifile::CertRecord::
+CertRecord(X509 *cert) :
+  _cert(cert)
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Multifile::CertRecord::Copy Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE Multifile::CertRecord::
+CertRecord(const Multifile::CertRecord &copy) :
+  _cert(X509_dup(copy._cert))
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: Multifile::CertRecord::Destructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE Multifile::CertRecord::
+~CertRecord() {
+  X509_free(_cert);
+}

panda/src/express/multifile.cxx

@@ -570,6 +570,10 @@ update_subfile(const string &subfile_name, const Filename &filename,
 //               subsequent changes to the Multifile will
 //               automatically invalidate and remove the signature.
 //
+//               The chain filename may be empty if the certificate
+//               does not require an authenticating certificate chain
+//               (e.g. because it is self-signed).
+//
 //               The specified private key must match the certificate,
 //               and the Multifile must be open in read-write mode.
 //               The private key is only used for generating the
@@ -589,10 +593,12 @@ update_subfile(const string &subfile_name, const Filename &filename,
 //               parameter will be used as the password to decrypt it.
 ////////////////////////////////////////////////////////////////////
 bool Multifile::
-add_signature(const Filename &certificate, const Filename &pkey,
-              const string &password) {
+add_signature(const Filename &certificate, const Filename &chain,
+              const Filename &pkey, const string &password) {
   VirtualFileSystem *vfs = VirtualFileSystem::get_global_ptr();
 
+  CertChain cert_chain;
+
   // Read the certificate file from VFS.  First, read the complete
   // file into memory.
   string certificate_data;
@@ -612,6 +618,34 @@ add_signature(const Filename &certificate, const Filename &pkey,
     return false;
   }
 
+  // Store the first X509--the actual certificate--as the first record
+  // in our CertChain object.
+  cert_chain.push_back(CertRecord(x509));
+
+  // Read the rest of the certificates in the chain file.
+  if (!chain.empty()) {
+    string chain_data;
+    if (!vfs->read_file(chain, chain_data, true)) {
+      express_cat.info()
+        << "Could not read " << chain << ".\n";
+      return false;
+    }
+
+    BIO *chain_mbio = BIO_new_mem_buf((void *)chain_data.data(), chain_data.size());
+    X509 *c = PEM_read_bio_X509(chain_mbio, NULL, NULL, (void *)"");
+    while (c != NULL) {
+      cert_chain.push_back(c);
+      c = PEM_read_bio_X509(chain_mbio, NULL, NULL, (void *)"");
+    }
+    BIO_free(chain_mbio);
+
+    if (cert_chain.size() == 1) {
+      express_cat.info()
+        << "Could not read certificate chain in " << chain << ".\n";
+      return false;
+    }
+  }
+
   // Now do the same thing with the private key.  This one may be
   // password-encrypted on disk.
   string pkey_data;
@@ -621,7 +655,6 @@ add_signature(const Filename &certificate, const Filename &pkey,
     return false;
   }
 
-  // Create an in-memory BIO to read the "file" from the buffer.
   BIO *pkey_mbio = BIO_new_mem_buf((void *)pkey_data.data(), pkey_data.size());
   EVP_PKEY *evp_pkey = PEM_read_bio_PrivateKey(pkey_mbio, NULL, NULL,
                                                (void *)password.c_str());
@@ -629,18 +662,57 @@ add_signature(const Filename &certificate, const Filename &pkey,
   if (evp_pkey == NULL) {
     express_cat.info()
       << "Could not read private key in " << pkey << ".\n";
-    X509_free(x509);
     return false;
   }
 
-  bool result = add_signature(x509, evp_pkey);
+  bool result = add_signature(cert_chain, evp_pkey);
 
   EVP_PKEY_free(evp_pkey);
-  X509_free(x509);
 
   return result;
 }
 #endif  // HAVE_OPENSSL
+#ifdef HAVE_OPENSSL
+////////////////////////////////////////////////////////////////////
+//     Function: Multifile::add_signature
+//       Access: Published
+//  Description: Adds a new signature to the Multifile.  This
+//               signature associates the indicated certificate with
+//               the current contents of the Multifile.  When the
+//               Multifile is read later, the signature will still be
+//               present only if the Multifile is unchanged; any
+//               subsequent changes to the Multifile will
+//               automatically invalidate and remove the signature.
+//
+//               If chain is non-NULL, it represents the certificate
+//               chain that validates the certificate.
+//
+//               The specified private key must match the certificate,
+//               and the Multifile must be open in read-write mode.
+//               The private key is only used for generating the
+//               signature; it is not written to the Multifile and
+//               cannot be retrieved from the Multifile later.
+//               (However, the certificate *can* be retrieved from the
+//               Multifile later, to identify the entity that created
+//               the signature.)
+//
+//               This implicitly causes a repack() operation if one is
+//               needed.  Returns true on success, false on failure.
+////////////////////////////////////////////////////////////////////
+bool Multifile::
+add_signature(X509 *certificate, STACK *chain, EVP_PKEY *pkey) {
+  // Convert the certificate and chain into our own CertChain
+  // structure.
+  CertChain cert_chain;
+  cert_chain.push_back(CertRecord(certificate));
+  if (chain != NULL) {
+    int num = sk_num(chain);
+    for (int i = 0; i < num; ++i) {
+      cert_chain.push_back(CertRecord((X509 *)sk_value(chain, i)));
+    }
+  }
+}
+#endif  // HAVE_OPENSSL
 
 #ifdef HAVE_OPENSSL
 ////////////////////////////////////////////////////////////////////
@@ -654,6 +726,10 @@ add_signature(const Filename &certificate, const Filename &pkey,
 //               subsequent changes to the Multifile will
 //               automatically invalidate and remove the signature.
 //
+//               The signature certificate is the first certificate on
+//               the CertChain object.  Any remaining certificates are
+//               support certificates to authenticate the first one.
+//
 //               The specified private key must match the certificate,
 //               and the Multifile must be open in read-write mode.
 //               The private key is only used for generating the
@@ -667,7 +743,7 @@ add_signature(const Filename &certificate, const Filename &pkey,
 //               needed.  Returns true on success, false on failure.
 ////////////////////////////////////////////////////////////////////
 bool Multifile::
-add_signature(X509 *certificate, EVP_PKEY *pkey) {
+add_signature(const Multifile::CertChain &cert_chain, EVP_PKEY *pkey) {
   if (_needs_repack) {
     if (!repack()) {
       return false;
@@ -677,18 +753,26 @@ add_signature(X509 *certificate, EVP_PKEY *pkey) {
       return false;
     }
   }
+  
+  // Now encode that list of certs to a stream in DER form.
+  stringstream der_stream;
+  StreamWriter der_writer(der_stream);
+  der_writer.add_uint32(cert_chain.size());
 
-  // Encode the certificate into DER form for writing to the
-  // Multifile.
-  int der_len = i2d_X509(certificate, NULL);
-  unsigned char *der_buf = new unsigned char[der_len];
-  unsigned char *p = der_buf;
-  i2d_X509(certificate, &p);
-  string der_string((char *)der_buf, der_len);
-  delete[] der_buf;
-  istringstream der_stream(der_string);
+  CertChain::const_iterator ci;
+  for (ci = cert_chain.begin(); ci != cert_chain.end(); ++ci) {
+    X509 *cert = (*ci)._cert;
+
+    int der_len = i2d_X509(cert, NULL);
+    unsigned char *der_buf = new unsigned char[der_len];
+    unsigned char *p = der_buf;
+    i2d_X509(cert, &p);
+    der_writer.append_data(der_buf, der_len);
+    delete[] der_buf;
+  }
 
   // Create a temporary Subfile for writing out the signature.
+  der_stream.seekg(0);
   Subfile *subfile = new Subfile;
   subfile->_pkey = pkey;
   subfile->_flags |= SF_signature;
@@ -733,9 +817,10 @@ get_num_signatures() const {
 //  Description: Returns the nth signature found on the Multifile.
 //               See the comments in get_num_signatures().
 ////////////////////////////////////////////////////////////////////
-X509 *Multifile::
+const Multifile::CertChain &Multifile::
 get_signature(int n) const {
-  nassertr(n >= 0 && n < (int)_signatures.size(), NULL);
+  static CertChain error_chain;
+  nassertr(n >= 0 && n < (int)_signatures.size(), error_chain);
   return _signatures[n];
 }
 #endif  // HAVE_OPENSSL
@@ -752,10 +837,10 @@ get_signature(int n) const {
 ////////////////////////////////////////////////////////////////////
 string Multifile::
 get_signature_subject_name(int n) const {
-  X509 *x509 = get_signature(n);
-  nassertr(x509 != NULL, "");
+  const CertChain &cert_chain = get_signature(n);
+  nassertr(!cert_chain.empty(), string());
 
-  X509_NAME *xname = X509_get_subject_name(x509);
+  X509_NAME *xname = X509_get_subject_name(cert_chain[0]._cert);
   if (xname != NULL) {
     // We use "print" to dump the output to a memory BIO.  Is
     // there an easier way to extract the X509_NAME text?  Curse
@@ -786,12 +871,12 @@ get_signature_subject_name(int n) const {
 ////////////////////////////////////////////////////////////////////
 string Multifile::
 get_signature_common_name(int n) const {
-  X509 *x509 = get_signature(n);
-  nassertr(x509 != NULL, "");
+  const CertChain &cert_chain = get_signature(n);
+  nassertr(!cert_chain.empty(), string());
 
   // A complex OpenSSL interface to extract out the common name in
   // utf-8.
-  X509_NAME *xname = X509_get_subject_name(x509);
+  X509_NAME *xname = X509_get_subject_name(cert_chain[0]._cert);
   if (xname != NULL) {
     int pos = X509_NAME_get_index_by_NID(xname, NID_commonName, -1);
     if (pos != -1) {
@@ -831,11 +916,11 @@ get_signature_common_name(int n) const {
 ////////////////////////////////////////////////////////////////////
 void Multifile::
 write_signature_certificate(int n, ostream &out) const {
-  X509 *x509 = get_signature(n);
-  nassertv(x509 != NULL);
+  const CertChain &cert_chain = get_signature(n);
+  nassertv(!cert_chain.empty());
 
   BIO *mbio = BIO_new(BIO_s_mem());
-  X509_print(mbio, x509);
+  X509_print(mbio, cert_chain[0]._cert);
 
   char *pp;
   long pp_size = BIO_get_mem_data(mbio, &pp);
@@ -844,49 +929,6 @@ write_signature_certificate(int n, ostream &out) const {
 }
 #endif  // HAVE_OPENSSL
 
-#ifdef HAVE_OPENSSL
-////////////////////////////////////////////////////////////////////
-//     Function: Multifile::load_certificate_chains
-//       Access: Published
-//  Description: Loads any certificate chains specified in the
-//               Multifile into the global certificate store.  This
-//               may be necessary to successfully validate the
-//               certificate used to sign the multifile in
-//               validate_signature_certificate(), below.
-////////////////////////////////////////////////////////////////////
-void Multifile::
-load_certificate_chains() {
-  OpenSSLWrapper *sslw = OpenSSLWrapper::get_global_ptr();
-
-  Subfiles::iterator si;
-  for (si = _subfiles.begin(); si != _subfiles.end(); ++si) {
-    Subfile *subfile = (*si);
-
-    if ((subfile->_flags & SF_cert_chain) != 0) {
-      // If it's a certificate chain, add it to the global chain list.
-
-      // Read the cert chain into memory.
-      istream *stream = open_read_subfile(subfile);
-      nassertv(stream != NULL);
-      pvector<unsigned char> buffer;
-      bool success = read_to_pvector(buffer, *stream);
-      nassertv(success);
-      close_read_subfile(stream);
-
-      // Now it to the global chain.
-      if (!buffer.empty()) {
-        int result = sslw->load_certificates_from_ram((char *)&buffer[0], buffer.size());
-        if (result > 0) {
-          express_cat.info()
-            << "Loaded " << result << " certificates from "
-            << subfile->_name << "\n";
-        }
-      }
-    }
-  }
-}
-#endif // HAVE_OPENSSL
-
 #ifdef HAVE_OPENSSL
 ////////////////////////////////////////////////////////////////////
 //     Function: Multifile::validate_signature_certificate
@@ -902,17 +944,32 @@ int Multifile::
 validate_signature_certificate(int n) const {
   int verify_result = -1;
 
-  X509 *x509 = get_signature(n);
-  nassertr(x509 != NULL, false);
+  const CertChain &chain = get_signature(n);
+  nassertr(!chain.empty(), false);
 
   OpenSSLWrapper *sslw = OpenSSLWrapper::get_global_ptr();
 
+  // Copy our CertChain structure into an X509 pointer and
+  // accompanying STACK pointer.
+  X509 *x509 = chain[0]._cert;
+  STACK *stack = NULL;
+  if (chain.size() > 1) {
+    stack = sk_new(NULL);
+    for (size_t n = 1; n < chain.size(); ++n) {
+      sk_push(stack, (char *)chain[n]._cert);
+    }
+  }
+
+  // Create the X509_STORE_CTX for verifying the cert and chain.
   X509_STORE_CTX *ctx = X509_STORE_CTX_new();
-  X509_STORE_CTX_init(ctx, sslw->get_x509_store(), x509, NULL);
+  X509_STORE_CTX_init(ctx, sslw->get_x509_store(), x509, stack);
   X509_STORE_CTX_set_cert(ctx, x509);
 
-  X509_verify_cert(ctx);
-  verify_result = X509_STORE_CTX_get_error(ctx);
+  if (X509_verify_cert(ctx)) {
+    verify_result = 0;
+  } else {
+    verify_result = X509_STORE_CTX_get_error(ctx);
+  }
 
   if (express_cat.is_debug()) {
     express_cat.debug()
@@ -920,6 +977,7 @@ validate_signature_certificate(int n) const {
       << "\n";
   }
 
+  sk_free(stack);
   X509_STORE_CTX_cleanup(ctx);
   X509_STORE_CTX_free(ctx);
 
@@ -1393,51 +1451,6 @@ is_subfile_encrypted(int index) const {
   return (_subfiles[index]->_flags & SF_encrypted) != 0;
 }
 
-////////////////////////////////////////////////////////////////////
-//     Function: Multifile::set_subfile_is_cert_chain
-//       Access: Published
-//  Description: Sets the cert_chain flag on the indicated subfile.
-//               This should be set for any subfiles that define the
-//               certificate chain that will be necessary to validate
-//               signatures that are subsequently used to sign the
-//               multifile.
-//
-//               Note that the certificate chain subfiles must be
-//               added and this flag set *before* signing the
-//               multifile.
-////////////////////////////////////////////////////////////////////
-void Multifile::
-set_subfile_is_cert_chain(int index, bool flag) {
-  nassertv(is_write_valid());
-  nassertv(index >= 0 && index < (int)_subfiles.size());
-
-  Subfile *subfile = _subfiles[index];
-  bool current_flag = (subfile->_flags & SF_cert_chain) != 0;
-  if (current_flag != flag) {
-    if (flag) {
-      subfile->_flags |= SF_cert_chain;
-    } else {
-      subfile->_flags &= ~SF_cert_chain;
-    }
-    subfile->rewrite_index_flags(*_write);
-  }
-}
-
-////////////////////////////////////////////////////////////////////
-//     Function: Multifile::get_subfile_is_cert_chain
-//       Access: Published
-//  Description: Returns the cert_chain flag on the indicated subfile.
-//               This should be set for any subfiles that define the
-//               certificate chain that will be necessary to validate
-//               signatures that are subsequently used to sign the
-//               multifile.
-////////////////////////////////////////////////////////////////////
-bool Multifile::
-get_subfile_is_cert_chain(int index) const {
-  nassertr(index >= 0 && index < (int)_subfiles.size(), false);
-  return (_subfiles[index]->_flags & SF_cert_chain) != 0;
-}
-
 ////////////////////////////////////////////////////////////////////
 //     Function: Multifile::get_index_end
 //       Access: Published
@@ -2021,13 +2034,6 @@ clear_subfiles() {
     delete subfile;
   }
   _cert_special.clear();
-
-  Certificates::iterator ci;
-  for (ci = _signatures.begin(); ci != _signatures.end(); ++ci) {
-    X509 *cert = (*ci);
-    X509_free(cert);
-  }
-  _signatures.clear();
 #endif  // HAVE_OPENSSL
 
   Subfiles::iterator fi;
@@ -2288,21 +2294,36 @@ check_signatures() {
     StreamReader reader(*stream);
     size_t sig_size = reader.get_uint32();
     string sig_string = reader.extract_bytes(sig_size);
-    
+
+    size_t num_certs = reader.get_uint32();
+
+    // Read the remaining buffer of certificate data.
     pvector<unsigned char> buffer;
     bool success = read_to_pvector(buffer, *stream);
     nassertv(success);
     close_read_subfile(stream);
-    
-    X509 *x509 = NULL;
+
+    // Now convert each of the certificates to an X509 object, and
+    // store it in our CertChain.
+    CertChain chain;
     EVP_PKEY *pkey = NULL;
     if (!buffer.empty()) {
       const unsigned char *bp = (const unsigned char *)&buffer[0];
-      x509 = d2i_X509(NULL, &bp, buffer.size());
+      const unsigned char *bp_end = bp + buffer.size();
+      X509 *x509 = d2i_X509(NULL, &bp, bp_end - bp);
+      while (num_certs > 0 && x509 != NULL) {
+        chain.push_back(CertRecord(x509));
+        --num_certs;
+        x509 = d2i_X509(NULL, &bp, bp_end - bp);
+      }
+      if (num_certs != 0 || x509 != NULL) {
+        express_cat.warning() 
+          << "Extra data in signature record.\n";
+      }
     }
     
-    if (x509 != NULL) {
-      pkey = X509_get_pubkey(x509);
+    if (!chain.empty()) {
+      pkey = X509_get_pubkey(chain[0]._cert);
     }
     
     if (pkey != NULL) {
@@ -2342,20 +2363,13 @@ check_signatures() {
                         (unsigned char *)sig_string.data(), 
                         sig_string.size(), pkey);
       if (verify_result == 1) {
-        // The signature matches; save the certificate.
-        _signatures.push_back(x509);
-        x509 = NULL;
+        // The signature matches; save the certificate and its chain.
+        _signatures.push_back(chain);
       } else {
         // Bad match.
         _needs_repack = true;
       }
     }
-    
-    if (x509 != NULL) {
-      // If we still have the X509 pointer by this point, we haven't
-      // saved it anywhere, so free it now.
-      X509_free(x509);
-    }
   }
 #endif  // HAVE_OPENSSL
 

panda/src/express/multifile.h

@@ -84,17 +84,28 @@ PUBLISHED:
                         int compression_level);
 
 #ifdef HAVE_OPENSSL
-  bool add_signature(const Filename &certificate, const Filename &pkey,
+  class CertRecord {
+  public:
+    INLINE CertRecord(X509 *cert);
+    INLINE CertRecord(const CertRecord &copy);
+    INLINE ~CertRecord();
+    X509 *_cert;
+  };
+  typedef pvector<CertRecord> CertChain;
+
+  bool add_signature(const Filename &certificate, 
+                     const Filename &chain,
+                     const Filename &pkey,
                      const string &password = "");
-  bool add_signature(X509 *certificate, EVP_PKEY *pkey);
+  bool add_signature(X509 *certificate, STACK *chain, EVP_PKEY *pkey);
+  bool add_signature(const CertChain &chain, EVP_PKEY *pkey);
 
   int get_num_signatures() const;
-  X509 *get_signature(int n) const;
+  const CertChain &get_signature(int n) const;
   string get_signature_subject_name(int n) const;
   string get_signature_common_name(int n) const;
   void write_signature_certificate(int n, ostream &out) const;
 
-  void load_certificate_chains();
   int validate_signature_certificate(int n) const;
 #endif  // HAVE_OPENSSL
 
@@ -114,8 +125,6 @@ PUBLISHED:
   time_t get_subfile_timestamp(int index) const;
   bool is_subfile_compressed(int index) const;
   bool is_subfile_encrypted(int index) const;
-  void set_subfile_is_cert_chain(int index, bool flag);
-  bool get_subfile_is_cert_chain(int index) const;
 
   streampos get_index_end() const;
   streampos get_subfile_internal_start(int index) const;
@@ -147,8 +156,7 @@ private:
     SF_data_invalid   = 0x0004,
     SF_compressed     = 0x0008,
     SF_encrypted      = 0x0010,
-    SF_cert_chain     = 0x0020,
-    SF_signature      = 0x0040,
+    SF_signature      = 0x0020,
   };
 
   class Subfile {
@@ -209,7 +217,7 @@ private:
   PendingSubfiles _cert_special;
 
 #ifdef HAVE_OPENSSL
-  typedef pvector<X509 *> Certificates;
+  typedef pvector<CertChain> Certificates;
   Certificates _signatures;
 #endif
 

panda/src/express/openSSLWrapper.cxx

@@ -36,6 +36,7 @@ OpenSSLWrapper() {
   OpenSSL_add_all_algorithms();
 
   _x509_store = X509_STORE_new();
+  X509_STORE_set_default_paths(_x509_store);
 
   // Load in any default certificates listed in the Config.prc file.
   int num_certs = ssl_certificates.get_num_unique_values();
@@ -67,6 +68,10 @@ OpenSSLWrapper::
 //
 //               Returns the number of certificates read on success,
 //               or 0 on failure.
+//
+//               You should call this only with trusted,
+//               locally-stored certificates; not with certificates
+//               received from an untrusted source.
 ////////////////////////////////////////////////////////////////////
 int OpenSSLWrapper::
 load_certificates(const Filename &filename) {
@@ -100,10 +105,14 @@ load_certificates(const Filename &filename) {
 ////////////////////////////////////////////////////////////////////
 //     Function: OpenSSLWrapper::load_certificates_from_ram
 //       Access: Public
-//  Description: Reads a chain of certificates from the indicated
-//               data buffer and adds them to the X509_STORE object.
-//               Returns the number of certificates read on success,
-//               or 0 on failure.
+//  Description: Reads a chain of trusted certificates from the
+//               indicated data buffer and adds them to the X509_STORE
+//               object.  Returns the number of certificates read on
+//               success, or 0 on failure.
+//
+//               You should call this only with trusted,
+//               locally-stored certificates; not with certificates
+//               received from an untrusted source.
 ////////////////////////////////////////////////////////////////////
 int OpenSSLWrapper::
 load_certificates_from_ram(const char *data, size_t data_size) {