# HG changeset patch # User Alain Mazy # Date 1645195528 -3600 # Node ID 8523078f3f4b7cba603949aabf30dd838371513d # Parent a29a6bdba9dd34d729a3a2eef1831a6e2694ce9a added new configuration to authorize C-Find for worklist independently from other C-Find diff -r a29a6bdba9dd -r 8523078f3f4b NEWS --- 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. diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp --- 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()) { diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp --- 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_; diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.h --- 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_; diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/Sources/Enumerations.cpp --- 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; diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/Sources/Enumerations.h --- 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, diff -r a29a6bdba9dd -r 8523078f3f4b OrthancFramework/UnitTestsSources/JobsTests.cpp --- 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 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); diff -r a29a6bdba9dd -r 8523078f3f4b OrthancServer/Resources/Configuration.json --- 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, diff -r a29a6bdba9dd -r 8523078f3f4b OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp --- 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; diff -r a29a6bdba9dd -r 8523078f3f4b OrthancServer/Sources/main.cpp --- 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";