Fixed worker's REST service to prevent unauthorized access to static …

…content

The previous version of the service was wide-open allowing clients to read
all files from the container's inner filesystem.

src/wbase/FileChannelShared.cc

@@ -456,7 +456,7 @@ bool FileChannelShared::_writeToFile(lock_guard<mutex> const& tMtxLock, shared_p
     LOGS(_log, LOG_LVL_TRACE, __func__ << " file write " << task->getIdStr() << " start");
     // Create the file if not open.
     if (!_file.is_open()) {
-        _fileName = task->resultFilePath();
+        _fileName = task->resultFileAbsPath();
         _file.open(_fileName, ios::out | ios::trunc | ios::binary);
         if (!(_file.is_open() && _file.good())) {
             throw runtime_error("FileChannelShared::" + string(__func__) +

src/wbase/Task.cc

@@ -68,14 +68,15 @@ namespace {
 
 LOG_LOGGER _log = LOG_GET("lsst.qserv.wbase.Task");
 
-string buildResultFilePath(shared_ptr<lsst::qserv::proto::TaskMsg> const& taskMsg,
-                           string const& resultsDirname) {
+string buildResultFileName(shared_ptr<lsst::qserv::proto::TaskMsg> const& taskMsg) {
+    return to_string(taskMsg->czarid()) + "-" + to_string(taskMsg->queryid()) + "-" +
+           to_string(taskMsg->jobid()) + "-" + to_string(taskMsg->chunkid()) + "-" +
+           to_string(taskMsg->attemptcount()) + ".proto";
+}
+
+string buildResultFilePath(string const& resultFileName, string const& resultsDirname) {
     if (resultsDirname.empty()) return resultsDirname;
-    fs::path path(resultsDirname);
-    path /= to_string(taskMsg->czarid()) + "-" + to_string(taskMsg->queryid()) + "-" +
-            to_string(taskMsg->jobid()) + "-" + to_string(taskMsg->chunkid()) + "-" +
-            to_string(taskMsg->attemptcount()) + ".proto";
-    return path.string();
+    return fs::weakly_canonical(fs::path(resultsDirname) / resultFileName).string();
 }
 
 size_t const MB_SIZE_BYTES = 1024 * 1024;
@@ -139,15 +140,16 @@ Task::Task(TaskMsgPtr const& t, int fragmentNumber, shared_ptr<UserQueryInfo> co
     // to advice which result delivery channel to use.
     auto const workerConfig = wconfig::WorkerConfig::instance();
     auto const resultDeliveryProtocol = workerConfig->resultDeliveryProtocol();
-    _resultFilePath = ::buildResultFilePath(t, workerConfig->resultsDirname());
+    _resultFileName = ::buildResultFileName(t);
+    _resultFileAbsPath = ::buildResultFilePath(_resultFileName, workerConfig->resultsDirname());
     auto const fqdn = util::get_current_host_fqdn();
     if (resultDeliveryProtocol == wconfig::ConfigValResultDeliveryProtocol::XROOT) {
         // NOTE: one extra '/' after the <host>[:<port>] spec is required to make
         // a "valid" XROOTD url.
         _resultFileXrootUrl = "xroot://" + fqdn + ":" + to_string(workerConfig->resultsXrootdPort()) + "/" +
-                              _resultFilePath;
+                              _resultFileAbsPath;
     } else if (resultDeliveryProtocol == wconfig::ConfigValResultDeliveryProtocol::HTTP) {
-        _resultFileHttpUrl = "http://" + fqdn + ":" + to_string(resultsHttpPort) + _resultFilePath;
+        _resultFileHttpUrl = "http://" + fqdn + ":" + to_string(resultsHttpPort) + "/" + _resultFileName;
     } else {
         throw runtime_error("wbase::Task::Task: unsupported results delivery protocol: " +
                             wconfig::ConfigValResultDeliveryProtocol::toString(resultDeliveryProtocol));

src/wbase/Task.h

@@ -207,7 +207,7 @@ class Task : public util::CommandForThreadPool {
     TaskState state() const { return _state; }
     std::string getQueryString() const;
     int getQueryFragmentNum() { return _queryFragmentNum; }
-    std::string const& resultFilePath() const { return _resultFilePath; }
+    std::string const& resultFileAbsPath() const { return _resultFileAbsPath; }
     std::string const& resultFileXrootUrl() const { return _resultFileXrootUrl; }
     std::string const& resultFileHttpUrl() const { return _resultFileHttpUrl; }
     bool setTaskQueryRunner(
@@ -334,13 +334,16 @@ class Task : public util::CommandForThreadPool {
     std::unique_ptr<DbTblsAndSubchunks> _dbTblsAndSubchunks;
 
     /// The path to the result file.
-    std::string _resultFilePath;
+    std::string _resultFileAbsPath;
 
-    /// The XROOTD URL for the result file: "xroot://<host>:<xrootd-port>" + "/" + _resultFilePath
+    /// The name of the result file.
+    std::string _resultFileName;
+
+    /// The XROOTD URL for the result file: "xroot://<host>:<xrootd-port>" + "/" + _resultFileAbsPath
     /// @note an extra '/' after server:port spec is required to make a "valid" XROOTD url
     std::string _resultFileXrootUrl;
 
-    /// The HTTP URL for the result file: "http://<host>:<http-port>" + _resultFilePath
+    /// The HTTP URL for the result file: "http://<host>:<http-port>/" + _resultFileName
     std::string _resultFileHttpUrl;
 
     std::atomic<bool> _queryStarted{false};  ///< Set to true when the query is about to be run.

src/wcontrol/Foreman.cc

@@ -110,17 +110,9 @@ Foreman::Foreman(Scheduler::Ptr const& scheduler, unsigned int poolSize, unsigne
     _mark = make_shared<util::HoldTrack::Mark>(ERR_LOC, "Forman Test Msg");
 
     // Read-only access to the result files via the HTTP protocol's method "GET"
-    //
-    // NOTE: The following config doesn't seem to work due to multiple instances
-    //       of '/' that's present in a value passed for the pattern parameter
-    //      (the first parameter) of the called method.
-    //
-    // _httpServer->addStaticContent(workerConfig->resultsDirname() + "/*", "/");
-    //
-    // Using this insecure config instead. The problem will get fixed later.
     auto const workerConfig = wconfig::WorkerConfig::instance();
-    _httpServer->addStaticContent("/*", "/");
-    _httpServer->addHandler("DELETE", workerConfig->resultsDirname() + "/:file",
+    _httpServer->addStaticContent("/*", workerConfig->resultsDirname());
+    _httpServer->addHandler("DELETE", "/:file",
                             [](qhttp::Request::Ptr const req, qhttp::Response::Ptr const resp) {
                                 resp->sendStatus(::removeResultFile(req->path));
                             });