Mercurial > hg > orthanc

changeset 4482:8efeaba1b7f9

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
new configuration options: "DicomAlwaysAllowFind" and "DicomAlwaysAllowGet"
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 28 Jan 2021 15:54:30 +0100
parents fe7c2be5bce2
children a926f8995d0b
files NEWS OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp OrthancFramework/Sources/DicomNetworking/Internals/FindScp.h OrthancServer/Resources/Configuration.json OrthancServer/Sources/OrthancFindRequestHandler.cpp OrthancServer/Sources/OrthancGetRequestHandler.cpp OrthancServer/Sources/OrthancGetRequestHandler.h OrthancServer/Sources/main.cpp
diffstat 9 files changed, 108 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Thu Jan 28 14:07:49 2021 +0100
+++ b/NEWS	Thu Jan 28 15:54:30 2021 +0100
@@ -15,6 +15,8 @@
   - "LocalAet" in "DicomModalities" to overwrite global "DicomAet" for SCU on a per-modality basis
   - "AcceptedTransferSyntaxes" to set the transfer syntax UIDs accepted by Orthanc C-STORE SCP
   - "H265TransferSyntaxAccepted" to enable/disable all the transfer syntaxes related to H.265
+  - "DicomAlwaysAllowFind" to disable verification of the remote modality in C-FIND SCP
+  - "DicomAlwaysAllowGet" to disable verification of the remote modality in C-GET SCP
 * New configuration option: "DicomScuPreferredTransferSyntax" to control transcoding in C-STORE SCU
 * New command-line option: "--openapi" to write the OpenAPI documentation of the REST API to a file
 * New metadata automatically computed at the series level: "RemoteAET"
--- a/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Thu Jan 28 15:54:30 2021 +0100
@@ -883,8 +883,7 @@
                   worklistHandler.reset(server_.GetWorklistRequestHandlerFactory().ConstructWorklistRequestHandler());
                 }
 
-                cond = Internals::findScp(assoc_, &msg, presID, server_.GetRemoteModalities(),
-                                          findHandler.get(), worklistHandler.get(),
+                cond = Internals::findScp(assoc_, &msg, presID, findHandler.get(), worklistHandler.get(),
                                           remoteIp_, remoteAet_, calledAet_, associationTimeout_);
               }
               break;
--- a/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp	Thu Jan 28 15:54:30 2021 +0100
@@ -129,7 +129,6 @@
   {  
     struct FindScpData
     {
-      DicomServer::IRemoteModalities* modalities_;
       IFindRequestHandler* findHandler_;
       IWorklistRequestHandler* worklistHandler_;
       DicomFindAnswers answers_;
@@ -139,7 +138,6 @@
       const std::string* calledAet_;
 
       FindScpData() :
-        modalities_(NULL),
         findHandler_(NULL),
         worklistHandler_(NULL),
         answers_(false),
@@ -227,15 +225,6 @@
            * Ensure that the remote modality is known to Orthanc for C-FIND requests.
            **/
 
-          assert(data.modalities_ != NULL);
-          if (!data.modalities_->LookupAETitle(modality, *data.remoteAet_))
-          {
-            throw OrthancException(ErrorCode_UnknownModality,
-                                   "Modality with AET \"" + (*data.remoteAet_) +
-                                   "\" is not defined in the \"DicomModalities\" configuration option");
-          }
-
-          
           if (sopClassUid == UID_FINDModalityWorklistInformationModel)
           {
             data.answers_.SetWorklist(true);
@@ -359,7 +348,6 @@
   OFCondition Internals::findScp(T_ASC_Association * assoc, 
                                  T_DIMSE_Message * msg, 
                                  T_ASC_PresentationContextID presID,
-                                 DicomServer::IRemoteModalities& modalities,
                                  IFindRequestHandler* findHandler,
                                  IWorklistRequestHandler* worklistHandler,
                                  const std::string& remoteIp,
@@ -368,7 +356,6 @@
                                  int timeout)
   {
     FindScpData data;
-    data.modalities_ = &modalities;
     data.findHandler_ = findHandler;
     data.worklistHandler_ = worklistHandler;
     data.lastRequest_ = NULL;
--- a/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.h	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/FindScp.h	Thu Jan 28 15:54:30 2021 +0100
@@ -33,7 +33,6 @@
     OFCondition findScp(T_ASC_Association * assoc, 
                         T_DIMSE_Message * msg, 
                         T_ASC_PresentationContextID presID,
-                        DicomServer::IRemoteModalities& modalities,
                         IFindRequestHandler* findHandler,   // can be NULL
                         IWorklistRequestHandler* worklistHandler,   // can be NULL
                         const std::string& remoteIp,
--- a/OrthancServer/Resources/Configuration.json	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancServer/Resources/Configuration.json	Thu Jan 28 15:54:30 2021 +0100
@@ -264,7 +264,7 @@
   // 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
-  // is the only version to behave as if this argument was set to "false".
+  // is the only version to behave as if this argument were set to "false".
   "DicomAlwaysAllowEcho" : true,
 
   // Whether the Orthanc SCP allows incoming C-STORE requests, even
@@ -272,6 +272,18 @@
   // listed in the "DicomModalities" option above)
   "DicomAlwaysAllowStore" : true,
 
+  // Whether the Orthanc SCP allows incoming C-FIND requests, 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.9.0)
+  "DicomAlwaysAllowFind" : 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
+  // option to "true" implies security risks. (new in Orthanc 1.9.0)
+  "DicomAlwaysAllowGet" : false,
+
   // Whether Orthanc checks the IP/hostname address of the remote
   // modality initiating a DICOM connection (as listed in the
   // "DicomModalities" option above). If this option is set to
--- a/OrthancServer/Sources/OrthancFindRequestHandler.cpp	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancServer/Sources/OrthancFindRequestHandler.cpp	Thu Jan 28 15:54:30 2021 +0100
@@ -592,6 +592,36 @@
   {
     MetricsRegistry::Timer timer(context_.GetMetricsRegistry(), "orthanc_find_scp_duration_ms");
 
+
+    /**
+     * Deal with global configuration
+     **/
+
+    bool caseSensitivePN;
+
+    {
+      OrthancConfiguration::ReaderLock lock;
+      caseSensitivePN = lock.GetConfiguration().GetBooleanParameter("CaseSensitivePN", false);
+
+      RemoteModalityParameters remote;
+      if (!lock.GetConfiguration().LookupDicomModalityUsingAETitle(remote, remoteAet))
+      {
+        if (lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowFind", false))
+        {
+          CLOG(INFO, DICOM) << "C-FIND: Allowing SCU request from unknown modality with AET: " << remoteAet;
+        }
+        else
+        {
+          // This should never happen, given the test at bottom of
+          // "OrthancApplicationEntityFilter::IsAllowedRequest()"
+          throw OrthancException(ErrorCode_InexistentItem,
+                                 "C-FIND: Rejecting SCU request from unknown modality with AET: " + remoteAet);
+        }
+      }
+    }
+
+
+
     /**
      * Possibly apply the user-supplied Lua filter.
      **/
@@ -668,13 +698,6 @@
 
     DatabaseLookup lookup;
 
-    bool caseSensitivePN;
-
-    {
-      OrthancConfiguration::ReaderLock lock;
-      caseSensitivePN = lock.GetConfiguration().GetBooleanParameter("CaseSensitivePN", false);
-    }
-
     for (size_t i = 0; i < query.GetSize(); i++)
     {
       const DicomElement& element = query.GetElement(i);
--- a/OrthancServer/Sources/OrthancGetRequestHandler.cpp	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancServer/Sources/OrthancGetRequestHandler.cpp	Thu Jan 28 15:54:30 2021 +0100
@@ -240,13 +240,10 @@
                              ") " + sopClassUid);
     }
 
-    bool allowTranscoding = (context_.IsTranscodeDicomProtocol() &&
-                             remote_.IsTranscodingAllowed());
-    
     T_ASC_PresentationContextID presId = 0;  // Unnecessary initialization, makes code clearer
     DicomTransferSyntax selectedSyntax;
     if (!SelectPresentationContext(presId, selectedSyntax, assoc, sopClassUid,
-                                   sourceSyntax, allowTranscoding) ||
+                                   sourceSyntax, allowTranscoding_) ||
         presId == 0)
     {
       failedCount_++;
@@ -479,13 +476,14 @@
 
 
   OrthancGetRequestHandler::OrthancGetRequestHandler(ServerContext& context) :
-    context_(context)
+    context_(context),
+    position_(0),
+    completedCount_ (0),
+    warningCount_(0),
+    failedCount_(0),
+    timeout_(0),
+    allowTranscoding_(false)
   {
-    position_ = 0;
-    completedCount_  = 0;
-    warningCount_ = 0;
-    failedCount_ = 0;
-    timeout_ = 0;
   }
 
 
@@ -546,7 +544,26 @@
     
     {
       OrthancConfiguration::ReaderLock lock;
-      remote_ = lock.GetConfiguration().GetModalityUsingAet(originatorAet);
+
+      RemoteModalityParameters remote;
+
+      if (lock.GetConfiguration().LookupDicomModalityUsingAETitle(remote, originatorAet))
+      {
+        allowTranscoding_ = (context_.IsTranscodeDicomProtocol() &&
+                             remote.IsTranscodingAllowed());
+      }
+      else if (lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowGet", false))
+      {
+        CLOG(INFO, DICOM) << "C-GET: Allowing SCU request from unknown modality with AET: " << originatorAet;
+        allowTranscoding_ = context_.IsTranscodeDicomProtocol();
+      }
+      else
+      {
+        // This should never happen, given the test at bottom of
+        // "OrthancApplicationEntityFilter::IsAllowedRequest()"
+        throw OrthancException(ErrorCode_InexistentItem,
+                               "C-GET: Rejecting SCU request from unknown modality with AET: " + originatorAet);
+      }
     }
 
     for (std::list<std::string>::const_iterator
--- a/OrthancServer/Sources/OrthancGetRequestHandler.h	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancServer/Sources/OrthancGetRequestHandler.h	Thu Jan 28 15:54:30 2021 +0100
@@ -53,7 +53,6 @@
     std::string localAet_;
     std::vector<std::string> instances_;
     size_t position_;
-    RemoteModalityParameters remote_;
     std::string originatorAet_;
     
     unsigned int completedCount_;
@@ -62,6 +61,7 @@
     std::string failedUIDs_;
     
     uint32_t timeout_;
+    bool allowTranscoding_;
 
     bool LookupIdentifiers(std::list<std::string>& publicIds,
                            ResourceType level,
--- a/OrthancServer/Sources/main.cpp	Thu Jan 28 14:07:49 2021 +0100
+++ b/OrthancServer/Sources/main.cpp	Thu Jan 28 15:54:30 2021 +0100
@@ -280,15 +280,31 @@
 private:
   ServerContext&  context_;
   bool            alwaysAllowEcho_;
+  bool            alwaysAllowFind_;  // New in Orthanc 1.9.0
+  bool            alwaysAllowGet_;   // New in Orthanc 1.9.0
   bool            alwaysAllowStore_;
 
 public:
   explicit OrthancApplicationEntityFilter(ServerContext& context) :
     context_(context)
   {
-    OrthancConfiguration::ReaderLock lock;
-    alwaysAllowEcho_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowEcho", true);
-    alwaysAllowStore_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowStore", true);
+    {
+      OrthancConfiguration::ReaderLock lock;
+      alwaysAllowEcho_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowEcho", true);
+      alwaysAllowFind_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowFind", false);
+      alwaysAllowGet_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowGet", false);
+      alwaysAllowStore_ = lock.GetConfiguration().GetBooleanParameter("DicomAlwaysAllowStore", true);
+    }
+
+    if (alwaysAllowFind_)
+    {
+      LOG(WARNING) << "Security risk in DICOM SCP: C-FIND requests 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";
+    }
   }
 
   virtual bool IsAllowedConnection(const std::string& remoteIp,
@@ -299,6 +315,8 @@
               << " on IP " << remoteIp << ", calling AET " << calledAet;
 
     if (alwaysAllowEcho_ ||
+        alwaysAllowFind_ ||
+        alwaysAllowGet_ ||
         alwaysAllowStore_)
     {
       return true;
@@ -324,12 +342,24 @@
       // Incoming C-Echo requests are always accepted, even from unknown AET
       return true;
     }
+    else if (type == DicomRequestType_Find &&
+             alwaysAllowFind_)
+    {
+      // Incoming C-Find requests are always accepted, even from unknown AET
+      return true;
+    }
     else if (type == DicomRequestType_Store &&
              alwaysAllowStore_)
     {
       // Incoming C-Store requests are always accepted, even from unknown AET
       return true;
     }
+    else if (type == DicomRequestType_Get &&
+             alwaysAllowGet_)
+    {
+      // Incoming C-Get requests are always accepted, even from unknown AET
+      return true;
+    }
     else
     {
       OrthancConfiguration::ReaderLock lock;