changeset 5643:b1a18218860c

2 new configurations: DicomTlsMinimumProtocolVersion + DicomTlsCiphersAccepted
author Alain Mazy <am@orthanc.team>
date Fri, 31 May 2024 16:56:35 +0200
parents 95e282478cda
children 343149762476 4a2bfda999c6
files NEWS OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp 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 OrthancServer/Resources/Configuration.json OrthancServer/Sources/OrthancConfiguration.cpp OrthancServer/Sources/OrthancConfiguration.h OrthancServer/Sources/main.cpp
diffstat 12 files changed, 218 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri May 31 09:20:35 2024 +0200
+++ b/NEWS	Fri May 31 16:56:35 2024 +0200
@@ -58,6 +58,9 @@
     DCMTK option).
   - When working with "DicomTlsEnabled": true and "DicomTlsRemoteCertificateRequired": false,
     Orthanc was refusing to start if no "DicomTlsTrustedCertificates" was provided.
+  - New configurations:
+    - "DicomTlsMinimumProtocolVersion" to select the minimum TLS protocol version
+    - "DicomTlsCiphersAccepted" to fine tune the list of accepted ciphers
 * Upgraded dependencies for static builds:
   - boost 1.85.0
 
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Fri May 31 16:56:35 2024 +0200
@@ -300,11 +300,12 @@
       {
         assert(net_ != NULL &&
                params_ != NULL);
-        
         tls_.reset(Internals::InitializeDicomTls(net_, NET_REQUESTOR, parameters.GetOwnPrivateKeyPath(),
                                                  parameters.GetOwnCertificatePath(),
                                                  parameters.GetTrustedCertificatesPath(),
-                                                 parameters.IsRemoteCertificateRequired()));
+                                                 parameters.IsRemoteCertificateRequired(),
+                                                 parameters.GetMinimumTlsVersion(),
+                                                 parameters.GetAcceptedCiphers()));
       }
       catch (OrthancException&)
       {
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp	Fri May 31 16:56:35 2024 +0200
@@ -44,7 +44,8 @@
 static std::string   defaultTrustedCertificatesPath_;
 static unsigned int  defaultMaximumPduLength_ = ASC_DEFAULTMAXPDU;
 static bool          defaultRemoteCertificateRequired_ = true;
-
+static unsigned int  minimumTlsVersion_ = 0;
+static std::set<std::string> acceptedCiphers_;
 
 namespace Orthanc
 {
@@ -252,7 +253,26 @@
     return remoteCertificateRequired_;
   }
 
+  unsigned int DicomAssociationParameters::GetMinimumTlsVersion()
+  {
+    return minimumTlsVersion_;
+  }
   
+  void DicomAssociationParameters::SetMinimumTlsVersion(unsigned int version)
+  {
+    minimumTlsVersion_ = version;
+  }
+
+  void DicomAssociationParameters::SetAcceptedCiphers(const std::set<std::string>& acceptedCiphers)
+  {
+    acceptedCiphers_ = acceptedCiphers;
+  }
+
+  const std::set<std::string>& DicomAssociationParameters::GetAcceptedCiphers()
+  {
+    return acceptedCiphers_;
+  }
+
 
   static const char* const LOCAL_AET = "LocalAet";
   static const char* const REMOTE = "Remote";
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h	Fri May 31 16:56:35 2024 +0200
@@ -128,5 +128,13 @@
     static void SetDefaultRemoteCertificateRequired(bool required);
 
     static bool GetDefaultRemoteCertificateRequired();
+
+    static void SetMinimumTlsVersion(unsigned int version);
+
+    static unsigned int GetMinimumTlsVersion();
+
+    static void SetAcceptedCiphers(const std::set<std::string>& acceptedCiphers);
+
+    static const std::set<std::string>& GetAcceptedCiphers();
   };
 }
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Fri May 31 16:56:35 2024 +0200
@@ -411,7 +411,7 @@
       {
         pimpl_->tls_.reset(Internals::InitializeDicomTls(
                              pimpl_->network_, NET_ACCEPTOR, ownPrivateKeyPath_, ownCertificatePath_,
-                             trustedCertificatesPath_, remoteCertificateRequired_));
+                             trustedCertificatesPath_, remoteCertificateRequired_, minimumTlsVersion_, acceptedCiphers_));
       }
       catch (OrthancException&)
       {
@@ -494,6 +494,18 @@
     return useDicomTls_;
   }
 
+  void DicomServer::SetMinimumTlsVersion(unsigned int version)
+  {
+    minimumTlsVersion_ = version;
+    DicomAssociationParameters::SetMinimumTlsVersion(version);
+  }
+
+  void DicomServer::SetAcceptedCiphers(const std::set<std::string>& ciphers)
+  {
+    acceptedCiphers_ = ciphers;
+    DicomAssociationParameters::SetAcceptedCiphers(ciphers);
+  }
+
   void DicomServer::SetOwnCertificatePath(const std::string& privateKeyPath,
                                           const std::string& certificatePath)
   {
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Fri May 31 16:56:35 2024 +0200
@@ -91,6 +91,8 @@
     std::string  trustedCertificatesPath_;
     unsigned int maximumPduLength_;
     bool         remoteCertificateRequired_;  // New in 1.9.3
+    unsigned int minimumTlsVersion_;          // New in 1.12.4
+    std::set<std::string> acceptedCiphers_;   // New in 1.12.4
 
 
     static void ServerThread(DicomServer* server,
@@ -154,6 +156,9 @@
     void SetDicomTlsEnabled(bool enabled);
     bool IsDicomTlsEnabled() const;
 
+    void SetMinimumTlsVersion(unsigned int version);
+    void SetAcceptedCiphers(const std::set<std::string>& ciphers);
+
     void SetOwnCertificatePath(const std::string& privateKeyPath,
                                const std::string& certificatePath);
     const std::string& GetOwnPrivateKeyPath() const;    
--- a/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.cpp	Fri May 31 16:56:35 2024 +0200
@@ -28,6 +28,9 @@
 #include "../../Logging.h"
 #include "../../OrthancException.h"
 #include "../../SystemToolbox.h"
+#include "../../Toolbox.h"
+#include <openssl/ssl.h>
+#include <openssl/err.h>
 
 #if DCMTK_VERSION_NUMBER < 364
 #  define DCF_Filetype_PEM  SSL_FILETYPE_PEM
@@ -63,7 +66,9 @@
                                              const std::string& ownPrivateKeyPath,
                                              const std::string& ownCertificatePath,
                                              const std::string& trustedCertificatesPath,
-                                             bool requireRemoteCertificate)
+                                             bool requireRemoteCertificate,
+                                             unsigned int minimalTlsVersion,
+                                             const std::set<std::string>& ciphers)
     {
       if (network == NULL)
       {
@@ -156,14 +161,94 @@
       }
 
 #if DCMTK_VERSION_NUMBER >= 364
-      if (IsFailure(tls->setTLSProfile(TSP_Profile_BCP195 /*opt_tlsProfile*/)))
+      if (minimalTlsVersion == 0) // use the default values (same behavior as before 1.12.4)
       {
-        throw OrthancException(ErrorCode_InternalError, "Cannot set the DICOM TLS profile");
+        if (ciphers.size() > 0)
+        {
+          throw OrthancException(ErrorCode_BadFileFormat, "The cipher suites can not be specified when using the default BCP profile");
+        }
+
+        if (IsFailure(tls->setTLSProfile(TSP_Profile_BCP195 /*opt_tlsProfile*/)))
+        {
+          throw OrthancException(ErrorCode_InternalError, "Cannot set the DICOM TLS profile");
+        }
+      
+        if (IsFailure(tls->activateCipherSuites()))
+        {
+          throw OrthancException(ErrorCode_InternalError, "Cannot activate the cipher suites for DICOM TLS");
+        }
       }
-    
-      if (IsFailure(tls->activateCipherSuites()))
+      else
       {
-        throw OrthancException(ErrorCode_InternalError, "Cannot activate the cipher suites for DICOM TLS");
+        // Fine tune the SSL context
+        if (IsFailure(tls->setTLSProfile(TSP_Profile_None)))
+        {
+          throw OrthancException(ErrorCode_InternalError, "Cannot set the DICOM TLS profile");
+        }
+
+        DcmTLSTransportLayer::native_handle_type sslNativeHandle = tls->getNativeHandle();
+        SSL_CTX_clear_options(sslNativeHandle, SSL_OP_NO_SSL_MASK);
+        if (minimalTlsVersion > 1) 
+        {
+          SSL_CTX_set_options(sslNativeHandle, SSL_OP_NO_SSLv3);
+        }
+        if (minimalTlsVersion > 2) 
+        {
+          SSL_CTX_set_options(sslNativeHandle, SSL_OP_NO_TLSv1);
+        }
+        if (minimalTlsVersion > 3) 
+        {
+          SSL_CTX_set_options(sslNativeHandle, SSL_OP_NO_TLSv1_1);
+        }
+        if (minimalTlsVersion > 4) 
+        {
+          SSL_CTX_set_options(sslNativeHandle, SSL_OP_NO_TLSv1_2);
+        }
+
+        std::set<std::string> ciphersTls;
+        std::set<std::string> ciphersTls13;
+
+        // DCMTK 3.8 is missing a method to add TLS13 cipher suite in the DcmTLSTransportLayer interface.
+        // And, anyway, since we do not run dcmtkPrepare.cmake, DCMTK is not aware of TLS v1.3 cipher suite names.
+        for (std::set<std::string>::const_iterator it = ciphers.begin(); it != ciphers.end(); ++it)
+        {
+          bool isValid = false;
+          if (DcmTLSCiphersuiteHandler::lookupCiphersuiteByOpenSSLName(it->c_str()) != DcmTLSCiphersuiteHandler::unknownCipherSuiteIndex)
+          {
+            ciphersTls.insert(it->c_str());
+            isValid = true;
+          }
+          
+          // list of TLS v1.3 ciphers according to https://www.openssl.org/docs/man3.3/man1/openssl-ciphers.html
+          if (strstr("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_CCM_SHA256:TLS_AES_128_CCM_8_SHA256", it->c_str()) != NULL)
+          {
+            ciphersTls13.insert(it->c_str());
+            isValid = true;
+          }
+
+          if (!isValid)
+          {
+            throw OrthancException(ErrorCode_BadFileFormat, "The cipher suite " + *it + " is not recognized as valid cipher suite by OpenSSL ");
+          }
+        }
+
+        std::string joinedCiphersTls;
+        std::string joinedCiphersTls13;
+        Toolbox::JoinStrings(joinedCiphersTls, ciphersTls, ":");
+        Toolbox::JoinStrings(joinedCiphersTls13, ciphersTls13, ":");
+
+        if (joinedCiphersTls.size() > 0 && SSL_CTX_set_cipher_list(sslNativeHandle, joinedCiphersTls.c_str()) != 1)
+        {
+          OFCondition cond = DcmTLSTransportLayer::convertOpenSSLError(ERR_get_error(), OFTrue);
+          throw OrthancException(ErrorCode_InternalError, "Unable to configure cipher suite.  OpenSSL error: " + boost::lexical_cast<std::string>(cond.code()) + " - " + cond.text());
+        }
+
+        if (joinedCiphersTls13.size() > 0 && SSL_CTX_set_ciphersuites(sslNativeHandle, joinedCiphersTls13.c_str()) != 1)
+        {
+          OFCondition cond = DcmTLSTransportLayer::convertOpenSSLError(ERR_get_error(), OFTrue);
+          throw OrthancException(ErrorCode_InternalError, "Unable to configure cipher suite for TLS 1.3.  OpenSSL error: " + boost::lexical_cast<std::string>(cond.code()) + " - " + cond.text());
+        }
+
       }
 #else
       CLOG(INFO, DICOM) << "Using the following cipher suites for DICOM TLS: " << opt_ciphersuites;
--- a/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/DicomTls.h	Fri May 31 16:56:35 2024 +0200
@@ -39,7 +39,7 @@
 
 #include <dcmtk/dcmnet/dimse.h>
 #include <dcmtk/dcmtls/tlslayer.h>
-
+#include <set>
 
 namespace Orthanc
 {
@@ -51,6 +51,9 @@
       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
-      bool requireRemoteCertificate);              // "true" means "--require-peer-cert", "false" means "--verify-peer-cert"
+      bool requireRemoteCertificate,               // "true" means "--require-peer-cert", "false" means "--ignore-peer-cert"
+      unsigned int minimalTlsVersion,              // 0 = default BCP195, 5 = TLS1.3 only
+      const std::set<std::string>& acceptedCiphers
+    );
   }
 }
--- a/OrthancServer/Resources/Configuration.json	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancServer/Resources/Configuration.json	Fri May 31 16:56:35 2024 +0200
@@ -309,10 +309,39 @@
   // 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
+  // (resp. "--ignore-peer-cert") in the DCMTK command-line
   // tools. (new in Orthanc 1.9.3)
   "DicomTlsRemoteCertificateRequired" : true,
 
+  // Sets the minimum accepted TLS protocol version for the DICOM server
+  // By default, require TLS 1.2 or 1.3. This option is only meaningful 
+  // if "DicomTlsEnabled" is true (new in Orthanc 1.12.4).
+  // Note that, internally, Orthanc is configured to use the BCP195 profile
+  // by default.  As soon as you switch to another protocol version, you
+  // must also provide the list of supported cipher suites.
+  // This configuration applies to Orthanc acting both as SCU and SCP.
+  // Value => Protocols
+  //   0      use default BCP 195 profile and default cipher suites
+  //   1      SSL3+TLS1.0+TLS1.1+TLS1.2+TLS1.3
+  //   2      TLS1.0+TLS1.1+TLS1.2+TLS1.3
+  //   3      TLS1.1+TLS1.2+TLS1.3
+  //   4      TLS1.2+TLS1.3
+  //   5      TLS1.3
+  "DicomTlsMinimumProtocolVersion" : 0,
+
+  // Set the accepted ciphers for TLS connections for the DICOM server. 
+  // The ciphers must be provided as a list of strings. If not set, 
+  // this will default to BCP195 ciphers if DicomTlsMinimumProtocolVersion is 0
+  // or to an empty list for other values. This option is only 
+  // meaningful if "DicomTlsEnabled" is true. (new in Orthanc 1.12.4).
+  // This configuration must be provided if DicomTlsMinimumProtocolVersion != 0.
+  // The list of valid cipher names are available in 
+  // https://www.openssl.org/docs/man3.3/man1/openssl-ciphers.html
+  // The OpenSSL names are used.
+  /**
+     "DicomTlsCiphersAccepted" : []
+  **/
+  
   // 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/OrthancConfiguration.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancServer/Sources/OrthancConfiguration.cpp	Fri May 31 16:56:35 2024 +0200
@@ -751,7 +751,7 @@
 
     if (lst.type() != Json::arrayValue)
     {
-      throw OrthancException(ErrorCode_BadFileFormat, "Badly formatted list of strings");
+      throw OrthancException(ErrorCode_BadFileFormat, "Badly formatted list of strings: " + key);
     }
 
     for (Json::Value::ArrayIndex i = 0; i < lst.size(); i++)
@@ -760,7 +760,31 @@
     }    
   }
 
-    
+
+  void OrthancConfiguration::GetSetOfStringsParameter(std::set<std::string>& target,
+                                                      const std::string& key) const
+  {
+    target.clear();
+  
+    if (!json_.isMember(key))
+    {
+      return;
+    }
+
+    const Json::Value& lst = json_[key];
+
+    if (lst.type() != Json::arrayValue)
+    {
+      throw OrthancException(ErrorCode_BadFileFormat, "Badly formatted set of strings: " + key);
+    }
+
+    for (Json::Value::ArrayIndex i = 0; i < lst.size(); i++)
+    {
+      target.insert(lst[i].asString());
+    }    
+  }
+
+
   bool OrthancConfiguration::IsSameAETitle(const std::string& aet1,
                                            const std::string& aet2) const
   {
--- a/OrthancServer/Sources/OrthancConfiguration.h	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancServer/Sources/OrthancConfiguration.h	Fri May 31 16:56:35 2024 +0200
@@ -204,7 +204,10 @@
     
     void GetListOfStringsParameter(std::list<std::string>& target,
                                    const std::string& key) const;
-    
+
+    void GetSetOfStringsParameter(std::set<std::string>& target,
+                                  const std::string& key) const;
+
     bool IsSameAETitle(const std::string& aet1,
                        const std::string& aet2) const;
 
--- a/OrthancServer/Sources/main.cpp	Fri May 31 09:20:35 2024 +0200
+++ b/OrthancServer/Sources/main.cpp	Fri May 31 16:56:35 2024 +0200
@@ -59,8 +59,10 @@
 static const char* const KEY_DICOM_TLS_ENABLED = "DicomTlsEnabled";
 static const char* const KEY_DICOM_TLS_CERTIFICATE = "DicomTlsCertificate";
 static const char* const KEY_DICOM_TLS_TRUSTED_CERTIFICATES = "DicomTlsTrustedCertificates";
+static const char* const KEY_DICOM_TLS_REMOTE_CERTIFICATE_REQUIRED = "DicomTlsRemoteCertificateRequired";
+static const char* const KEY_DICOM_TLS_MINIMUM_PROTOCOL_VERSION = "DicomTlsMinimumProtocolVersion";
+static const char* const KEY_DICOM_TLS_ACCEPTED_CIPHERS = "DicomTlsCiphersAccepted";
 static const char* const KEY_MAXIMUM_PDU_LENGTH = "MaximumPduLength";
-static const char* const KEY_DICOM_TLS_REMOTE_CERTIFICATE_REQUIRED = "DicomTlsRemoteCertificateRequired";
 
 
 class OrthancStoreRequestHandler : public IStoreRequestHandler
@@ -1279,6 +1281,12 @@
           lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_CERTIFICATE, ""));
         dicomServer.SetTrustedCertificatesPath(
           lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_TRUSTED_CERTIFICATES, ""));
+        dicomServer.SetMinimumTlsVersion(
+          lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_DICOM_TLS_MINIMUM_PROTOCOL_VERSION, 0));
+        
+        std::set<std::string> acceptedCiphers;
+        lock.GetConfiguration().GetSetOfStringsParameter(acceptedCiphers, KEY_DICOM_TLS_ACCEPTED_CIPHERS);
+        dicomServer.SetAcceptedCiphers(acceptedCiphers);
       }
 
       dicomServer.SetMaximumPduLength(lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_MAXIMUM_PDU_LENGTH, 16384));