changeset 4382:3aacd2bd8bbc varian

review changeset 4381:df313e410f0c
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 17 Dec 2020 15:10:04 +0100
parents df313e410f0c
children e49cf50b54c8 f5cde118a284
files NEWS OrthancFramework/Sources/HttpServer/HttpServer.cpp OrthancFramework/Sources/HttpServer/HttpServer.h OrthancServer/Resources/Configuration.json OrthancServer/Sources/main.cpp
diffstat 5 files changed, 125 insertions(+), 49 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri Dec 11 11:59:10 2020 -0500
+++ b/NEWS	Thu Dec 17 15:10:04 2020 +0100
@@ -12,6 +12,8 @@
   - "DeidentifyLogs" to remove patient identification from the logs (C-GET, C-MOVE, C-FIND)
   - "DeidentifyLogsDicomVersion" to specify the deidentification rules for the logs
   - "OrthancExplorerEnabled" to enable/disable the Orthanc Explorer Web user interface
+  - "SslMinimumProtocolVersion" to set the minimal SSL protocol version (now defaults to SSL 1.2)
+  - "SslCiphersAccepted" to set the accepted ciphers over SSL (now defaults to FIPS 140-2)
 
 REST API
 --------
--- a/OrthancFramework/Sources/HttpServer/HttpServer.cpp	Fri Dec 11 11:59:10 2020 -0500
+++ b/OrthancFramework/Sources/HttpServer/HttpServer.cpp	Thu Dec 17 15:10:04 2020 +0100
@@ -1503,15 +1503,17 @@
     authentication_(false),
     sslVerifyPeers_(false),
     ssl_(false),
+    sslMinimumVersion_(0),  // Default to any of "SSL2+SSL3+TLS1.0+TLS1.1+TLS1.2"
+    sslHasCiphers_(false),
     port_(8000),
     filter_(NULL),
     keepAlive_(false),
     httpCompression_(true),
     exceptionFormatter_(NULL),
     realm_(ORTHANC_REALM),
-    threadsCount_(50),  // Default value in mongoose
+    threadsCount_(50),  // Default value in mongoose/civetweb
     tcpNoDelay_(true),
-    requestTimeout_(30)  // Default value in mongoose/civetweb (30 seconds)    
+    requestTimeout_(30)  // Default value in mongoose/civetweb (30 seconds)
   {
 #if ORTHANC_ENABLE_MONGOOSE == 1
     CLOG(INFO, HTTP) << "This Orthanc server uses Mongoose as its embedded HTTP server";
@@ -1574,6 +1576,7 @@
       std::string numThreads = boost::lexical_cast<std::string>(threadsCount_);
       std::string requestTimeoutMilliseconds = boost::lexical_cast<std::string>(requestTimeout_ * 1000);
       std::string keepAliveTimeoutMilliseconds = boost::lexical_cast<std::string>(CIVETWEB_KEEP_ALIVE_TIMEOUT_SECONDS * 1000);
+      std::string sslMinimumVersion = boost::lexical_cast<std::string>(sslMinimumVersion_);
 
       if (ssl_)
       {
@@ -1631,15 +1634,19 @@
         options.push_back("ssl_ca_file");
         options.push_back(trustedClientCertificates_.c_str());
       }
+      
       if (ssl_)
       {
         // Restrict minimum SSL/TLS protocol version
         options.push_back("ssl_protocol_version");
-        options.push_back(sslMinimumVersion_.c_str());
+        options.push_back(sslMinimumVersion.c_str());
 
         // Set the accepted ciphers list
-        options.push_back("ssl_cipher_list");
-        options.push_back(sslCiphers_.c_str());
+        if (sslHasCiphers_)
+        {
+          options.push_back("ssl_cipher_list");
+          options.push_back(sslCiphers_.c_str());
+        }
 
         // Set the SSL certificate, if any
         options.push_back("ssl_certificate");
@@ -1790,16 +1797,72 @@
 #endif
   }
 
-  void HttpServer::SetSslMinimumVersion(std::string version)
+  void HttpServer::SetSslMinimumVersion(unsigned int version)
   {
     Stop();
-    sslMinimumVersion_ = std::move(version);
+    sslMinimumVersion_ = version;
+
+    std::string info;
+    
+    switch (version)
+    {
+      case 0:
+        info = "SSL2+SSL3+TLS1.0+TLS1.1+TLS1.2";
+        break;
+
+      case 1:
+        info = "SSL3+TLS1.0+TLS1.1+TLS1.2";
+        break;
+
+      case 2:
+        info = "TLS1.0+TLS1.1+TLS1.2";
+        break;
+
+      case 3:
+        info = "TLS1.1+TLS1.2";
+        break;
+
+      case 4:
+        info = "TLS1.2";
+        break;
+
+      default:
+        info = "Unknown value (" + boost::lexical_cast<std::string>(version) + ")";
+        break;
+    }
+
+    CLOG(INFO, HTTP) << "Minimal accepted version of SSL/TLS protocol: " << info;
   }
 
-  void HttpServer::SetSslCiphers(std::string ciphers)
+  void HttpServer::SetSslCiphers(const std::list<std::string>& ciphers)
   {
     Stop();
-    sslCiphers_ = std::move(ciphers);
+
+    sslHasCiphers_ = true;
+    sslCiphers_.clear();
+
+    for (std::list<std::string>::const_iterator
+           it = ciphers.begin(); it != ciphers.end(); ++it)
+    {
+      if (it->empty())
+      {
+        throw OrthancException(ErrorCode_ParameterOutOfRange, "Empty name for a cipher");
+      }
+
+      if (!sslCiphers_.empty())
+      {
+        sslCiphers_ += ':';
+      }
+      
+      sslCiphers_ += (*it);
+    }      
+
+    CLOG(INFO, HTTP) << "List of accepted SSL ciphers: " << sslCiphers_;
+    
+    if (sslCiphers_.empty())
+    {
+      CLOG(WARNING, HTTP) << "No cipher is accepted for SSL";
+    }
   }
 
   void HttpServer::SetKeepAliveEnabled(bool enabled)
--- a/OrthancFramework/Sources/HttpServer/HttpServer.h	Fri Dec 11 11:59:10 2020 -0500
+++ b/OrthancFramework/Sources/HttpServer/HttpServer.h	Thu Dec 17 15:10:04 2020 +0100
@@ -97,7 +97,8 @@
     std::string trustedClientCertificates_;
     bool ssl_;
     std::string certificate_;
-    std::string sslMinimumVersion_;
+    unsigned int sslMinimumVersion_;
+    bool sslHasCiphers_;
     std::string sslCiphers_;
     uint16_t port_;
     IIncomingHttpRequestFilter* filter_;
@@ -145,9 +146,9 @@
 
     // set the minimum accepted version of SSL/TLS protocol according to the CivetWeb table published here:
     // https://github.com/civetweb/civetweb/blob/master/docs/UserManual.md#ssl_protocol_version-0
-    void SetSslMinimumVersion(std::string version);
+    void SetSslMinimumVersion(unsigned int version);
 
-    void SetSslCiphers(std::string ciphers);
+    void SetSslCiphers(const std::list<std::string>& ciphers);
     
     void SetSslTrustedClientCertificates(const char* path);
 
--- a/OrthancServer/Resources/Configuration.json	Fri Dec 11 11:59:10 2020 -0500
+++ b/OrthancServer/Resources/Configuration.json	Thu Dec 17 15:10:04 2020 +0100
@@ -171,19 +171,24 @@
   "SslCertificate" : "certificate.pem",
 
   // Sets the minimum accepted SSL protocol version
-  // See https://github.com/civetweb/civetweb/blob/master/docs/UserManual.md 
-  // "ssl_protocol_version" for mapping
-  // By default require SSL 1.2
-  // This option is only meaningful if "SslEnabled" is true.
+  // (cf. "ssl_protocol_version" option of civetweb). By default,
+  // require SSL 1.2. This option is only meaningful if "SslEnabled"
+  // is true. (new in Orthanc 1.8.2)
+  //
+  // Value => Protocols
+  //   0      SSL2+SSL3+TLS1.0+TLS1.1+TLS1.2
+  //   1      SSL3+TLS1.0+TLS1.1+TLS1.2
+  //   2      TLS1.0+TLS1.1+TLS1.2
+  //   3      TLS1.1+TLS1.2
+  //   4      TLS1.2
+  "SslMinimumProtocolVersion" : 4,
+
+  // Set the accepted ciphers for SSL connections. The ciphers must be
+  // provided as a list of strings. If not set, this will default to
+  // FIPS 140-2 ciphers. This option is only meaningful if
+  // "SslEnabled" is true. (new in Orthanc 1.8.2)
   /**
-    "SslMinimumProtocolVersion" : "4",
-  **/
-
-  // Set the allowed ciphers for SSL connections
-  // If not set, this will default to FIPS 140-2 ciphers
-  // This option is only meaningful if "SslEnabled" is true.
-  /**
-    "SslCiphersAccepted" : "",
+    "SslCiphersAccepted" : [ "AES128-GCM-SHA256" ],
   **/
 
   // Whether or not peer client certificates shall be checked. This
--- a/OrthancServer/Sources/main.cpp	Fri Dec 11 11:59:10 2020 -0500
+++ b/OrthancServer/Sources/main.cpp	Thu Dec 17 15:10:04 2020 +0100
@@ -1046,35 +1046,40 @@
         
         // Default to TLS 1.2 as SSL minimum
         // See https://github.com/civetweb/civetweb/blob/master/docs/UserManual.md "ssl_protocol_version" for mapping
-        std::string tls1_2 = "4";
-        std::string minimumVersion = lock.GetConfiguration().GetStringParameter("SslMinimumProtocolVersion", tls1_2);
+        static const unsigned int TLS_1_2 = 4;
+        unsigned int minimumVersion = lock.GetConfiguration().GetUnsignedIntegerParameter("SslMinimumProtocolVersion", TLS_1_2);
         httpServer.SetSslMinimumVersion(minimumVersion);
 
-        // Default to FIPS 140-2 ciphers 
-        const std::vector<std::string> fipsCiphers = { 
-          "ECDHE-ECDSA-AES256-GCM-SHA384", 
-          "ECDHE-ECDSA-AES256-SHA384",
-          "ECDHE-RSA-AES256-GCM-SHA384", 
-          "ECDHE-RSA-AES128-GCM-SHA256", 
-          "ECDHE-RSA-AES256-SHA384",
-          "ECDHE-RSA-AES128-SHA256", 
-          "ECDHE-RSA-AES128-SHA", 
-          "ECDHE-RSA-AES256-SHA", 
-          "DHE-RSA-AES256-SHA",
-          "DHE-RSA-AES128-SHA", 
-          "AES256-SHA",
-          "AES128-SHA"};
+        static const char* SSL_CIPHERS_ACCEPTED = "SslCiphersAccepted";
+
+        std::list<std::string> ciphers;
+
+        if (lock.GetJson().type() == Json::objectValue &&
+            lock.GetJson().isMember(SSL_CIPHERS_ACCEPTED))
+        {
+          lock.GetConfiguration().GetListOfStringsParameter(ciphers, SSL_CIPHERS_ACCEPTED);
+        }
+        else
+        {
+          // Defaults to FIPS 140-2 ciphers 
+          CLOG(INFO, HTTP) << "No configuration option \"" << SSL_CIPHERS_ACCEPTED
+                           << "\", will accept the FIPS 140-2 ciphers";
 
-        // Format default cipher string
-        std::string defaultCipherString;
-        for (const auto &cipher : fipsCiphers)
-        {
-          defaultCipherString += cipher + ":";
+          ciphers.push_back("ECDHE-ECDSA-AES256-GCM-SHA384");
+          ciphers.push_back("ECDHE-ECDSA-AES256-SHA384");
+          ciphers.push_back("ECDHE-RSA-AES256-GCM-SHA384");
+          ciphers.push_back("ECDHE-RSA-AES128-GCM-SHA256");
+          ciphers.push_back("ECDHE-RSA-AES256-SHA384");
+          ciphers.push_back("ECDHE-RSA-AES128-SHA256");
+          ciphers.push_back("ECDHE-RSA-AES128-SHA");
+          ciphers.push_back("ECDHE-RSA-AES256-SHA");
+          ciphers.push_back("DHE-RSA-AES256-SHA");
+          ciphers.push_back("DHE-RSA-AES128-SHA");
+          ciphers.push_back("AES256-SHA");
+          ciphers.push_back("AES128-SHA");
         }
-        defaultCipherString.pop_back();
-
-        std::string ciphersAccepted = lock.GetConfiguration().GetStringParameter("SslCiphersAccepted", defaultCipherString); 
-        httpServer.SetSslCiphers(ciphersAccepted);
+        
+        httpServer.SetSslCiphers(ciphers);
       }
       else
       {