# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1382697360 -7200
# Node ID 4aa6f0d799478bad58bc2265fd07e593eaaecc85
# Parent  70d0f27e5bd3297efe21261f8948b22b0fd14dd5
security filter for dicom requests

diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/DicomProtocol/IApplicationEntityFilter.h
--- a/OrthancServer/DicomProtocol/IApplicationEntityFilter.h	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/DicomProtocol/IApplicationEntityFilter.h	Fri Oct 25 12:36:00 2013 +0200
@@ -32,6 +32,8 @@
 
 #pragma once
 
+#include "../ServerEnumerations.h"
+
 #include <string>
 
 namespace Orthanc
@@ -43,7 +45,11 @@
     {
     }
 
-    virtual bool IsAllowed(const std::string& callingIp,
-                           const std::string& callingAet) = 0;
+    virtual bool IsAllowedConnection(const std::string& callingIp,
+                                     const std::string& callingAet) = 0;
+
+    virtual bool IsAllowedRequest(const std::string& callingIp,
+                                  const std::string& callingAet,
+                                  DicomRequestType type) = 0;
   };
 }
diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/Internals/CommandDispatcher.cpp
--- a/OrthancServer/Internals/CommandDispatcher.cpp	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/Internals/CommandDispatcher.cpp	Fri Oct 25 12:36:00 2013 +0200
@@ -326,6 +326,9 @@
         AssociationCleanup(assoc);
         return NULL;
       }
+
+      std::string callingIP;
+      std::string callingTitle;
   
       /* check the AETs */
       {
@@ -347,8 +350,8 @@
           return NULL;
         }
 
-        std::string callingIP(/*OFSTRING_GUARD*/(callingIP_C));
-        std::string callingTitle(/*OFSTRING_GUARD*/(callingTitle_C));
+        callingIP = std::string(/*OFSTRING_GUARD*/(callingIP_C));
+        callingTitle = std::string(/*OFSTRING_GUARD*/(callingTitle_C));
         std::string calledTitle(/*OFSTRING_GUARD*/(calledTitle_C));
         Toolbox::ToUpperCase(callingIP);
         Toolbox::ToUpperCase(callingTitle);
@@ -369,7 +372,7 @@
         }
 
         if (server.HasApplicationEntityFilter() &&
-            !server.GetApplicationEntityFilter().IsAllowed(callingIP, callingTitle))
+            !server.GetApplicationEntityFilter().IsAllowedConnection(callingIP, callingTitle))
         {
           T_ASC_RejectParameters rej =
             {
@@ -416,7 +419,8 @@
           LOG(INFO) << "    (but no valid presentation contexts)";
       }
 
-      return new CommandDispatcher(server, assoc);
+      IApplicationEntityFilter* filter = server.HasApplicationEntityFilter() ? &server.GetApplicationEntityFilter() : NULL;
+      return new CommandDispatcher(server, assoc, callingIP, callingTitle, filter);
     }
 
     bool CommandDispatcher::Step()
@@ -463,56 +467,94 @@
         // Reset the client timeout counter
         elapsedTimeSinceLastCommand_ = 0;
 
-        // in case we received a valid message, process this command
-        // note that storescp can only process a C-ECHO-RQ and a C-STORE-RQ
+        // Convert the type of request to Orthanc's internal type
+        bool supported = false;
+        DicomRequestType request;
         switch (msg.CommandField)
         {
-        case DIMSE_C_ECHO_RQ:
-          // process C-ECHO-Request
-          cond = EchoScp(assoc_, &msg, presID);
-          break;
+          case DIMSE_C_ECHO_RQ:
+            request = DicomRequestType_Echo;
+            supported = true;
+            break;
+
+          case DIMSE_C_STORE_RQ:
+            request = DicomRequestType_Store;
+            supported = true;
+            break;
+
+          case DIMSE_C_MOVE_RQ:
+            request = DicomRequestType_Move;
+            supported = true;
+            break;
+
+          case DIMSE_C_FIND_RQ:
+            request = DicomRequestType_Find;
+            supported = true;
+            break;
 
-        case DIMSE_C_STORE_RQ:
-          // process C-STORE-Request
-          if (server_.HasStoreRequestHandlerFactory())
-          {
-            std::auto_ptr<IStoreRequestHandler> handler
-              (server_.GetStoreRequestHandlerFactory().ConstructStoreRequestHandler());
-            cond = Internals::storeScp(assoc_, &msg, presID, *handler);
-          }
-          else
-            cond = DIMSE_BADCOMMANDTYPE;  // Should never happen
-          break;
+          default:
+            // we cannot handle this kind of message
+            cond = DIMSE_BADCOMMANDTYPE;
+            LOG(ERROR) << "cannot handle command: 0x" << std::hex << msg.CommandField;
+            break;
+        }
+
 
-        case DIMSE_C_MOVE_RQ:
-          // process C-MOVE-Request
-          if (server_.HasMoveRequestHandlerFactory())
+        // Check whether this request is allowed by the security filter
+        if (supported && 
+            filter_ != NULL &&
+            !filter_->IsAllowedRequest(callingIP_, callingAETitle_, request))
+        {
+          LOG(ERROR) << EnumerationToString(request) 
+                     << " requests are disallowed for the AET \"" 
+                     << callingAETitle_ << "\"";
+          cond = DIMSE_BADCOMMANDTYPE;
+          supported = false;
+        }
+
+        // in case we received a supported message, process this command
+        if (supported)
+        {
+          // If anything goes wrong, there will be a "BADCOMMANDTYPE" answer
+          cond = DIMSE_BADCOMMANDTYPE;
+
+          switch (request)
           {
-            std::auto_ptr<IMoveRequestHandler> handler
-              (server_.GetMoveRequestHandlerFactory().ConstructMoveRequestHandler());
-            cond = Internals::moveScp(assoc_, &msg, presID, *handler);
-          }
-          else
-            cond = DIMSE_BADCOMMANDTYPE;  // Should never happen
-          break;
+            case DicomRequestType_Echo:
+              cond = EchoScp(assoc_, &msg, presID);
+              break;
+
+            case DicomRequestType_Store:
+              if (server_.HasStoreRequestHandlerFactory()) // Should always be true
+              {
+                std::auto_ptr<IStoreRequestHandler> handler
+                  (server_.GetStoreRequestHandlerFactory().ConstructStoreRequestHandler());
+                cond = Internals::storeScp(assoc_, &msg, presID, *handler);
+              }
+              break;
 
-        case DIMSE_C_FIND_RQ:
-          // process C-FIND-Request
-          if (server_.HasFindRequestHandlerFactory())
-          {
-            std::auto_ptr<IFindRequestHandler> handler
-              (server_.GetFindRequestHandlerFactory().ConstructFindRequestHandler());
-            cond = Internals::findScp(assoc_, &msg, presID, *handler);
+            case DicomRequestType_Move:
+              if (server_.HasMoveRequestHandlerFactory()) // Should always be true
+              {
+                std::auto_ptr<IMoveRequestHandler> handler
+                  (server_.GetMoveRequestHandlerFactory().ConstructMoveRequestHandler());
+                cond = Internals::moveScp(assoc_, &msg, presID, *handler);
+              }
+              break;
+
+            case DicomRequestType_Find:
+              if (server_.HasFindRequestHandlerFactory()) // Should always be true
+              {
+                std::auto_ptr<IFindRequestHandler> handler
+                  (server_.GetFindRequestHandlerFactory().ConstructFindRequestHandler());
+                cond = Internals::findScp(assoc_, &msg, presID, *handler);
+              }
+              break;
+
+            default:
+              // Should never happen
+              break;
           }
-          else
-            cond = DIMSE_BADCOMMANDTYPE;  // Should never happen
-          break;
-
-        default:
-          // we cannot handle this kind of message
-          cond = DIMSE_BADCOMMANDTYPE;
-          LOG(ERROR) << "cannot handle command: 0x" << std::hex << msg.CommandField;
-          break;
         }
       }
       else
diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/Internals/CommandDispatcher.h
--- a/OrthancServer/Internals/CommandDispatcher.h	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/Internals/CommandDispatcher.h	Fri Oct 25 12:36:00 2013 +0200
@@ -50,12 +50,21 @@
       uint32_t elapsedTimeSinceLastCommand_;
       const DicomServer& server_;
       T_ASC_Association* assoc_;
+      std::string callingIP_;
+      std::string callingAETitle_;
+      IApplicationEntityFilter* filter_;
 
     public:
       CommandDispatcher(const DicomServer& server,
-                        T_ASC_Association* assoc) : 
+                        T_ASC_Association* assoc,
+                        const std::string& callingIP,
+                        const std::string& callingAETitle,
+                        IApplicationEntityFilter* filter) :
         server_(server),
-        assoc_(assoc)
+        assoc_(assoc),
+        callingIP_(callingIP),
+        callingAETitle_(callingAETitle),
+        filter_(filter)
       {
         clientTimeout_ = server.GetClientTimeout();
         elapsedTimeSinceLastCommand_ = 0;
diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/ServerEnumerations.cpp
--- a/OrthancServer/ServerEnumerations.cpp	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/ServerEnumerations.cpp	Fri Oct 25 12:36:00 2013 +0200
@@ -254,6 +254,38 @@
   }
 
 
+  const char* EnumerationToString(DicomRequestType type)
+  {
+    switch (type)
+    {
+      case DicomRequestType_Echo:
+        return "Echo";
+        break;
+
+      case DicomRequestType_Find:
+        return "Find";
+        break;
+
+      case DicomRequestType_Get:
+        return "Get";
+        break;
+
+      case DicomRequestType_Move:
+        return "Move";
+        break;
+
+      case DicomRequestType_Store:
+        return "Store";
+        break;
+
+
+      default: 
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+  }
+
+
+
   ModalityManufacturer StringToModalityManufacturer(const std::string& manufacturer)
   {
     if (manufacturer == "Generic")
diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/ServerEnumerations.h
--- a/OrthancServer/ServerEnumerations.h	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/ServerEnumerations.h	Fri Oct 25 12:36:00 2013 +0200
@@ -59,6 +59,15 @@
     ModalityManufacturer_ClearCanvas
   };
 
+  enum DicomRequestType
+  {
+    DicomRequestType_Echo,
+    DicomRequestType_Find,
+    DicomRequestType_Get,
+    DicomRequestType_Move,
+    DicomRequestType_Store
+  };
+
 
   /**
    * WARNING: Do not change the explicit values in the enumerations
@@ -126,6 +135,8 @@
 
   const char* EnumerationToString(ModalityManufacturer manufacturer);
 
+  const char* EnumerationToString(DicomRequestType type);
+
   ModalityManufacturer StringToModalityManufacturer(const std::string& manufacturer);
 
   ResourceType GetParentResourceType(ResourceType type);
diff -r 70d0f27e5bd3 -r 4aa6f0d79947 OrthancServer/main.cpp
--- a/OrthancServer/main.cpp	Fri Oct 25 11:57:30 2013 +0200
+++ b/OrthancServer/main.cpp	Fri Oct 25 12:36:00 2013 +0200
@@ -113,9 +113,22 @@
 class OrthancApplicationEntityFilter : public IApplicationEntityFilter
 {
 public:
-  virtual bool IsAllowed(const std::string& /*callingIp*/,
-                         const std::string& callingAet)
+  virtual bool IsAllowedConnection(const std::string& /*callingIp*/,
+                                   const std::string& /*callingAet*/)
   {
+    return true;
+  }
+
+  virtual bool IsAllowedRequest(const std::string& /*callingIp*/,
+                                const std::string& callingAet,
+                                DicomRequestType type)
+  {
+    if (type == DicomRequestType_Store)
+    {
+      // Incoming store requests are always accepted, even from unknown AET
+      return true;
+    }
+
     if (!IsKnownAETitle(callingAet))
     {
       LOG(ERROR) << "Unkwnown remote DICOM modality AET: \"" << callingAet << "\"";