# HG changeset patch # User Sebastien Jodogne # Date 1539087588 -7200 # Node ID 9d08edde614b31b74fbcab7d732e09a660a73916 # Parent 6eebc2eb3168c59681d608640b925af52d999635 Possibility to restrict the allowed DICOM commands for each modality diff -r 6eebc2eb3168 -r 9d08edde614b Core/DicomNetworking/RemoteModalityParameters.cpp --- 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(tmp); - } + CheckPortNumber(tmp); + return static_cast(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: diff -r 6eebc2eb3168 -r 9d08edde614b Core/DicomNetworking/RemoteModalityParameters.h --- 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; diff -r 6eebc2eb3168 -r 9d08edde614b NEWS --- 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 -------- diff -r 6eebc2eb3168 -r 9d08edde614b OrthancServer/main.cpp --- 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; + } } } diff -r 6eebc2eb3168 -r 9d08edde614b Resources/Configuration.json --- 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 diff -r 6eebc2eb3168 -r 9d08edde614b UnitTestsSources/MultiThreadingTests.cpp --- 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 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::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::const_iterator + it2 = operations.begin(); it2 != operations.end(); ++it2) + { + if (*it2 != *it) + { + ASSERT_TRUE(modality.IsRequestAllowed(*it2)); + } + } + } + } }