# HG changeset patch # User Sebastien Jodogne # Date 1611845670 -3600 # Node ID 8efeaba1b7f995c72899e3e5a72779418c39f583 # Parent fe7c2be5bce27da4803f0a9706c3e2e8f5d7c373 new configuration options: "DicomAlwaysAllowFind" and "DicomAlwaysAllowGet" diff -r fe7c2be5bce2 -r 8efeaba1b7f9 NEWS --- 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" diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp --- 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; diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancFramework/Sources/DicomNetworking/Internals/FindScp.cpp --- 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; diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancFramework/Sources/DicomNetworking/Internals/FindScp.h --- 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, diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancServer/Resources/Configuration.json --- 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 diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancServer/Sources/OrthancFindRequestHandler.cpp --- 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); diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancServer/Sources/OrthancGetRequestHandler.cpp --- 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::const_iterator diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancServer/Sources/OrthancGetRequestHandler.h --- 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 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& publicIds, ResourceType level, diff -r fe7c2be5bce2 -r 8efeaba1b7f9 OrthancServer/Sources/main.cpp --- 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;