changeset 3506:d2b9981017c4

better handling of HTTP security
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 28 Aug 2019 15:19:04 +0200
parents b2d4dd16dae8
children 69e49fc044f8
files Core/Logging.cpp NEWS OrthancServer/OrthancConfiguration.cpp OrthancServer/OrthancConfiguration.h OrthancServer/main.cpp Resources/Configuration.json
diffstat 6 files changed, 52 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/Core/Logging.cpp	Wed Aug 28 12:21:23 2019 +0200
+++ b/Core/Logging.cpp	Wed Aug 28 15:19:04 2019 +0200
@@ -161,7 +161,7 @@
 #include <boost/lexical_cast.hpp>
 
 #ifdef __EMSCRIPTEN__
-#include "emscripten/html5.h"
+#  include <emscripten/html5.h>
 #endif
 
 namespace Orthanc
--- a/NEWS	Wed Aug 28 12:21:23 2019 +0200
+++ b/NEWS	Wed Aug 28 15:19:04 2019 +0200
@@ -4,7 +4,7 @@
 Maintenance
 -----------
 
-* Security: If remote HTTP access is enabled, HTTP authentication automatically gets enabled
+* Security: If remote access is enabled, HTTP authentication is also enabled by default
 * Log an explicit error if uploading an empty DICOM file using REST API
 * Fix compatibility of LSB binaries with Ubuntu >= 18.04
 
--- a/OrthancServer/OrthancConfiguration.cpp	Wed Aug 28 12:21:23 2019 +0200
+++ b/OrthancServer/OrthancConfiguration.cpp	Wed Aug 28 15:19:04 2019 +0200
@@ -484,8 +484,8 @@
   }
 
 
-  bool OrthancConfiguration::GetBooleanParameter(const std::string& parameter,
-                                                 bool defaultValue) const
+  bool OrthancConfiguration::LookupBooleanParameter(bool& target,
+                                                    const std::string& parameter) const
   {
     if (json_.isMember(parameter))
     {
@@ -497,11 +497,27 @@
       }
       else
       {
-        return json_[parameter].asBool();
+        target = json_[parameter].asBool();
+        return true;
       }
     }
     else
     {
+      return false;
+    }
+  }
+
+
+  bool OrthancConfiguration::GetBooleanParameter(const std::string& parameter,
+                                                 bool defaultValue) const
+  {
+    bool value;
+    if (LookupBooleanParameter(value, parameter))
+    {
+      return value;
+    }
+    else
+    {
       return defaultValue;
     }
   }
--- a/OrthancServer/OrthancConfiguration.h	Wed Aug 28 12:21:23 2019 +0200
+++ b/OrthancServer/OrthancConfiguration.h	Wed Aug 28 15:19:04 2019 +0200
@@ -172,6 +172,9 @@
     unsigned int GetUnsignedIntegerParameter(const std::string& parameter,
                                              unsigned int defaultValue) const;
 
+    bool LookupBooleanParameter(bool& target,
+                                const std::string& parameter) const;
+
     bool GetBooleanParameter(const std::string& parameter,
                              bool defaultValue) const;
 
--- a/OrthancServer/main.cpp	Wed Aug 28 12:21:23 2019 +0200
+++ b/OrthancServer/main.cpp	Wed Aug 28 15:19:04 2019 +0200
@@ -823,20 +823,32 @@
       httpServer.SetHttpCompressionEnabled(lock.GetConfiguration().GetBooleanParameter("HttpCompressionEnabled", true));
       httpServer.SetTcpNoDelay(lock.GetConfiguration().GetBooleanParameter("TcpNoDelay", true));
 
-      bool authenticationEnabled = lock.GetConfiguration().GetBooleanParameter("AuthenticationEnabled", false);
-      if (httpServer.IsRemoteAccessAllowed())
+      bool authenticationEnabled;
+      if (lock.GetConfiguration().LookupBooleanParameter(authenticationEnabled, "AuthenticationEnabled"))
       {
-        if (!authenticationEnabled)
+        httpServer.SetAuthenticationEnabled(authenticationEnabled);
+
+        if (httpServer.IsRemoteAccessAllowed() &&
+            !authenticationEnabled)
         {
-          LOG(WARNING) << "Remote access is allowed, automatically turning on HTTP authentication for security";
+          LOG(WARNING) << "Remote access is enabled while user authentication is disabled, "
+                       << "make sure this does not affect the security of your setup";
         }
-
-        // Starting with Orthanc 1.5.8, enabling remote access forces user authentication.
+      }
+      else if (httpServer.IsRemoteAccessAllowed())
+      {
+        // Starting with Orthanc 1.5.8, it is impossible to enable
+        // remote access without having explicitly disabled user
+        // authentication.
+        LOG(WARNING) << "Remote access is allowed but \"AuthenticationEnabled\" is not in the configuration, "
+                     << "automatically enabling HTTP authentication for security";          
         httpServer.SetAuthenticationEnabled(true);
       }
       else
       {
-        httpServer.SetAuthenticationEnabled(authenticationEnabled);
+        // If Orthanc only listens on the localhost, it is OK to have
+        // "AuthenticationEnabled" disabled
+        httpServer.SetAuthenticationEnabled(false);
       }
 
       bool hasUsers = lock.GetConfiguration().SetupRegisteredUsers(httpServer);
--- a/Resources/Configuration.json	Wed Aug 28 12:21:23 2019 +0200
+++ b/Resources/Configuration.json	Wed Aug 28 15:19:04 2019 +0200
@@ -139,10 +139,7 @@
    * Security-related options for the HTTP server
    **/
 
-  // Whether remote hosts can connect to the HTTP server. For security
-  // reasons, starting with Orthanc 1.5.8, as soon as this option is
-  // set to "true", authentication is enabled, and you have to declare
-  // an user in "RegisteredUsers" to access the HTTP server.
+  // Whether remote hosts can connect to the HTTP server
   "RemoteAccessAllowed" : false,
 
   // Whether or not SSL is enabled
@@ -152,10 +149,14 @@
   // SSL is enabled)
   "SslCertificate" : "certificate.pem",
 
-  // Whether or not the password protection is enabled. Starting with
-  // Orthanc 1.5.8, password protection is automatically enabled as
-  // soon as "RemoteAccessAllowed" is set to "true".
-  "AuthenticationEnabled" : false,
+  // Whether or not the password protection is enabled (using HTTP
+  // basic access authentication). Starting with Orthanc 1.5.8, if
+  // "AuthenticationEnabled" is not explicitly set, authentication is
+  // enabled iff. remote access is allowed (i.e. the default value of
+  // "AuthenticationEnabled" equals that of "RemoteAccessAllowed").
+  /**
+     "AuthenticationEnabled" : false,
+   **/
 
   // The list of the registered users. Because Orthanc uses HTTP
   // Basic Authentication, the passwords are stored as plain text.