Fix use-after-free in error case.
[libdcp.git] / src / certificate_chain.cc
index 0d99d1c920a28083c4bcf748fe3d3dbe822928a6..449dba89e7a07a0bfda61c4db9a66e1952122f48 100644 (file)
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2013-2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2013-2021 Carl Hetherington <cth@carlh.net>
 
     This file is part of libdcp.
 
     files in the program, then also delete it here.
 */
 
-/** @file  src/signer_chain.cc
- *  @brief Functions to make signer chains.
+
+/** @file  src/certificate_chain.cc
+ *  @brief CertificateChain class
  */
 
+
 #include "certificate_chain.h"
+#include "compose.hpp"
+#include "dcp_assert.h"
 #include "exceptions.h"
 #include "util.h"
-#include "dcp_assert.h"
-#include "compose.hpp"
+#include "warnings.h"
 #include <asdcp/KM_util.h>
 #include <libcxml/cxml.h>
+LIBDCP_DISABLE_WARNINGS
 #include <libxml++/libxml++.h>
+LIBDCP_ENABLE_WARNINGS
 #include <xmlsec/xmldsig.h>
 #include <xmlsec/dl.h>
 #include <xmlsec/app.h>
 #include <openssl/rsa.h>
 #include <boost/filesystem.hpp>
 #include <boost/algorithm/string.hpp>
-#include <boost/foreach.hpp>
 #include <fstream>
 #include <iostream>
 
+
 using std::string;
 using std::ofstream;
 using std::ifstream;
 using std::runtime_error;
 using namespace dcp;
 
+
 /** Run a shell command.
  *  @param cmd Command to run (UTF8-encoded).
  */
@@ -75,7 +81,7 @@ command (string cmd)
           is handled correctly.
        */
        int const wn = MultiByteToWideChar (CP_UTF8, 0, cmd.c_str(), -1, 0, 0);
-       wchar_t* buffer = new wchar_t[wn];
+       auto buffer = new wchar_t[wn];
        if (MultiByteToWideChar (CP_UTF8, 0, cmd.c_str(), -1, buffer, wn) == 0) {
                delete[] buffer;
                return;
@@ -112,6 +118,7 @@ command (string cmd)
        }
 }
 
+
 /** Extract a public key from a private key and create a SHA1 digest of it.
  *  @param private_key Private key
  *  @param openssl openssl binary name (or full path if openssl is not on the system path).
@@ -123,12 +130,12 @@ public_key_digest (boost::filesystem::path private_key, boost::filesystem::path
        boost::filesystem::path public_name = private_key.string() + ".public";
 
        /* Create the public key from the private key */
-       command (String::compose("\"%1\" rsa -outform PEM -pubout -in %2 -out %3", openssl.string(), private_key.string(), public_name.string ()));
+       command (String::compose("\"%1\" rsa -outform PEM -pubout -in %2 -out %3", openssl.string(), private_key.string(), public_name.string()));
 
        /* Read in the public key from the file */
 
        string pub;
-       ifstream f (public_name.string().c_str ());
+       ifstream f (public_name.string().c_str());
        if (!f.good ()) {
                throw dcp::MiscError ("public key not found");
        }
@@ -177,8 +184,10 @@ public_key_digest (boost::filesystem::path private_key, boost::filesystem::path
        return dig;
 }
 
+
 CertificateChain::CertificateChain (
        boost::filesystem::path openssl,
+       int validity_in_days,
        string organisation,
        string organisational_unit,
        string root_common_name,
@@ -186,10 +195,10 @@ CertificateChain::CertificateChain (
        string leaf_common_name
        )
 {
-       boost::filesystem::path directory = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path ();
+       auto directory = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path ();
        boost::filesystem::create_directories (directory);
 
-       boost::filesystem::path const cwd = boost::filesystem::current_path ();
+       auto const cwd = boost::filesystem::current_path ();
        boost::filesystem::current_path (directory);
 
        string quoted_openssl = "\"" + openssl.string() + "\"";
@@ -221,9 +230,9 @@ CertificateChain::CertificateChain (
        {
                command (
                        String::compose (
-                               "%1 req -new -x509 -sha256 -config ca.cnf -days 3650 -set_serial 5"
-                               " -subj \"%2\" -key ca.key -outform PEM -out ca.self-signed.pem",
-                               quoted_openssl, ca_subject
+                               "%1 req -new -x509 -sha256 -config ca.cnf -days %2 -set_serial 5"
+                               " -subj \"%3\" -key ca.key -outform PEM -out ca.self-signed.pem",
+                               quoted_openssl, validity_in_days, ca_subject
                                )
                        );
        }
@@ -255,16 +264,18 @@ CertificateChain::CertificateChain (
        {
                command (
                        String::compose (
-                               "%1 req -new -config intermediate.cnf -days 3649 -subj \"%2\" -key intermediate.key -out intermediate.csr",
-                               quoted_openssl, inter_subject
+                               "%1 req -new -config intermediate.cnf -days %2 -subj \"%3\" -key intermediate.key -out intermediate.csr",
+                               quoted_openssl, validity_in_days - 1, inter_subject
                                )
                        );
        }
 
        command (
-               quoted_openssl +
-               " x509 -req -sha256 -days 3649 -CA ca.self-signed.pem -CAkey ca.key -set_serial 6"
-               " -in intermediate.csr -extfile intermediate.cnf -extensions v3_ca -out intermediate.signed.pem"
+               String::compose (
+                       "%1 x509 -req -sha256 -days %2 -CA ca.self-signed.pem -CAkey ca.key -set_serial 6"
+                       " -in intermediate.csr -extfile intermediate.cnf -extensions v3_ca -out intermediate.signed.pem",
+                       quoted_openssl, validity_in_days - 1
+                       )
                );
 
        command (quoted_openssl + " genrsa -out leaf.key 2048");
@@ -294,29 +305,32 @@ CertificateChain::CertificateChain (
        {
                command (
                        String::compose (
-                               "%1 req -new -config leaf.cnf -days 3648 -subj \"%2\" -key leaf.key -outform PEM -out leaf.csr",
-                               quoted_openssl, leaf_subject
+                               "%1 req -new -config leaf.cnf -days %2 -subj \"%3\" -key leaf.key -outform PEM -out leaf.csr",
+                               quoted_openssl, validity_in_days - 2, leaf_subject
                                )
                        );
        }
 
        command (
-               quoted_openssl +
-               " x509 -req -sha256 -days 3648 -CA intermediate.signed.pem -CAkey intermediate.key"
-               " -set_serial 7 -in leaf.csr -extfile leaf.cnf -extensions v3_ca -out leaf.signed.pem"
+               String::compose (
+                       "%1 x509 -req -sha256 -days %2 -CA intermediate.signed.pem -CAkey intermediate.key"
+                       " -set_serial 7 -in leaf.csr -extfile leaf.cnf -extensions v3_ca -out leaf.signed.pem",
+                       quoted_openssl, validity_in_days - 2
+                       )
                );
 
        boost::filesystem::current_path (cwd);
 
-       _certificates.push_back (dcp::Certificate (dcp::file_to_string (directory / "ca.self-signed.pem")));
-       _certificates.push_back (dcp::Certificate (dcp::file_to_string (directory / "intermediate.signed.pem")));
-       _certificates.push_back (dcp::Certificate (dcp::file_to_string (directory / "leaf.signed.pem")));
+       _certificates.push_back (dcp::Certificate(dcp::file_to_string(directory / "ca.self-signed.pem")));
+       _certificates.push_back (dcp::Certificate(dcp::file_to_string(directory / "intermediate.signed.pem")));
+       _certificates.push_back (dcp::Certificate(dcp::file_to_string(directory / "leaf.signed.pem")));
 
        _key = dcp::file_to_string (directory / "leaf.key");
 
        boost::filesystem::remove_all (directory);
 }
 
+
 CertificateChain::CertificateChain (string s)
 {
        while (true) {
@@ -334,62 +348,60 @@ CertificateChain::CertificateChain (string s)
        leaf_to_root ();
 }
 
-/** @return Root certificate */
+
 Certificate
 CertificateChain::root () const
 {
        DCP_ASSERT (!_certificates.empty());
-       return root_to_leaf().front ();
+       return root_to_leaf().front();
 }
 
-/** @return Leaf certificate */
+
 Certificate
 CertificateChain::leaf () const
 {
        DCP_ASSERT (!_certificates.empty());
-       return root_to_leaf().back ();
+       return root_to_leaf().back();
 }
 
-/** @return Certificates in order from leaf to root */
+
 CertificateChain::List
 CertificateChain::leaf_to_root () const
 {
-       List l = root_to_leaf ();
-       l.reverse ();
+       auto l = root_to_leaf ();
+       std::reverse (l.begin(), l.end());
        return l;
 }
 
+
 CertificateChain::List
 CertificateChain::unordered () const
 {
        return _certificates;
 }
 
-/** Add a certificate to the chain.
- *  @param c Certificate to add.
- */
+
 void
 CertificateChain::add (Certificate c)
 {
        _certificates.push_back (c);
 }
 
-/** Remove a certificate from the chain.
- *  @param c Certificate to remove.
- */
+
 void
 CertificateChain::remove (Certificate c)
 {
-       _certificates.remove (c);
+       auto i = std::find(_certificates.begin(), _certificates.end(), c);
+       if (i != _certificates.end()) {
+               _certificates.erase (i);
+       }
 }
 
-/** Remove the i'th certificate in the list, as listed
- *  from root to leaf.
- */
+
 void
 CertificateChain::remove (int i)
 {
-       List::iterator j = _certificates.begin ();
+       auto j = _certificates.begin ();
         while (j != _certificates.end () && i > 0) {
                --i;
                ++j;
@@ -400,19 +412,21 @@ CertificateChain::remove (int i)
        }
 }
 
+
 bool
 CertificateChain::chain_valid () const
 {
        return chain_valid (_certificates);
 }
 
-/** Check to see if a chain is valid (i.e. root signs the intermediate, intermediate
- *  signs the leaf and so on) and that the private key (if there is one) matches the
- *  leaf certificate.
- *  @return true if it's ok, false if not.
+
+/** @param error if non-null, filled with an error if a certificate in the list has a
+ *  a problem.
+ *  @return true if all the given certificates verify OK, and are in the correct order in the list
+ *  (root to leaf).  false if any certificate has a problem, or the order is wrong.
  */
 bool
-CertificateChain::chain_valid (List const & chain) const
+CertificateChain::chain_valid(List const & chain, string* error) const
 {
         /* Here I am taking a chain of certificates A/B/C/D and checking validity of B wrt A,
           C wrt B and D wrt C.  It also appears necessary to check the issuer of B/C/D matches
@@ -421,29 +435,29 @@ CertificateChain::chain_valid (List const & chain) const
           any time soon.
        */
 
-       X509_STORE* store = X509_STORE_new ();
+       auto store = X509_STORE_new ();
        if (!store) {
                throw MiscError ("could not create X509 store");
        }
 
        /* Put all the certificates into the store */
-       for (List::const_iterator i = chain.begin(); i != chain.end(); ++i) {
-               if (!X509_STORE_add_cert (store, i->x509 ())) {
-                       X509_STORE_free (store);
+       for (auto const& i: chain) {
+               if (!X509_STORE_add_cert(store, i.x509())) {
+                       X509_STORE_free(store);
                        return false;
                }
        }
 
        /* Verify each one */
-       for (List::const_iterator i = chain.begin(); i != chain.end(); ++i) {
+       for (auto i = chain.begin(); i != chain.end(); ++i) {
 
-               List::const_iterator j = i;
+               auto j = i;
                ++j;
                if (j == chain.end ()) {
                        break;
                }
 
-               X509_STORE_CTX* ctx = X509_STORE_CTX_new ();
+               auto ctx = X509_STORE_CTX_new ();
                if (!ctx) {
                        X509_STORE_free (store);
                        throw MiscError ("could not create X509 store context");
@@ -457,13 +471,18 @@ CertificateChain::chain_valid (List const & chain) const
                }
 
                int const v = X509_verify_cert (ctx);
-               X509_STORE_CTX_free (ctx);
 
                if (v != 1) {
                        X509_STORE_free (store);
+                       if (error) {
+                               *error = X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx));
+                       }
+                       X509_STORE_CTX_free(ctx);
                        return false;
                }
 
+               X509_STORE_CTX_free(ctx);
+
                /* I don't know why OpenSSL doesn't check this stuff
                   in verify_cert, but without these checks the
                   certificates_validation8 test fails.
@@ -480,9 +499,7 @@ CertificateChain::chain_valid (List const & chain) const
        return true;
 }
 
-/** Check that there is a valid private key for the leaf certificate.
- *  Will return true if there are no certificates.
- */
+
 bool
 CertificateChain::private_key_valid () const
 {
@@ -494,19 +511,26 @@ CertificateChain::private_key_valid () const
                return false;
        }
 
-       BIO* bio = BIO_new_mem_buf (const_cast<char *> (_key->c_str ()), -1);
+       auto bio = BIO_new_mem_buf (const_cast<char *> (_key->c_str ()), -1);
        if (!bio) {
                throw MiscError ("could not create memory BIO");
        }
 
-       RSA* private_key = PEM_read_bio_RSAPrivateKey (bio, 0, 0, 0);
-       RSA* public_key = leaf().public_key ();
+       auto private_key = PEM_read_bio_RSAPrivateKey (bio, 0, 0, 0);
+       if (!private_key) {
+               return false;
+       }
+
+       auto public_key = leaf().public_key ();
 
 #if OPENSSL_VERSION_NUMBER > 0x10100000L
        BIGNUM const * private_key_n;
        RSA_get0_key(private_key, &private_key_n, 0, 0);
        BIGNUM const * public_key_n;
        RSA_get0_key(public_key, &public_key_n, 0, 0);
+       if (!private_key_n || !public_key_n) {
+               return false;
+       }
        bool const valid = !BN_cmp (private_key_n, public_key_n);
 #else
        bool const valid = !BN_cmp (private_key->n, public_key->n);
@@ -516,6 +540,7 @@ CertificateChain::private_key_valid () const
        return valid;
 }
 
+
 bool
 CertificateChain::valid (string* reason) const
 {
@@ -538,35 +563,33 @@ CertificateChain::valid (string* reason) const
        return true;
 }
 
-/** @return Certificates in order from root to leaf */
+
 CertificateChain::List
 CertificateChain::root_to_leaf () const
 {
-       List rtl = _certificates;
-       rtl.sort ();
+       auto rtl = _certificates;
+       std::sort (rtl.begin(), rtl.end());
+       string error;
        do {
-               if (chain_valid (rtl)) {
+               if (chain_valid(rtl, &error)) {
                        return rtl;
                }
        } while (std::next_permutation (rtl.begin(), rtl.end()));
 
-       throw CertificateChainError ("certificate chain is not consistent");
+       throw CertificateChainError(error.empty() ? string{"certificate chain is not consistent"} : error);
 }
 
-/** Add a &lt;Signer&gt; and &lt;ds:Signature&gt; nodes to an XML node.
- *  @param parent XML node to add to.
- *  @param standard INTEROP or SMPTE.
- */
+
 void
 CertificateChain::sign (xmlpp::Element* parent, Standard standard) const
 {
        /* <Signer> */
 
        parent->add_child_text("  ");
-       xmlpp::Element* signer = parent->add_child("Signer");
+       auto signer = parent->add_child("Signer");
        signer->set_namespace_declaration ("http://www.w3.org/2000/09/xmldsig#", "dsig");
-       xmlpp::Element* data = signer->add_child("X509Data", "dsig");
-       xmlpp::Element* serial_element = data->add_child("X509IssuerSerial", "dsig");
+       auto data = signer->add_child("X509Data", "dsig");
+       auto serial_element = data->add_child("X509IssuerSerial", "dsig");
        serial_element->add_child("X509IssuerName", "dsig")->add_child_text (leaf().issuer());
        serial_element->add_child("X509SerialNumber", "dsig")->add_child_text (leaf().serial());
        data->add_child("X509SubjectName", "dsig")->add_child_text (leaf().subject());
@@ -576,24 +599,24 @@ CertificateChain::sign (xmlpp::Element* parent, Standard standard) const
        /* <Signature> */
 
        parent->add_child_text("\n  ");
-       xmlpp::Element* signature = parent->add_child("Signature");
+       auto signature = parent->add_child("Signature");
        signature->set_namespace_declaration ("http://www.w3.org/2000/09/xmldsig#", "dsig");
        signature->set_namespace ("dsig");
        parent->add_child_text("\n");
 
-       xmlpp::Element* signed_info = signature->add_child ("SignedInfo", "dsig");
+       auto signed_info = signature->add_child ("SignedInfo", "dsig");
        signed_info->add_child("CanonicalizationMethod", "dsig")->set_attribute ("Algorithm", "http://www.w3.org/TR/2001/REC-xml-c14n-20010315");
 
-       if (standard == INTEROP) {
+       if (standard == Standard::INTEROP) {
                signed_info->add_child("SignatureMethod", "dsig")->set_attribute("Algorithm", "http://www.w3.org/2000/09/xmldsig#rsa-sha1");
        } else {
                signed_info->add_child("SignatureMethod", "dsig")->set_attribute("Algorithm", "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256");
        }
 
-       xmlpp::Element* reference = signed_info->add_child("Reference", "dsig");
+       auto reference = signed_info->add_child("Reference", "dsig");
        reference->set_attribute ("URI", "");
 
-       xmlpp::Element* transforms = reference->add_child("Transforms", "dsig");
+       auto transforms = reference->add_child("Transforms", "dsig");
        transforms->add_child("Transform", "dsig")->set_attribute (
                "Algorithm", "http://www.w3.org/2000/09/xmldsig#enveloped-signature"
                );
@@ -608,23 +631,18 @@ CertificateChain::sign (xmlpp::Element* parent, Standard standard) const
 }
 
 
-/** Sign an XML node.
- *
- *  @param parent Node to sign.
- *  @param ns Namespace to use for the signature XML nodes.
- */
 void
 CertificateChain::add_signature_value (xmlpp::Element* parent, string ns, bool add_indentation) const
 {
        cxml::Node cp (parent);
-       xmlpp::Node* key_info = cp.node_child("KeyInfo")->node ();
+       auto key_info = cp.node_child("KeyInfo")->node();
 
        /* Add the certificate chain to the KeyInfo child node of parent */
-       BOOST_FOREACH (Certificate const & i, leaf_to_root ()) {
-               xmlpp::Element* data = key_info->add_child("X509Data", ns);
+       for (auto const& i: leaf_to_root()) {
+               auto data = key_info->add_child("X509Data", ns);
 
                {
-                       xmlpp::Element* serial = data->add_child("X509IssuerSerial", ns);
+                       auto serial = data->add_child("X509IssuerSerial", ns);
                        serial->add_child("X509IssuerName", ns)->add_child_text (i.issuer ());
                        serial->add_child("X509SerialNumber", ns)->add_child_text (i.serial ());
                }
@@ -632,7 +650,7 @@ CertificateChain::add_signature_value (xmlpp::Element* parent, string ns, bool a
                data->add_child("X509Certificate", ns)->add_child_text (i.certificate());
        }
 
-       xmlSecDSigCtxPtr signature_context = xmlSecDSigCtxCreate (0);
+       auto signature_context = xmlSecDSigCtxCreate (0);
        if (signature_context == 0) {
                throw MiscError ("could not create signature context");
        }
@@ -656,11 +674,12 @@ CertificateChain::add_signature_value (xmlpp::Element* parent, string ns, bool a
        xmlSecDSigCtxDestroy (signature_context);
 }
 
+
 string
 CertificateChain::chain () const
 {
        string o;
-       BOOST_FOREACH (Certificate const &i, root_to_leaf ()) {
+       for (auto const& i: root_to_leaf()) {
                o += i.certificate(true);
        }