# HG changeset patch # User Sebastien Jodogne # Date 1620319159 -7200 # Node ID 82a314325351472a647a2754dc9f8d8be31b6466 # Parent 9f7eef20bc7d65980036ac5bb33246a87e73273e New configuration option: "DicomTlsRemoteCertificateRequired" diff -r 9f7eef20bc7d -r 82a314325351 NEWS --- a/NEWS Thu May 06 16:54:46 2021 +0200 +++ b/NEWS Thu May 06 18:39:19 2021 +0200 @@ -1,6 +1,8 @@ Pending changes in the mainline =============================== +* New configuration option: "DicomTlsRemoteCertificateRequired" to allow secure DICOM TLS + connections without certificate * "ETag" headers for metadata and attachments now allow strong comparison (MD5 is included) * Fixed the lifetime of temporary files associated with jobs that create ZIP archive/media: - In synchronous mode, their number could grow up to "JobsHistorySize" in Orthanc <= 1.9.2 diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp --- a/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp Thu May 06 18:39:19 2021 +0200 @@ -75,19 +75,6 @@ } - void DicomAssociation::Initialize() - { - role_ = DicomAssociationRole_Default; - isOpen_ = false; - net_ = NULL; - params_ = NULL; - assoc_ = NULL; - - // Must be after "isOpen_ = false" - ClearPresentationContexts(); - } - - void DicomAssociation::CheckConnecting(const DicomAssociationParameters& parameters, const OFCondition& cond) { @@ -176,6 +163,19 @@ } + DicomAssociation::DicomAssociation() + { + role_ = DicomAssociationRole_Default; + isOpen_ = false; + net_ = NULL; + params_ = NULL; + assoc_ = NULL; + + // Must be after "isOpen_ = false" + ClearPresentationContexts(); + } + + DicomAssociation::~DicomAssociation() { try @@ -295,7 +295,8 @@ tls_.reset(Internals::InitializeDicomTls(net_, NET_REQUESTOR, parameters.GetOwnPrivateKeyPath(), parameters.GetOwnCertificatePath(), - parameters.GetTrustedCertificatesPath())); + parameters.GetTrustedCertificatesPath(), + parameters.IsRemoteCertificateRequired())); } catch (OrthancException&) { diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomAssociation.h --- a/OrthancFramework/Sources/DicomNetworking/DicomAssociation.h Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociation.h Thu May 06 18:39:19 2021 +0200 @@ -74,8 +74,6 @@ std::unique_ptr tls_; #endif - void Initialize(); - void CheckConnecting(const DicomAssociationParameters& parameters, const OFCondition& cond); @@ -86,10 +84,7 @@ uint8_t presentationContextId); public: - DicomAssociation() - { - Initialize(); - } + DicomAssociation(); ~DicomAssociation(); diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp --- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp Thu May 06 18:39:19 2021 +0200 @@ -41,6 +41,7 @@ static std::string defaultOwnCertificatePath_; static std::string defaultTrustedCertificatesPath_; static unsigned int defaultMaximumPduLength_ = ASC_DEFAULTMAXPDU; +static bool defaultRemoteCertificateRequired_ = true; namespace Orthanc @@ -70,6 +71,7 @@ ownCertificatePath_ = defaultOwnCertificatePath_; trustedCertificatesPath_ = defaultTrustedCertificatesPath_; maximumPduLength_ = defaultMaximumPduLength_; + remoteCertificateRequired_ = defaultRemoteCertificateRequired_; } @@ -237,7 +239,17 @@ CheckMaximumPduLength(pdu); maximumPduLength_ = pdu; } - + + void DicomAssociationParameters::SetRemoteCertificateRequired(bool required) + { + remoteCertificateRequired_ = required; + } + + bool DicomAssociationParameters::IsRemoteCertificateRequired() const + { + return remoteCertificateRequired_; + } + static const char* const LOCAL_AET = "LocalAet"; @@ -247,6 +259,7 @@ static const char* const OWN_CERTIFICATE = "OwnCertificate"; // New in Orthanc 1.9.0 static const char* const TRUSTED_CERTIFICATES = "TrustedCertificates"; // New in Orthanc 1.9.0 static const char* const MAXIMUM_PDU_LENGTH = "MaximumPduLength"; // New in Orthanc 1.9.0 + static const char* const REMOTE_CERTIFICATE_REQUIRED = "RemoteCertificateRequired"; // New in Orthanc 1.9.3 void DicomAssociationParameters::SerializeJob(Json::Value& target) const @@ -261,6 +274,7 @@ remote_.Serialize(target[REMOTE], true /* force advanced format */); target[TIMEOUT] = timeout_; target[MAXIMUM_PDU_LENGTH] = maximumPduLength_; + target[REMOTE_CERTIFICATE_REQUIRED] = remoteCertificateRequired_; // Don't write the DICOM TLS parameters if they are not required if (ownPrivateKeyPath_.empty()) @@ -341,6 +355,11 @@ { result.trustedCertificatesPath_.clear(); } + + if (serialized.isMember(REMOTE_CERTIFICATE_REQUIRED)) + { + result.remoteCertificateRequired_ = SerializationToolbox::ReadBoolean(serialized, REMOTE_CERTIFICATE_REQUIRED); + } return result; } @@ -464,4 +483,18 @@ boost::mutex::scoped_lock lock(defaultConfigurationMutex_); return defaultMaximumPduLength_; } + + + void DicomAssociationParameters::SetDefaultRemoteCertificateRequired(bool required) + { + boost::mutex::scoped_lock lock(defaultConfigurationMutex_); + defaultRemoteCertificateRequired_ = required; + } + + + bool DicomAssociationParameters::GetDefaultRemoteCertificateRequired() + { + boost::mutex::scoped_lock lock(defaultConfigurationMutex_); + return defaultRemoteCertificateRequired_; + } } diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h --- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h Thu May 06 18:39:19 2021 +0200 @@ -40,6 +40,7 @@ std::string ownCertificatePath_; std::string trustedCertificatesPath_; unsigned int maximumPduLength_; + bool remoteCertificateRequired_; // New in 1.9.3, for DICOM TLS static void CheckHost(const std::string& host); @@ -99,6 +100,10 @@ void SetMaximumPduLength(unsigned int pdu); + void SetRemoteCertificateRequired(bool required); + + bool IsRemoteCertificateRequired() const; + void SerializeJob(Json::Value& target) const; static DicomAssociationParameters UnserializeJob(const Json::Value& serialized); @@ -117,5 +122,9 @@ static void SetDefaultMaximumPduLength(unsigned int pdu); static unsigned int GetDefaultMaximumPduLength(); + + static void SetDefaultRemoteCertificateRequired(bool required); + + static bool GetDefaultRemoteCertificateRequired(); }; } diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomServer.cpp --- a/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp Thu May 06 18:39:19 2021 +0200 @@ -102,7 +102,8 @@ storageCommitmentFactory_(NULL), applicationEntityFilter_(NULL), useDicomTls_(false), - maximumPduLength_(ASC_DEFAULTMAXPDU) + maximumPduLength_(ASC_DEFAULTMAXPDU), + remoteCertificateRequired_(true) { } @@ -404,8 +405,9 @@ try { - pimpl_->tls_.reset(Internals::InitializeDicomTls(pimpl_->network_, NET_ACCEPTOR, ownPrivateKeyPath_, - ownCertificatePath_, trustedCertificatesPath_)); + pimpl_->tls_.reset(Internals::InitializeDicomTls( + pimpl_->network_, NET_ACCEPTOR, ownPrivateKeyPath_, ownCertificatePath_, + trustedCertificatesPath_, remoteCertificateRequired_)); } catch (OrthancException&) { @@ -575,4 +577,15 @@ Stop(); maximumPduLength_ = pdu; } + + void DicomServer::SetRemoteCertificateRequired(bool required) + { + Stop(); + remoteCertificateRequired_ = required; + } + + bool DicomServer::IsRemoteCertificateRequired() const + { + return remoteCertificateRequired_; + } } diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/DicomServer.h --- a/OrthancFramework/Sources/DicomNetworking/DicomServer.h Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.h Thu May 06 18:39:19 2021 +0200 @@ -87,6 +87,7 @@ std::string ownCertificatePath_; std::string trustedCertificatesPath_; unsigned int maximumPduLength_; + bool remoteCertificateRequired_; // New in 1.9.3 static void ServerThread(DicomServer* server, unsigned int maximumPduLength, @@ -159,5 +160,8 @@ unsigned int GetMaximumPduLength() const; void SetMaximumPduLength(unsigned int pdu); + + void SetRemoteCertificateRequired(bool required); + bool IsRemoteCertificateRequired() const; }; } diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp --- a/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp Thu May 06 18:39:19 2021 +0200 @@ -48,7 +48,8 @@ T_ASC_NetworkRole role, const std::string& ownPrivateKeyPath, const std::string& ownCertificatePath, - const std::string& trustedCertificatesPath) + const std::string& trustedCertificatesPath, + bool requireRemoteCertificate) { if (network == NULL) { @@ -147,7 +148,16 @@ } #endif - tls->setCertificateVerification(DCV_requireCertificate /*opt_certVerification*/); + if (requireRemoteCertificate) + { + // Check remote certificate, fail if no certificate is present + tls->setCertificateVerification(DCV_requireCertificate /*opt_certVerification*/); + } + else + { + // Check remote certificate if present, succeed if no certificate is present + tls->setCertificateVerification(DCV_checkCertificate /*opt_certVerification*/); + } if (ASC_setTransportLayer(network, tls.get(), 0).bad()) { diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h --- a/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h Thu May 06 18:39:19 2021 +0200 @@ -48,6 +48,7 @@ T_ASC_NetworkRole role, const std::string& ownPrivateKeyPath, // This is the first argument of "+tls" option from DCMTK command-line tools const std::string& ownCertificatePath, // This is the second argument of "+tls" option - const std::string& trustedCertificatesPath); // This is the "--add-cert-file" ("+cf") option + const std::string& trustedCertificatesPath, // This is the "--add-cert-file" ("+cf") option + bool requireRemoteCertificate); // "true" means "--require-peer-cert", "false" means "--verify-peer-cert" } } diff -r 9f7eef20bc7d -r 82a314325351 OrthancFramework/UnitTestsSources/JobsTests.cpp --- a/OrthancFramework/UnitTestsSources/JobsTests.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancFramework/UnitTestsSources/JobsTests.cpp Thu May 06 18:39:19 2021 +0200 @@ -1493,7 +1493,7 @@ ASSERT_TRUE(v.isMember("Remote")); ASSERT_TRUE(v.isMember("MaximumPduLength")); - ASSERT_EQ(4u, v.getMemberNames().size()); + ASSERT_EQ(5u, v.getMemberNames().size()); DicomAssociationParameters b; b.UnserializeJob(v); @@ -1507,6 +1507,7 @@ ASSERT_THROW(b.GetRemoteModality().GetLocalAet(), OrthancException); ASSERT_FALSE(b.GetRemoteModality().HasTimeout()); ASSERT_EQ(0u, b.GetRemoteModality().GetTimeout()); + ASSERT_TRUE(b.IsRemoteCertificateRequired()); } { @@ -1520,6 +1521,7 @@ DicomAssociationParameters a("HELLO", p); a.SetOwnCertificatePath("key", "crt"); a.SetTrustedCertificatesPath("trusted"); + a.SetRemoteCertificateRequired(false); ASSERT_THROW(a.SetMaximumPduLength(4095), OrthancException); ASSERT_THROW(a.SetMaximumPduLength(131073), OrthancException); @@ -1529,7 +1531,7 @@ Json::Value v = Json::objectValue; a.SerializeJob(v); - ASSERT_EQ(7u, v.getMemberNames().size()); + ASSERT_EQ(8u, v.getMemberNames().size()); DicomAssociationParameters b = DicomAssociationParameters::UnserializeJob(v); @@ -1544,5 +1546,6 @@ ASSERT_EQ(131072u, b.GetMaximumPduLength()); ASSERT_TRUE(b.GetRemoteModality().HasTimeout()); ASSERT_EQ(42u, b.GetRemoteModality().GetTimeout()); + ASSERT_FALSE(b.IsRemoteCertificateRequired()); } } diff -r 9f7eef20bc7d -r 82a314325351 OrthancServer/Resources/Configuration.json --- a/OrthancServer/Resources/Configuration.json Thu May 06 16:54:46 2021 +0200 +++ b/OrthancServer/Resources/Configuration.json Thu May 06 18:39:19 2021 +0200 @@ -261,6 +261,13 @@ "DicomTlsTrustedCertificates" : "trusted.crt", **/ + // Whether Orthanc rejects DICOM TLS connections to/from remote + // modalities that do not provide a certificate. Setting this option + // to "true" (resp. "false") corresponds to "--require-peer-cert" + // (resp. "--verify-peer-cert") in the DCMTK command-line + // tools. (new in Orthanc 1.9.3) + "DicomTlsRemoteCertificateRequired" : true, + // Whether the Orthanc SCP allows incoming C-ECHO requests, even // from SCU modalities it does not know about (i.e. that are not // listed in the "DicomModalities" option above). Orthanc 1.3.0 diff -r 9f7eef20bc7d -r 82a314325351 OrthancServer/Sources/main.cpp --- a/OrthancServer/Sources/main.cpp Thu May 06 16:54:46 2021 +0200 +++ b/OrthancServer/Sources/main.cpp Thu May 06 18:39:19 2021 +0200 @@ -69,6 +69,7 @@ static const char* const KEY_DICOM_TLS_CERTIFICATE = "DicomTlsCertificate"; static const char* const KEY_DICOM_TLS_TRUSTED_CERTIFICATES = "DicomTlsTrustedCertificates"; static const char* const KEY_MAXIMUM_PDU_LENGTH = "MaximumPduLength"; +static const char* const KEY_DICOM_TLS_REMOTE_CERTIFICATE_REQUIRED = "DicomTlsRemoteCertificateRequired"; class OrthancStoreRequestHandler : public IStoreRequestHandler @@ -1209,6 +1210,10 @@ } dicomServer.SetMaximumPduLength(lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_MAXIMUM_PDU_LENGTH, 16384)); + + // New option in Orthanc 1.9.3 + dicomServer.SetRemoteCertificateRequired( + lock.GetConfiguration().GetBooleanParameter(KEY_DICOM_TLS_REMOTE_CERTIFICATE_REQUIRED, true)); } #if ORTHANC_ENABLE_PLUGINS == 1 @@ -1467,6 +1472,10 @@ lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_TRUSTED_CERTIFICATES, "")); DicomAssociationParameters::SetDefaultMaximumPduLength( lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_MAXIMUM_PDU_LENGTH, 16384)); + + // New option in Orthanc 1.9.3 + DicomAssociationParameters::SetDefaultRemoteCertificateRequired( + lock.GetConfiguration().GetBooleanParameter(KEY_DICOM_TLS_REMOTE_CERTIFICATE_REQUIRED, true)); } ServerContext context(database, storageArea, false /* not running unit tests */, maxCompletedJobs);