From 104c27f20cf80f29ed8a572b1639d8eae5cbc164 Mon Sep 17 00:00:00 2001 From: David Rose Date: Sun, 6 Sep 2009 21:05:06 +0000 Subject: [PATCH] correct handling of certificate chains --- panda/src/downloadertools/multify.cxx | 82 +++---- panda/src/express/multifile.I | 34 +++ panda/src/express/multifile.cxx | 298 ++++++++++++++------------ panda/src/express/multifile.h | 26 ++- panda/src/express/openSSLWrapper.cxx | 17 +- 5 files changed, 250 insertions(+), 207 deletions(-) diff --git a/panda/src/downloadertools/multify.cxx b/panda/src/downloadertools/multify.cxx index 9935f5e0a8..c397b416a6 100644 --- a/panda/src/downloadertools/multify.cxx +++ b/panda/src/downloadertools/multify.cxx @@ -49,7 +49,6 @@ bool got_chdir_to = false; size_t scale_factor = 0; // -F pset dont_compress; // -Z vector_string sign_params; // -S -pvector 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 &filenames, - bool cert_chain_flag); +do_add_files(Multifile *multifile, const pvector &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 &filenames, - bool cert_chain_flag) { +do_add_files(Multifile *multifile, const pvector &filenames) { bool okflag = true; pvector::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 &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 ¶ms) { 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 ¶m = (*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 ¶ms) { 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 ¶ms) { 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 ¶ms) { #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; diff --git a/panda/src/express/multifile.I b/panda/src/express/multifile.I index 4ff32dc915..c43766723a 100644 --- a/panda/src/express/multifile.I +++ b/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 ©) : + _cert(X509_dup(copy._cert)) +{ +} + +//////////////////////////////////////////////////////////////////// +// Function: Multifile::CertRecord::Destructor +// Access: Public +// Description: +//////////////////////////////////////////////////////////////////// +INLINE Multifile::CertRecord:: +~CertRecord() { + X509_free(_cert); +} diff --git a/panda/src/express/multifile.cxx b/panda/src/express/multifile.cxx index cede3b03fa..af2d0c27ef 100644 --- a/panda/src/express/multifile.cxx +++ b/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 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 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 diff --git a/panda/src/express/multifile.h b/panda/src/express/multifile.h index 3951031b08..837f4bbe08 100644 --- a/panda/src/express/multifile.h +++ b/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 ©); + INLINE ~CertRecord(); + X509 *_cert; + }; + typedef pvector 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 Certificates; + typedef pvector Certificates; Certificates _signatures; #endif diff --git a/panda/src/express/openSSLWrapper.cxx b/panda/src/express/openSSLWrapper.cxx index f8de7731c8..8f52d8684a 100644 --- a/panda/src/express/openSSLWrapper.cxx +++ b/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) {