changeset 2872:9d08edde614b

Possibility to restrict the allowed DICOM commands for each modality
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 09 Oct 2018 14:19:48 +0200
parents 6eebc2eb3168
children 703d1e848907
files Core/DicomNetworking/RemoteModalityParameters.cpp Core/DicomNetworking/RemoteModalityParameters.h NEWS OrthancServer/main.cpp Resources/Configuration.json UnitTestsSources/MultiThreadingTests.cpp
diffstat 6 files changed, 238 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- a/Core/DicomNetworking/RemoteModalityParameters.cpp	Tue Oct 09 12:51:20 2018 +0200
+++ b/Core/DicomNetworking/RemoteModalityParameters.cpp	Tue Oct 09 14:19:48 2018 +0200
@@ -43,26 +43,38 @@
 
 
 static const char* KEY_AET = "AET";
+static const char* KEY_ALLOW_ECHO = "AllowEcho";
+static const char* KEY_ALLOW_FIND = "AllowFind";
+static const char* KEY_ALLOW_GET = "AllowGet";
+static const char* KEY_ALLOW_MOVE = "AllowMove";
+static const char* KEY_ALLOW_STORE = "AllowStore";
+static const char* KEY_HOST = "Host";
 static const char* KEY_MANUFACTURER = "Manufacturer";
 static const char* KEY_PORT = "Port";
-static const char* KEY_HOST = "Host";
 
 
 namespace Orthanc
 {
-  RemoteModalityParameters::RemoteModalityParameters() :
-    aet_("ORTHANC"),
-    host_("127.0.0.1"),
-    port_(104),
-    manufacturer_(ModalityManufacturer_Generic)
+  void RemoteModalityParameters::Clear()
   {
+    aet_ = "ORTHANC";
+    host_ = "127.0.0.1";
+    port_ = 104;
+    manufacturer_ = ModalityManufacturer_Generic;
+    allowEcho_ = true;
+    allowStore_ = true;
+    allowFind_ = true;
+    allowMove_ = true;
+    allowGet_ = true;
   }
 
+
   RemoteModalityParameters::RemoteModalityParameters(const std::string& aet,
                                                      const std::string& host,
                                                      uint16_t port,
                                                      ModalityManufacturer manufacturer)
   {
+    Clear();
     SetApplicationEntityTitle(aet);
     SetHost(host);
     SetPortNumber(port);
@@ -70,6 +82,17 @@
   }
 
 
+  static void CheckPortNumber(int value)
+  {
+    if (value <= 0 || 
+        value >= 65535)
+    {
+      LOG(ERROR) << "A TCP port number must be in range [1..65534], but found: " << value;
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+  }
+
+
   static uint16_t ReadPortNumber(const Json::Value& value)
   {
     int tmp;
@@ -96,15 +119,15 @@
         throw OrthancException(ErrorCode_BadFileFormat);
     }
 
-    if (tmp <= 0 || tmp >= 65535)
-    {
-      LOG(ERROR) << "A TCP port number must be in range [1..65534], but found: " << tmp;
-      throw OrthancException(ErrorCode_ParameterOutOfRange);
-    }
-    else
-    {
-      return static_cast<uint16_t>(tmp);
-    }
+    CheckPortNumber(tmp);
+    return static_cast<uint16_t>(tmp);
+  }
+
+
+  void RemoteModalityParameters::SetPortNumber(uint16_t port)
+  {
+    CheckPortNumber(port);
+    port_ = port;
   }
 
 
@@ -162,12 +185,97 @@
     {
       manufacturer_ = ModalityManufacturer_Generic;
     }
+
+    if (serialized.isMember(KEY_ALLOW_ECHO))
+    {
+      allowEcho_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_ECHO);
+    }
+
+    if (serialized.isMember(KEY_ALLOW_FIND))
+    {
+      allowFind_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_FIND);
+    }
+
+    if (serialized.isMember(KEY_ALLOW_STORE))
+    {
+      allowStore_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_STORE);
+    }
+
+    if (serialized.isMember(KEY_ALLOW_GET))
+    {
+      allowGet_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_GET);
+    }
+
+    if (serialized.isMember(KEY_ALLOW_MOVE))
+    {
+      allowMove_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_MOVE);
+    }
+  }
+
+
+  bool RemoteModalityParameters::IsRequestAllowed(DicomRequestType type) const
+  {
+    switch (type)
+    {
+      case DicomRequestType_Echo:
+        return allowEcho_;
+
+      case DicomRequestType_Find:
+        return allowFind_;
+
+      case DicomRequestType_Get:
+        return allowGet_;
+
+      case DicomRequestType_Move:
+        return allowMove_;
+
+      case DicomRequestType_Store:
+        return allowStore_;
+
+      default:
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+  }
+
+
+  void RemoteModalityParameters::SetRequestAllowed(DicomRequestType type,
+                                                   bool allowed)
+  {
+    switch (type)
+    {
+      case DicomRequestType_Echo:
+        allowEcho_ = allowed;
+        break;
+
+      case DicomRequestType_Find:
+        allowFind_ = allowed;
+        break;
+
+      case DicomRequestType_Get:
+        allowGet_ = allowed;
+        break;
+
+      case DicomRequestType_Move:
+        allowMove_ = allowed;
+        break;
+
+      case DicomRequestType_Store:
+        allowStore_ = allowed;
+        break;
+
+      default:
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
   }
 
 
   bool RemoteModalityParameters::IsAdvancedFormatNeeded() const
   {
-    return false; // TODO
+    return (!allowEcho_ ||
+            !allowStore_ ||
+            !allowFind_ ||
+            !allowGet_ ||
+            !allowMove_);
   }
 
   
@@ -182,6 +290,11 @@
       target[KEY_HOST] = host_;
       target[KEY_PORT] = port_;
       target[KEY_MANUFACTURER] = EnumerationToString(manufacturer_);
+      target[KEY_ALLOW_ECHO] = allowEcho_;
+      target[KEY_ALLOW_STORE] = allowStore_;
+      target[KEY_ALLOW_FIND] = allowFind_;
+      target[KEY_ALLOW_GET] = allowGet_;
+      target[KEY_ALLOW_MOVE] = allowMove_;
     }
     else
     {
@@ -196,6 +309,8 @@
   
   void RemoteModalityParameters::Unserialize(const Json::Value& serialized)
   {
+    Clear();
+
     switch (serialized.type())
     {
       case Json::objectValue:
--- a/Core/DicomNetworking/RemoteModalityParameters.h	Tue Oct 09 12:51:20 2018 +0200
+++ b/Core/DicomNetworking/RemoteModalityParameters.h	Tue Oct 09 14:19:48 2018 +0200
@@ -44,17 +44,27 @@
   class RemoteModalityParameters
   {
   private:
-    std::string aet_;
-    std::string host_;
-    uint16_t port_;
-    ModalityManufacturer manufacturer_;
+    std::string           aet_;
+    std::string           host_;
+    uint16_t              port_;
+    ModalityManufacturer  manufacturer_;
+    bool                  allowEcho_;
+    bool                  allowStore_;
+    bool                  allowFind_;
+    bool                  allowMove_;
+    bool                  allowGet_;
+
+    void Clear();
 
     void UnserializeArray(const Json::Value& serialized);
 
     void UnserializeObject(const Json::Value& serialized);
 
   public:
-    RemoteModalityParameters();
+    RemoteModalityParameters()
+    {
+      Clear();
+    }
 
     RemoteModalityParameters(const Json::Value& serialized)
     {
@@ -91,10 +101,7 @@
       return port_;
     }
 
-    void SetPortNumber(uint16_t port)
-    {
-      port_ = port;
-    }
+    void SetPortNumber(uint16_t port);
 
     ModalityManufacturer GetManufacturer() const
     {
@@ -111,6 +118,11 @@
       manufacturer_ = StringToModalityManufacturer(manufacturer);
     }
 
+    bool IsRequestAllowed(DicomRequestType type) const;
+
+    void SetRequestAllowed(DicomRequestType type,
+                           bool allowed);
+
     void Unserialize(const Json::Value& modality);
 
     bool IsAdvancedFormatNeeded() const;
--- a/NEWS	Tue Oct 09 12:51:20 2018 +0200
+++ b/NEWS	Tue Oct 09 14:19:48 2018 +0200
@@ -2,6 +2,11 @@
 ===============================
 
 
+General
+-------
+
+* Possibility to restrict the allowed DICOM commands for each modality
+
 REST API
 --------
 
--- a/OrthancServer/main.cpp	Tue Oct 09 12:51:20 2018 +0200
+++ b/OrthancServer/main.cpp	Tue Oct 09 14:19:48 2018 +0200
@@ -212,13 +212,18 @@
       // Incoming C-Store requests are always accepted, even from unknown AET
       return true;
     }
-    else if (!Configuration::IsKnownAETitle(remoteAet, remoteIp))
-    {
-      return false;
-    }
     else
     {
-      return true;
+      RemoteModalityParameters modality;
+    
+      if (Configuration::LookupDicomModalityUsingAETitle(modality, remoteAet))
+      {
+        return modality.IsRequestAllowed(type);
+      }
+      else
+      {
+        return false;
+      }
     }
   }
 
--- a/Resources/Configuration.json	Tue Oct 09 12:51:20 2018 +0200
+++ b/Resources/Configuration.json	Tue Oct 09 14:19:48 2018 +0200
@@ -174,6 +174,23 @@
      * This parameter is case-sensitive.
      **/
     // "clearcanvas" : [ "CLEARCANVAS", "192.168.1.1", 104, "ClearCanvas" ]
+
+    /**
+     * By default, the Orthanc SCP accepts all DICOM commands (C-GET,
+     * C-STORE, C-FIND, C-MOVE) issued by the remote SCU
+     * modalities. Starting with Orthanc 1.4.3, it is possible to
+     * specify which DICOM commands are allowed, separately for each
+     * remote modality, using the syntax below.
+     **/
+    //"untrusted" : {
+    //  "AET" : "ORTHANC",
+    //  "Port" : 104,
+    //  "Host" : "127.0.0.1",
+    //  "AllowEcho" : false,
+    //  "AllowFind" : false,
+    //  "AllowMove" : false,
+    //  "AllowStore" : true
+    //}
   },
 
   // Whether the Orthanc SCP allows incoming C-Echo requests, even
--- a/UnitTestsSources/MultiThreadingTests.cpp	Tue Oct 09 12:51:20 2018 +0200
+++ b/UnitTestsSources/MultiThreadingTests.cpp	Tue Oct 09 14:19:48 2018 +0200
@@ -1875,6 +1875,7 @@
 
   {
     RemoteModalityParameters modality;
+    ASSERT_FALSE(modality.IsAdvancedFormatNeeded());
     modality.Serialize(s, false);
     ASSERT_EQ(Json::arrayValue, s.type());
   }
@@ -1885,6 +1886,11 @@
     ASSERT_EQ("127.0.0.1", modality.GetHost());
     ASSERT_EQ(104u, modality.GetPortNumber());
     ASSERT_EQ(ModalityManufacturer_Generic, modality.GetManufacturer());
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Echo));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Find));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Get));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Store));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Move));
   }
 
   s = Json::nullValue;
@@ -1892,6 +1898,8 @@
   {
     RemoteModalityParameters modality;
     ASSERT_FALSE(modality.IsAdvancedFormatNeeded());
+    ASSERT_THROW(modality.SetPortNumber(0), OrthancException);
+    ASSERT_THROW(modality.SetPortNumber(65535), OrthancException);
     modality.SetApplicationEntityTitle("HELLO");
     modality.SetHost("world");
     modality.SetPortNumber(45);
@@ -1906,6 +1914,11 @@
     ASSERT_EQ("world", modality.GetHost());
     ASSERT_EQ(45u, modality.GetPortNumber());
     ASSERT_EQ(ModalityManufacturer_Dcm4Chee, modality.GetManufacturer());
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Echo));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Find));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Get));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Store));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Move));
   }
 
   s["Port"] = "46";
@@ -1914,4 +1927,45 @@
     RemoteModalityParameters modality(s);
     ASSERT_EQ(46u, modality.GetPortNumber());
   }
+
+  s["Port"] = -1;     ASSERT_THROW(RemoteModalityParameters m(s), OrthancException);
+  s["Port"] = 65535;  ASSERT_THROW(RemoteModalityParameters m(s), OrthancException);
+  s["Port"] = "nope"; ASSERT_THROW(RemoteModalityParameters m(s), OrthancException);
+
+  std::set<DicomRequestType> operations;
+  operations.insert(DicomRequestType_Echo);
+  operations.insert(DicomRequestType_Find);
+  operations.insert(DicomRequestType_Get);
+  operations.insert(DicomRequestType_Move);
+  operations.insert(DicomRequestType_Store);
+
+  ASSERT_EQ(5u, operations.size());
+
+  for (std::set<DicomRequestType>::const_iterator 
+         it = operations.begin(); it != operations.end(); ++it)
+  {
+    {
+      RemoteModalityParameters modality;
+      modality.SetRequestAllowed(*it, false);
+      ASSERT_TRUE(modality.IsAdvancedFormatNeeded());
+
+      modality.Serialize(s, false);
+      ASSERT_EQ(Json::objectValue, s.type());
+    }
+
+    {
+      RemoteModalityParameters modality(s);
+
+      ASSERT_FALSE(modality.IsRequestAllowed(*it));
+
+      for (std::set<DicomRequestType>::const_iterator 
+             it2 = operations.begin(); it2 != operations.end(); ++it2)
+      {
+        if (*it2 != *it)
+        {
+          ASSERT_TRUE(modality.IsRequestAllowed(*it2));
+        }
+      }
+    }
+  }
 }