From: Carl Hetherington Date: Mon, 14 Feb 2022 10:00:52 +0000 (+0100) Subject: Add a hint about certificate validity, moving some things around X-Git-Tag: v2.16.1~3 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=bdf29b6d0a2cdb392a9d4fd1c214ff108ec84c90;hp=8b1736a39f256b37f75bdb99e69298992d3e1083 Add a hint about certificate validity, moving some things around so that it's easier for the user to re-make the certificates if they need to. --- diff --git a/src/lib/config.cc b/src/lib/config.cc index 371682966..bab15ecb7 100644 --- a/src/lib/config.cc +++ b/src/lib/config.cc @@ -450,25 +450,7 @@ try } } - optional bad; - - for (auto const& i: _signer_chain->unordered()) { - if (i.has_utf8_strings()) { - bad = BAD_SIGNER_UTF8_STRINGS; - } - if ((i.not_after().year() - i.not_before().year()) > 15) { - bad = BAD_SIGNER_VALIDITY_TOO_LONG; - } - } - - if (!_signer_chain->chain_valid() || !_signer_chain->private_key_valid()) { - bad = BAD_SIGNER_INCONSISTENT; - } - - if (!_decryption_chain->chain_valid() || !_decryption_chain->private_key_valid()) { - bad = BAD_DECRYPTION_INCONSISTENT; - } - + auto bad = check_certificates (); if (bad) { auto const remake = Bad(*bad); if (remake && *remake) { @@ -1470,3 +1452,29 @@ Config::add_custom_language (dcp::LanguageTag tag) } } + +optional +Config::check_certificates () const +{ + optional bad; + + for (auto const& i: _signer_chain->unordered()) { + if (i.has_utf8_strings()) { + bad = BAD_SIGNER_UTF8_STRINGS; + } + if ((i.not_after().year() - i.not_before().year()) > 15) { + bad = BAD_SIGNER_VALIDITY_TOO_LONG; + } + } + + if (!_signer_chain->chain_valid() || !_signer_chain->private_key_valid()) { + bad = BAD_SIGNER_INCONSISTENT; + } + + if (!_decryption_chain->chain_valid() || !_decryption_chain->private_key_valid()) { + bad = BAD_DECRYPTION_INCONSISTENT; + } + + return bad; +} + diff --git a/src/lib/config.h b/src/lib/config.h index be4e6ecf1..e01eab780 100644 --- a/src/lib/config.h +++ b/src/lib/config.h @@ -1084,6 +1084,8 @@ public: void rename_template (std::string old_name, std::string new_name) const; void delete_template (std::string name) const; + boost::optional check_certificates () const; + static Config* instance (); static void drop (); static void restore_defaults (); diff --git a/src/lib/hints.cc b/src/lib/hints.cc index 40b51e817..0f1cfece8 100644 --- a/src/lib/hints.cc +++ b/src/lib/hints.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2016-2021 Carl Hetherington + Copyright (C) 2016-2022 Carl Hetherington This file is part of DCP-o-matic. @@ -23,6 +23,7 @@ #include "audio_content.h" #include "audio_processor.h" #include "compose.hpp" +#include "config.h" #include "content.h" #include "cross.h" #include "dcp_content_type.h" @@ -381,6 +382,7 @@ try auto content = film->content (); + check_certificates (); check_interop (); check_big_font_files (); check_few_audio_channels (); @@ -652,3 +654,31 @@ Hints::check_audio_language () } } + +void +Hints::check_certificates () +{ + auto bad = Config::instance()->check_certificates(); + if (!bad) { + return; + } + + switch (*bad) { + case Config::BAD_SIGNER_UTF8_STRINGS: + hint(_("The certificate chain that DCP-o-matic uses for signing DCPs and KDMs contains a small error " + "which will prevent DCPs from being validated correctly on some systems. You are advised to " + "re-create the signing certificate chain by clicking the \"Re-make certificates and key...\" " + "button in the Keys page of Preferences.")); + break; + case Config::BAD_SIGNER_VALIDITY_TOO_LONG: + hint(_("The certificate chain that DCP-o-matic uses for signing DCPs and KDMs has a validity period " + "that is too long. This will cause problems playing back DCPs on some systems. " + "You are advised to re-create the signing certificate chain by clicking the " + "\"Re-make certificates and key...\" button in the Keys page of Preferences.")); + break; + default: + /* Some bad situations can't happen here as DCP-o-matic would have refused to start until they are fixed */ + break; + } +} + diff --git a/src/lib/hints.h b/src/lib/hints.h index 6553593a0..985fa1910 100644 --- a/src/lib/hints.h +++ b/src/lib/hints.h @@ -65,6 +65,7 @@ private: void closed_caption (PlayerText text, dcpomatic::DCPTimePeriod period); void open_subtitle (PlayerText text, dcpomatic::DCPTimePeriod period); + void check_certificates (); void check_interop (); void check_big_font_files (); void check_few_audio_channels (); diff --git a/src/wx/config_dialog.cc b/src/wx/config_dialog.cc index c5879d3bb..f91ebe670 100644 --- a/src/wx/config_dialog.cc +++ b/src/wx/config_dialog.cc @@ -536,62 +536,15 @@ CertificateChainEditor::update_certificate_list () void CertificateChainEditor::remake_certificates () { - auto chain = _get(); - - string subject_organization_name; - string subject_organizational_unit_name; - string root_common_name; - string intermediate_common_name; - string leaf_common_name; - - auto all = chain->root_to_leaf (); - - if (all.size() >= 1) { - /* Have a root */ - subject_organization_name = chain->root().subject_organization_name (); - subject_organizational_unit_name = chain->root().subject_organizational_unit_name (); - root_common_name = chain->root().subject_common_name (); - } - - if (all.size() >= 2) { - /* Have a leaf */ - leaf_common_name = chain->leaf().subject_common_name (); - } - - if (all.size() >= 3) { - /* Have an intermediate */ - dcp::CertificateChain::List::iterator i = all.begin (); - ++i; - intermediate_common_name = i->subject_common_name (); - } - if (_nag_alter()) { /* Cancel was clicked */ return; } - auto d = new MakeChainDialog ( - this, - subject_organization_name, - subject_organizational_unit_name, - root_common_name, - intermediate_common_name, - leaf_common_name - ); + auto d = new MakeChainDialog (this, _get()); if (d->ShowModal () == wxID_OK) { - _set ( - make_shared ( - openssl_path (), - CERTIFICATE_VALIDITY_PERIOD, - d->organisation (), - d->organisational_unit (), - d->root_common_name (), - d->intermediate_common_name (), - d->leaf_common_name () - ) - ); - + _set (d->get()); update_certificate_list (); update_private_key (); } @@ -694,18 +647,18 @@ KeysPage::setup () sizer->Add (m, 0, wxALL | wxEXPAND, _border); } - auto buttons = new wxBoxSizer (wxVERTICAL); + auto kdm_buttons = new wxBoxSizer (wxVERTICAL); auto export_decryption_certificate = new Button (_panel, _("Export KDM decryption leaf certificate...")); - buttons->Add (export_decryption_certificate, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); + kdm_buttons->Add (export_decryption_certificate, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); auto export_settings = new Button (_panel, _("Export all KDM decryption settings...")); - buttons->Add (export_settings, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); + kdm_buttons->Add (export_settings, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); auto import_settings = new Button (_panel, _("Import all KDM decryption settings...")); - buttons->Add (import_settings, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); + kdm_buttons->Add (import_settings, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); auto decryption_advanced = new Button (_panel, _("Advanced...")); - buttons->Add (decryption_advanced, 0); + kdm_buttons->Add (decryption_advanced, 0); - sizer->Add (buttons, 0, wxLEFT, _border); + sizer->Add (kdm_buttons, 0, wxLEFT, _border); export_decryption_certificate->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_certificate, this)); export_settings->Bind (wxEVT_BUTTON, bind (&KeysPage::export_decryption_chain_and_key, this)); @@ -718,11 +671,31 @@ KeysPage::setup () sizer->Add (m, 0, wxALL | wxEXPAND, _border); } + auto signing_buttons = new wxBoxSizer (wxVERTICAL); + auto signing_advanced = new Button (_panel, _("Advanced...")); - sizer->Add (signing_advanced, 0, wxLEFT | wxBOTTOM, _border); + signing_buttons->Add (signing_advanced, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); + auto remake_signing = new Button (_panel, _("Re-make certificates and key...")); + signing_buttons->Add (remake_signing, 0, wxBOTTOM, DCPOMATIC_BUTTON_STACK_GAP); + + sizer->Add (signing_buttons, 0, wxLEFT, _border); + signing_advanced->Bind (wxEVT_BUTTON, bind (&KeysPage::signing_advanced, this)); + remake_signing->Bind (wxEVT_BUTTON, bind(&KeysPage::remake_signing, this)); } + +void +KeysPage::remake_signing () +{ + auto d = new MakeChainDialog (_panel, Config::instance()->signer_chain()); + + if (d->ShowModal () == wxID_OK) { + Config::instance()->set_signer_chain(d->get()); + } +} + + void KeysPage::decryption_advanced () { diff --git a/src/wx/config_dialog.h b/src/wx/config_dialog.h index 992490927..b7f3a269f 100644 --- a/src/wx/config_dialog.h +++ b/src/wx/config_dialog.h @@ -176,6 +176,7 @@ private: void signing_advanced (); void export_decryption_chain_and_key (); void import_decryption_chain_and_key (); + void remake_signing (); }; diff --git a/src/wx/make_chain_dialog.cc b/src/wx/make_chain_dialog.cc index d681c00e8..4255fb307 100644 --- a/src/wx/make_chain_dialog.cc +++ b/src/wx/make_chain_dialog.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2014-2018 Carl Hetherington + Copyright (C) 2014-2022 Carl Hetherington This file is part of DCP-o-matic. @@ -18,22 +18,53 @@ */ + #include "make_chain_dialog.h" #include "static_text.h" +#include "lib/cross.h" +#include "lib/util.h" +#include #include + +using std::make_shared; +using std::shared_ptr; using std::string; + MakeChainDialog::MakeChainDialog ( wxWindow* parent, - string organisation, - string organisational_unit_name, - string root_common_name, - string intermediate_common_name, - string leaf_common_name + shared_ptr chain ) : TableDialog (parent, _("Make certificate chain"), 2, 1, true) { + string subject_organization_name; + string subject_organizational_unit_name; + string root_common_name; + string intermediate_common_name; + string leaf_common_name; + + auto all = chain->root_to_leaf (); + + if (all.size() >= 1) { + /* Have a root */ + subject_organization_name = chain->root().subject_organization_name (); + subject_organizational_unit_name = chain->root().subject_organizational_unit_name (); + root_common_name = chain->root().subject_common_name (); + } + + if (all.size() >= 2) { + /* Have a leaf */ + leaf_common_name = chain->leaf().subject_common_name (); + } + + if (all.size() >= 3) { + /* Have an intermediate */ + dcp::CertificateChain::List::iterator i = all.begin (); + ++i; + intermediate_common_name = i->subject_common_name (); + } + wxTextValidator validator (wxFILTER_EXCLUDE_CHAR_LIST); validator.SetCharExcludes (wxT ("/")); @@ -50,14 +81,14 @@ MakeChainDialog::MakeChainDialog ( } add (_("Organisation"), true); - add (_organisation = new wxTextCtrl (this, wxID_ANY, std_to_wx (organisation), wxDefaultPosition, wxSize (480, -1), 0, validator)); + add (_organisation = new wxTextCtrl (this, wxID_ANY, std_to_wx(subject_organization_name), wxDefaultPosition, wxSize (480, -1), 0, validator)); add (_("Organisational unit"), true); - add (_organisational_unit = new wxTextCtrl (this, wxID_ANY, std_to_wx (organisational_unit_name), wxDefaultPosition, wxDefaultSize, 0, validator)); + add (_organisational_unit = new wxTextCtrl (this, wxID_ANY, std_to_wx(subject_organizational_unit_name), wxDefaultPosition, wxDefaultSize, 0, validator)); add (_("Root common name"), true); { - wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); + auto s = new wxBoxSizer (wxHORIZONTAL); s->Add (new StaticText (this, wxT (".")), 0, wxALIGN_CENTER_VERTICAL); s->Add (_root_common_name = new wxTextCtrl ( this, wxID_ANY, std_to_wx (root_common_name), wxDefaultPosition, wxDefaultSize, 0, validator), 1, wxALIGN_CENTER_VERTICAL @@ -68,7 +99,7 @@ MakeChainDialog::MakeChainDialog ( add (_("Intermediate common name"), true); { - wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); + auto s = new wxBoxSizer (wxHORIZONTAL); s->Add (new StaticText (this, wxT (".")), 0, wxALIGN_CENTER_VERTICAL); s->Add (_intermediate_common_name = new wxTextCtrl ( this, wxID_ANY, std_to_wx (intermediate_common_name), wxDefaultPosition, wxDefaultSize, 0, validator), 1, wxALIGN_CENTER_VERTICAL @@ -79,7 +110,7 @@ MakeChainDialog::MakeChainDialog ( add (_("Leaf common name"), true); { - wxBoxSizer* s = new wxBoxSizer (wxHORIZONTAL); + auto s = new wxBoxSizer (wxHORIZONTAL); s->Add (new StaticText (this, wxT ("CS.")), 0, wxALIGN_CENTER_VERTICAL); s->Add (_leaf_common_name = new wxTextCtrl ( this, wxID_ANY, std_to_wx (leaf_common_name), wxDefaultPosition, wxDefaultSize, 0, validator), 1, wxALIGN_CENTER_VERTICAL @@ -91,3 +122,18 @@ MakeChainDialog::MakeChainDialog ( _organisation->SetFocus (); } + + +shared_ptr +MakeChainDialog::get () const +{ + return make_shared( + openssl_path(), + CERTIFICATE_VALIDITY_PERIOD, + wx_to_std(_organisation->GetValue()), + wx_to_std(_organisational_unit->GetValue()), + "." + wx_to_std(_root_common_name->GetValue()), + "." + wx_to_std(_intermediate_common_name->GetValue()), + "CS." + wx_to_std(_leaf_common_name->GetValue()) + ); +} diff --git a/src/wx/make_chain_dialog.h b/src/wx/make_chain_dialog.h index 5ad62430e..018db99a2 100644 --- a/src/wx/make_chain_dialog.h +++ b/src/wx/make_chain_dialog.h @@ -27,35 +27,9 @@ class MakeChainDialog : public TableDialog { public: - MakeChainDialog ( - wxWindow* parent, - std::string organisation, - std::string organisational_unit_name, - std::string root_common_name, - std::string intermediate_common_name, - std::string leaf_common_name - ); - - std::string organisation () const { - return wx_to_std (_organisation->GetValue ()); - } - - std::string organisational_unit () const { - return wx_to_std (_organisational_unit->GetValue ()); - } - - std::string root_common_name () const { - return "." + wx_to_std (_root_common_name->GetValue ()); - } - - std::string intermediate_common_name () const { - return "." + wx_to_std (_intermediate_common_name->GetValue ()); - } - - std::string leaf_common_name () const { - return "CS." + wx_to_std (_leaf_common_name->GetValue ()); - } + MakeChainDialog (wxWindow* parent, std::shared_ptr chain); + std::shared_ptr get () const; private: wxTextCtrl* _organisation; diff --git a/test/hints_test.cc b/test/hints_test.cc index c228cd07a..51374b274 100644 --- a/test/hints_test.cc +++ b/test/hints_test.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2020-2021 Carl Hetherington + Copyright (C) 2020-2022 Carl Hetherington This file is part of DCP-o-matic. @@ -20,6 +20,7 @@ #include "lib/audio_content.h" +#include "lib/config.h" #include "lib/content.h" #include "lib/content_factory.h" #include "lib/cross.h" @@ -254,6 +255,24 @@ BOOST_AUTO_TEST_CASE (hints_audio_with_no_language) "Some of your content has audio but you have not set the audio language. It is advisable to set the audio language " "in the \"DCP\" tab unless your audio has no spoken parts." ); +} + + +BOOST_AUTO_TEST_CASE (hints_certificate_validity) +{ + ConfigRestorer cr; + + Config::instance()->set_signer_chain(make_shared(openssl_path(), 40 * 365)); + auto film = new_test_film2 ("hints_certificate_validity"); + auto hints = get_hints (film); + BOOST_REQUIRE_EQUAL (hints.size(), 1U); + BOOST_CHECK_EQUAL ( + hints[0], + "The certificate chain that DCP-o-matic uses for signing DCPs and KDMs has a validity period " + "that is too long. This will cause problems playing back DCPs on some systems. " + "You are advised to re-create the signing certificate chain by clicking the " + "\"Re-make certificates and key...\" button in the Keys page of Preferences." + ); }