Mercurial > hg > orthanc

changeset 4656:82a314325351

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
New configuration option: "DicomTlsRemoteCertificateRequired"
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 06 May 2021 18:39:19 +0200
parents 9f7eef20bc7d
children e8967149d87a
files NEWS OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp OrthancFramework/Sources/DicomNetworking/DicomAssociation.h OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h OrthancFramework/Sources/DicomNetworking/DicomServer.cpp OrthancFramework/Sources/DicomNetworking/DicomServer.h OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h OrthancFramework/UnitTestsSources/JobsTests.cpp OrthancServer/Resources/Configuration.json OrthancServer/Sources/main.cpp
diffstat 12 files changed, 116 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- 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
--- 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&)
       {
--- 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<DcmTLSTransportLayer>     tls_;
 #endif
 
-    void Initialize();
-
     void CheckConnecting(const DicomAssociationParameters& parameters,
                          const OFCondition& cond);
     
@@ -86,10 +84,7 @@
                      uint8_t presentationContextId);
 
   public:
-    DicomAssociation()
-    {
-      Initialize();
-    }
+    DicomAssociation();
 
     ~DicomAssociation();
 
--- 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_;
+  }
 }
--- 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();
   };
 }
--- 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_;
+  }
 }
--- 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;
   };
 }
--- 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())
       {
--- 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"
   }
 }
--- 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());
   }  
 }
--- 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
--- 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);