Mercurial > hg > orthanc

changeset 4888:8523078f3f4b

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
added new configuration to authorize C-Find for worklist independently from other C-Find
author Alain Mazy <am@osimis.io>
date Fri, 18 Feb 2022 15:45:28 +0100
parents a29a6bdba9dd
children d8c8274d4e41
files NEWS OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.h OrthancFramework/Sources/Enumerations.cpp OrthancFramework/Sources/Enumerations.h OrthancFramework/UnitTestsSources/JobsTests.cpp OrthancServer/Resources/Configuration.json OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp OrthancServer/Sources/main.cpp
diffstat 10 files changed, 81 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed Feb 16 09:27:25 2022 +0100
+++ b/NEWS	Fri Feb 18 15:45:28 2022 +0100
@@ -4,6 +4,18 @@
 General
 -------
 
+* New configuration "DicomAlwaysAllowFindWorklist" to complement the existing
+  "DicomAlwaysAllowFind" configuration.  "DicomAlwaysAllowFind" applies now 
+  only to C-Find for Patients/Studies/Series/Instances while C-Find for worklists are
+  covered by "DicomAlwaysAllowFindWorklist".  The same changes applies to new 
+  configurations in "DicomModalities": "AllowFind" is now complemented by 
+  "AllowFindWorklist".
+  This new option allows improved security management.  E.g: a modality might have
+  only "AllowStore" and "AllowFindWorklist" enabled but might have "AllowFind"
+  disabled to prevent listing past patient studies.
+  Possible BREAKING-CHANGE: if you relied on "DicomAlwaysAllowFind" or "AllowFind"
+  to specifically authorize C-Find for worklist, you now need to explicitely enable
+  "DicomAlwaysAllowFindWorklist" and/or "AllowFindWorklist"
 * Added a storage cache in RAM to avoid reading the same files multiple times from 
   the storage.  This greatly improves, among other things, the performance of WADO-RS
   retrieval of individual frames of multiframe instances.
--- a/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -788,10 +788,18 @@
             break;
 
           case DIMSE_C_FIND_RQ:
-            request = DicomRequestType_Find;
-            supported = true;
-            break;
-
+            {
+              std::string sopClassUid(msg.msg.CFindRQ.AffectedSOPClassUID);
+              if (sopClassUid == UID_FINDModalityWorklistInformationModel)
+              {
+                request = DicomRequestType_FindWorklist;
+              }
+              else
+              {
+                request = DicomRequestType_Find;
+              }
+              supported = true;
+            }; break;
           case DIMSE_N_ACTION_RQ:
             request = DicomRequestType_NAction;
             supported = true;
@@ -875,6 +883,7 @@
               break;
 
             case DicomRequestType_Find:
+            case DicomRequestType_FindWorklist:
               if (server_.HasFindRequestHandlerFactory() || // Should always be true
                   server_.HasWorklistRequestHandlerFactory())
               {
--- a/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -35,6 +35,7 @@
 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_FIND_WORKLIST = "AllowFindWorklist";
 static const char* KEY_ALLOW_GET = "AllowGet";
 static const char* KEY_ALLOW_MOVE = "AllowMove";
 static const char* KEY_ALLOW_N_ACTION = "AllowNAction";
@@ -61,6 +62,7 @@
     allowEcho_ = true;
     allowStore_ = true;
     allowFind_ = true;
+    allowFindWorklist_ = true;
     allowMove_ = true;
     allowGet_ = true;
     allowNAction_ = true;  // For storage commitment
@@ -250,6 +252,11 @@
       allowFind_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_FIND);
     }
 
+    if (serialized.isMember(KEY_ALLOW_FIND_WORKLIST))
+    {
+      allowFindWorklist_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_FIND_WORKLIST);
+    }
+
     if (serialized.isMember(KEY_ALLOW_STORE))
     {
       allowStore_ = SerializationToolbox::ReadBoolean(serialized, KEY_ALLOW_STORE);
@@ -314,6 +321,9 @@
       case DicomRequestType_Find:
         return allowFind_;
 
+      case DicomRequestType_FindWorklist:
+        return allowFindWorklist_;
+
       case DicomRequestType_Get:
         return allowGet_;
 
@@ -348,6 +358,10 @@
         allowFind_ = allowed;
         break;
 
+      case DicomRequestType_FindWorklist:
+        allowFindWorklist_ = allowed;
+        break;
+
       case DicomRequestType_Get:
         allowGet_ = allowed;
         break;
@@ -379,6 +393,7 @@
     return (!allowEcho_ ||
             !allowStore_ ||
             !allowFind_ ||
+            !allowFindWorklist_ ||
             !allowGet_ ||
             !allowMove_ ||
             !allowNAction_ ||
@@ -403,6 +418,7 @@
       target[KEY_ALLOW_ECHO] = allowEcho_;
       target[KEY_ALLOW_STORE] = allowStore_;
       target[KEY_ALLOW_FIND] = allowFind_;
+      target[KEY_ALLOW_FIND_WORKLIST] = allowFindWorklist_;
       target[KEY_ALLOW_GET] = allowGet_;
       target[KEY_ALLOW_MOVE] = allowMove_;
       target[KEY_ALLOW_N_ACTION] = allowNAction_;
--- a/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.h	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.h	Fri Feb 18 15:45:28 2022 +0100
@@ -41,6 +41,7 @@
     bool                  allowEcho_;
     bool                  allowStore_;
     bool                  allowFind_;
+    bool                  allowFindWorklist_;
     bool                  allowMove_;
     bool                  allowGet_;
     bool                  allowNAction_;
--- a/OrthancFramework/Sources/Enumerations.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/Sources/Enumerations.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -838,6 +838,10 @@
         return "Find";
         break;
 
+      case DicomRequestType_FindWorklist:
+        return "Find Worklist";
+        break;
+
       case DicomRequestType_Get:
         return "Get";
         break;
--- a/OrthancFramework/Sources/Enumerations.h	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/Sources/Enumerations.h	Fri Feb 18 15:45:28 2022 +0100
@@ -628,6 +628,7 @@
   {
     DicomRequestType_Echo,
     DicomRequestType_Find,
+    DicomRequestType_FindWorklist,
     DicomRequestType_Get,
     DicomRequestType_Move,
     DicomRequestType_Store,
--- a/OrthancFramework/UnitTestsSources/JobsTests.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancFramework/UnitTestsSources/JobsTests.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -1312,6 +1312,7 @@
     ASSERT_EQ(ModalityManufacturer_Generic, modality.GetManufacturer());
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Echo));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Find));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_FindWorklist));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Get));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Store));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Move));
@@ -1351,6 +1352,7 @@
     ASSERT_EQ(ModalityManufacturer_GenericNoWildcardInDates, modality.GetManufacturer());
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Echo));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Find));
+    ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_FindWorklist));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Get));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Store));
     ASSERT_TRUE(modality.IsRequestAllowed(DicomRequestType_Move));
@@ -1377,6 +1379,7 @@
   std::set<DicomRequestType> operations;
   operations.insert(DicomRequestType_Echo);
   operations.insert(DicomRequestType_Find);
+  operations.insert(DicomRequestType_FindWorklist);
   operations.insert(DicomRequestType_Get);
   operations.insert(DicomRequestType_Move);
   operations.insert(DicomRequestType_Store);
--- a/OrthancServer/Resources/Configuration.json	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancServer/Resources/Configuration.json	Fri Feb 18 15:45:28 2022 +0100
@@ -289,8 +289,17 @@
   // from SCU modalities it does not know about (i.e. that are not
   // listed in the "DicomModalities" option above). Setting this
   // option to "true" implies security risks. (new in Orthanc 1.9.0)
+  // Note: from version 1.10.0, this option applies to C-FIND requests
+  // for Patients/Studies/Series/Instances. Another option is available
+  // for Worklists (see below)
   "DicomAlwaysAllowFind" : false,
 
+  // Whether the Orthanc SCP allows incoming C-FIND requests for worklists, 
+  // even from SCU modalities it does not know about (i.e. that are not
+  // listed in the "DicomModalities" option above). Setting this
+  // option to "true" implies security risks. (new in Orthanc 1.10.0)
+  "DicomAlwaysAllowFindWorklist" : false,
+
   // Whether the Orthanc SCP allows incoming C-GET requests, even
   // from SCU modalities it does not know about (i.e. that are not
   // listed in the "DicomModalities" option above). Setting this
@@ -387,6 +396,7 @@
     //  "Manufacturer" : "Generic",
     //  "AllowEcho" : false,
     //  "AllowFind" : false,
+    //  "AllowFindWorklist" : false,       // new in 1.10.0
     //  "AllowGet"  : false,
     //  "AllowMove" : false,
     //  "AllowStore" : true,
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -135,6 +135,8 @@
                          "Whether to accept C-STORE SCU commands issued by the remote modality", false)
         .SetRequestField("AllowFind", RestApiCallDocumentation::Type_Boolean,
                          "Whether to accept C-FIND SCU commands issued by the remote modality", false)
+        .SetRequestField("AllowFindWorklist", RestApiCallDocumentation::Type_Boolean,
+                         "Whether to accept C-FIND SCU commands for worklists issued by the remote modality", false)
         .SetRequestField("AllowMove", RestApiCallDocumentation::Type_Boolean,
                          "Whether to accept C-MOVE SCU commands issued by the remote modality", false)
         .SetRequestField("AllowGet", RestApiCallDocumentation::Type_Boolean,
@@ -2035,6 +2037,7 @@
       sample["AllowEcho"] = true;
       sample["AllowEventReport"] = true;
       sample["AllowFind"] = true;
+      sample["AllowFindWorklist"] = true;
       sample["AllowGet"] = true;
       sample["AllowMove"] = true;
       sample["AllowNAction"] = true;
--- a/OrthancServer/Sources/main.cpp	Wed Feb 16 09:27:25 2022 +0100
+++ b/OrthancServer/Sources/main.cpp	Fri Feb 18 15:45:28 2022 +0100
@@ -282,6 +282,7 @@
   ServerContext&  context_;
   bool            alwaysAllowEcho_;
   bool            alwaysAllowFind_;  // New in Orthanc 1.9.0
+  bool            alwaysAllowFindWorklist_; // New in Orthanc 1.10.0
   bool            alwaysAllowGet_;   // New in Orthanc 1.9.0
   bool            alwaysAllowMove_;  // New in Orthanc 1.9.7
   bool            alwaysAllowStore_;
@@ -294,6 +295,7 @@
       OrthancConfiguration::ReaderLock lock;
       alwaysAllowEcho_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowEcho", true);
       alwaysAllowFind_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowFind", false);
+      alwaysAllowFindWorklist_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowFindWorklist", false);
       alwaysAllowGet_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowGet", false);
       alwaysAllowMove_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowMove", false);
       alwaysAllowStore_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowStore", true);
@@ -304,6 +306,11 @@
       LOG(WARNING) << "Security risk in DICOM SCP: C-FIND requests are always allowed, even from unknown modalities";
     }
 
+    if (alwaysAllowFindWorklist_)
+    {
+      LOG(WARNING) << "Security risk in DICOM SCP: C-FIND requests for worklists are always allowed, even from unknown modalities";
+    }
+
     if (alwaysAllowGet_)
     {
       LOG(WARNING) << "Security risk in DICOM SCP: C-GET requests are always allowed, even from unknown modalities";
@@ -324,6 +331,7 @@
 
     if (alwaysAllowEcho_ ||
         alwaysAllowFind_ ||
+        alwaysAllowFindWorklist_ ||
         alwaysAllowGet_ ||
         alwaysAllowMove_ ||
         alwaysAllowStore_)
@@ -341,7 +349,7 @@
                                       const std::string& remoteAet,
                                       DicomRequestType type)
   {
-    LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+    LOG(WARNING) << "DICOM authorization rejected for AET " << remoteAet
                  << " on IP " << remoteIp << ": The DICOM command "
                  << EnumerationToString(type) << " is not allowed for this modality "
                  << "according to configuration option \"DicomModalities\"";
@@ -368,6 +376,12 @@
       // Incoming C-Find requests are always accepted, even from unknown AET
       return true;
     }
+    else if (type == DicomRequestType_FindWorklist &&
+             alwaysAllowFindWorklist_)
+    {
+      // Incoming C-Find requests for worklists are always accepted, even from unknown AET
+      return true;
+    }
     else if (type == DicomRequestType_Store &&
              alwaysAllowStore_)
     {
@@ -399,7 +413,7 @@
       
       if (modalities.empty())
       {
-        LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+        LOG(WARNING) << "DICOM authorization rejected  for AET " << remoteAet
                      << " on IP " << remoteIp << ": This AET is not listed in "
                      << "configuration option \"DicomModalities\"";
         return false;
@@ -410,7 +424,7 @@
         if (checkIp &&
             remoteIp != modalities.front().GetHost())
         {
-          LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+          LOG(WARNING) << "DICOM authorization rejected for AET " << remoteAet
                        << " on IP " << remoteIp << ": Its IP address should be "
                        << modalities.front().GetHost()
                        << " according to configuration option \"DicomModalities\"";
@@ -446,7 +460,7 @@
           }
         }
 
-        LOG(WARNING) << "Unable to check DICOM authorization for AET " << remoteAet
+        LOG(WARNING) << "DICOM authorization rejected for AET " << remoteAet
                      << " on IP " << remoteIp << ": " << modalities.size()
                      << " modalites found with this AET in configuration option "
                      << "\"DicomModalities\", but none of them matches the IP";