Improve management of certificate chains to make it harder to have

an inconsistent chain / private key set.

diff --git a/cscript b/cscript
index 8a4e605ca22457241296d185b700a3f79c3ef6a1..6fdeb8736899741dcedbfa52386daddc9f9fa9ba 100644 (file)
--- a/cscript
+++ b/cscript
@@ -280,8 +280,8 @@ def dependencies(target):
         ffmpeg_options = {}
 
     return (('ffmpeg-cdist', 'd993f38', ffmpeg_options),
-            ('libdcp', 'c871326'),
-            ('libsub', 'ce09e4e'),
+            ('libdcp', '284f247'),
+            ('libsub', '424041f'),
             ('rtaudio-cdist', None))
 
 def configure_options(target):

diff --git a/src/lib/config.cc b/src/lib/config.cc
index c2c6592cb9f48c3228020ee8526e7070ea42be1c..7b2e29f15ce065a1e9b366a4897256ab943f8351 100644 (file)
--- a/src/lib/config.cc
+++ b/src/lib/config.cc
@@ -194,12 +194,11 @@ try
        boost::optional<bool> u = f.optional_bool_child ("UseAnyServers");
        _use_any_servers = u.get_value_or (true);
 
-       list<cxml::NodePtr> servers = f.node_children ("Server");
-       for (list<cxml::NodePtr>::iterator i = servers.begin(); i != servers.end(); ++i) {
-               if ((*i)->node_children("HostName").size() == 1) {
-                       _servers.push_back ((*i)->string_child ("HostName"));
+       BOOST_FOREACH (cxml::ConstNodePtr i, f.node_children("Server")) {
+               if (i->node_children("HostName").size() == 1) {
+                       _servers.push_back (i->string_child ("HostName"));
                } else {
-                       _servers.push_back ((*i)->content ());
+                       _servers.push_back (i->content ());
                }
        }
 
@@ -289,8 +288,8 @@ try
 #endif
 
        list<cxml::NodePtr> his = f.node_children ("History");
-       for (list<cxml::NodePtr>::const_iterator i = his.begin(); i != his.end(); ++i) {
-               _history.push_back ((*i)->content ());
+       BOOST_FOREACH (cxml::ConstNodePtr i, f.node_children("History")) {
+               _history.push_back (i->content ());
        }
 
        cxml::NodePtr signer = f.optional_node_child ("Signer");
@@ -434,8 +433,8 @@ Config::write_config () const
        root->add_child("ServerPortBase")->add_child_text (raw_convert<string> (_server_port_base));
        root->add_child("UseAnyServers")->add_child_text (_use_any_servers ? "1" : "0");
 
-       for (vector<string>::const_iterator i = _servers.begin(); i != _servers.end(); ++i) {
-               root->add_child("Server")->add_child_text (*i);
+       BOOST_FOREACH (string i, _servers) {
+               root->add_child("Server")->add_child_text (i);
        }
 
        root->add_child("OnlyServersEncode")->add_child_text (_only_servers_encode ? "1" : "0");
@@ -498,20 +497,20 @@ Config::write_config () const
 
        xmlpp::Element* signer = root->add_child ("Signer");
        DCPOMATIC_ASSERT (_signer_chain);
-       BOOST_FOREACH (dcp::Certificate const & i, _signer_chain->root_to_leaf ()) {
+       BOOST_FOREACH (dcp::Certificate const & i, _signer_chain->unordered()) {
                signer->add_child("Certificate")->add_child_text (i.certificate (true));
        }
        signer->add_child("PrivateKey")->add_child_text (_signer_chain->key().get ());
 
        xmlpp::Element* decryption = root->add_child ("Decryption");
        DCPOMATIC_ASSERT (_decryption_chain);
-       BOOST_FOREACH (dcp::Certificate const & i, _decryption_chain->root_to_leaf ()) {
+       BOOST_FOREACH (dcp::Certificate const & i, _decryption_chain->unordered()) {
                decryption->add_child("Certificate")->add_child_text (i.certificate (true));
        }
        decryption->add_child("PrivateKey")->add_child_text (_decryption_chain->key().get ());
 
-       for (vector<boost::filesystem::path>::const_iterator i = _history.begin(); i != _history.end(); ++i) {
-               root->add_child("History")->add_child_text (i->string ());
+       BOOST_FOREACH (boost::filesystem::path i, _history) {
+               root->add_child("History")->add_child_text (i.string ());
        }
 
        _dkdms->as_xml (root);
@@ -551,8 +550,8 @@ Config::write_cinemas () const
        xmlpp::Element* root = doc.create_root_node ("Cinemas");
        root->add_child("Version")->add_child_text ("1");
 
-       for (list<shared_ptr<Cinema> >::const_iterator i = _cinemas.begin(); i != _cinemas.end(); ++i) {
-               (*i)->as_xml (root->add_child ("Cinema"));
+       BOOST_FOREACH (shared_ptr<Cinema> i, _cinemas) {
+               i->as_xml (root->add_child ("Cinema"));
        }
 
        try {
@@ -664,12 +663,12 @@ Config::read_cinemas (cxml::Document const & f)
 {
        _cinemas.clear ();
        list<cxml::NodePtr> cin = f.node_children ("Cinema");
-       for (list<cxml::NodePtr>::iterator i = cin.begin(); i != cin.end(); ++i) {
+       BOOST_FOREACH (cxml::ConstNodePtr i, f.node_children("Cinema")) {
                /* Slightly grotty two-part construction of Cinema here so that we can use
                   shared_from_this.
                */
-               shared_ptr<Cinema> cinema (new Cinema (*i));
-               cinema->read_screens (*i);
+               shared_ptr<Cinema> cinema (new Cinema (i));
+               cinema->read_screens (i);
                _cinemas.push_back (cinema);
        }
 }

diff --git a/src/wx/config_dialog.cc b/src/wx/config_dialog.cc
index a62164896f9369ce4b611a920ecb5f8aa86f6336..796972384e6a84c39ba2ccf3899ab9b2c0655a85 100644 (file)
--- a/src/wx/config_dialog.cc
+++ b/src/wx/config_dialog.cc
@@ -881,6 +881,13 @@ public:
                table->Add (_button_sizer, wxGBPosition (r, 0), wxGBSpan (1, 4));
                ++r;
 
+               _private_key_bad = new wxStaticText (this, wxID_ANY, _("Leaf private key does not match leaf certificate!"));
+               font = *wxSMALL_FONT;
+               font.SetWeight (wxFONTWEIGHT_BOLD);
+               _private_key_bad->SetFont (font);
+               table->Add (_private_key_bad, wxGBPosition (r, 0), wxGBSpan (1, 3));
+               ++r;
+
                _add_certificate->Bind     (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::add_certificate, this));
                _remove_certificate->Bind  (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::remove_certificate, this));
                _export_certificate->Bind  (wxEVT_BUTTON,       boost::bind (&CertificateChainEditor::export_certificate, this));
@@ -916,7 +923,15 @@ private:
                if (d->ShowModal() == wxID_OK) {
                        try {
                                dcp::Certificate c;
-                               string const extra = c.read_string (dcp::file_to_string (wx_to_std (d->GetPath ())));
+                               string extra;
+                               try {
+                                       extra = c.read_string (dcp::file_to_string (wx_to_std (d->GetPath ())));
+                               } catch (boost::filesystem::filesystem_error& e) {
+                                       error_dialog (this, wxString::Format (_("Could not load certificate (%s)"), d->GetPath().data()));
+                                       d->Destroy ();
+                                       return;
+                               }
+
                                if (!extra.empty ()) {
                                        message_dialog (
                                                this,
@@ -925,8 +940,17 @@ private:
                                                );
                                }
                                _chain->add (c);
-                               _set (_chain);
-                               update_certificate_list ();
+                               if (!_chain->chain_valid ()) {
+                                       error_dialog (
+                                               this,
+                                               _("Adding this certificate would make the chain inconsistent, so it will not be added. "
+                                                 "Add certificates in order from root to intermediate to leaf.")
+                                               );
+                                       _chain->remove (c);
+                               } else {
+                                       _set (_chain);
+                                       update_certificate_list ();
+                               }
                        } catch (dcp::MiscError& e) {
                                error_dialog (this, wxString::Format (_("Could not read certificate file (%s)"), e.what ()));
                        }
@@ -1003,6 +1027,16 @@ private:
 
                        ++n;
                }
+
+               static wxColour normal = _private_key_bad->GetForegroundColour ();
+
+               if (_chain->private_key_valid ()) {
+                       _private_key_bad->Hide ();
+                       _private_key_bad->SetForegroundColour (normal);
+               } else {
+                       _private_key_bad->Show ();
+                       _private_key_bad->SetForegroundColour (wxColour (255, 0, 0));
+               }
        }
 
        void remake_certificates ()
@@ -1069,7 +1103,8 @@ private:
 
        void update_sensitivity ()
        {
-               _remove_certificate->Enable (_certificates->GetNextItem (-1, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED) != -1);
+               /* We can only remove the leaf certificate */
+               _remove_certificate->Enable (_certificates->GetNextItem (-1, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED) == (_certificates->GetItemCount() - 1));
                _export_certificate->Enable (_certificates->GetNextItem (-1, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED) != -1);
        }
 
@@ -1140,6 +1175,7 @@ private:
        wxStaticText* _private_key;
        wxButton* _load_private_key;
        wxButton* _export_private_key;
+       wxStaticText* _private_key_bad;
        wxSizer* _sizer;
        wxBoxSizer* _button_sizer;
        shared_ptr<dcp::CertificateChain> _chain;