test/integration-http-server.cpp | 117 +++++++++++++++++++++++++++++++++++++++ wsd/LOOLWSD.cpp | 53 ++++++++++++++++- 2 files changed, 165 insertions(+), 5 deletions(-)
New commits: commit 503fb39e50a83ecd10e7b742bd32632bb56fb777 Author: Tamás Zolnai <[email protected]> AuthorDate: Thu Nov 8 21:16:57 2018 +0000 Commit: Michael Meeks <[email protected]> CommitDate: Thu Nov 8 21:20:12 2018 +0000 Handle X-Forwarded-For header for convert-to feature Extract the client IP from the X-Forwarded-For value and use that one to allow / deny the usage of convert-to feature. (cherry picked from commit 318f0629bbf94c38bce42589937f46a1f8fd79ee) Change-Id: I363c0931df5a0538236cae12f943fffd65086ee6 Need to use clientHost here Change-Id: I170e1d24e1a71749c3262c01a83251c6c157f6eb (cherry picked from commit 18eb13438487290a7d4928381d6ed073134d0ea8) Handle X-Forwarded-For with more secure Check all pariticipating IPs to be allowed to use convert-to functionality. In a simple use case it means the reverse-proxy's and the actual client's IP. Change-Id: I4ef9cb14a1c3003cba6c66f6e99d5b54b2c3b2b8 (cherry picked from commit b4b0e9c6d4d649e010e7742efae670ed541f78a2) Have a better log for convert-to denial (cherry picked from commit bf2dcdc01f04a440c8a12d5f5e212d9bdd39c1f6) Change-Id: I5c8d367b3f82d47a45df8c298e39515bc89f7b0d Signed-off-by: Michael Meeks <[email protected]> diff --git a/test/integration-http-server.cpp b/test/integration-http-server.cpp index 4a1c425ad..1d6ca742a 100644 --- a/test/integration-http-server.cpp +++ b/test/integration-http-server.cpp @@ -51,6 +51,7 @@ class HTTPServerTest : public CPPUNIT_NS::TestFixture CPPUNIT_TEST(testScriptsAndLinksGet); CPPUNIT_TEST(testScriptsAndLinksPost); // FIXME CPPUNIT_TEST(testConvertTo); + CPPUNIT_TEST(testConvertToWithForwardedClientIP); CPPUNIT_TEST_SUITE_END(); @@ -61,6 +62,7 @@ class HTTPServerTest : public CPPUNIT_NS::TestFixture void testScriptsAndLinksGet(); void testScriptsAndLinksPost(); void testConvertTo(); + void testConvertToWithForwardedClientIP(); public: HTTPServerTest() @@ -92,8 +94,25 @@ public: { testNoExtraLoolKitsLeft(); } + + // A server URI which was not added to loolwsd.xml as post_allow IP or a wopi storage host + Poco::URI getNotAllowedTestServerURI() + { + static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT"); + + static std::string serverURI( +#if ENABLE_SSL + "https://165.227.162.232:"; +#else + "http://165.227.162.232:"; +#endif + + (clientPort? std::string(clientPort) : std::to_string(DEFAULT_CLIENT_PORT_NUMBER))); + + return Poco::URI(serverURI); + } }; + void HTTPServerTest::testDiscovery() { std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri)); @@ -341,6 +360,104 @@ void HTTPServerTest::testConvertTo() CPPUNIT_ASSERT_EQUAL(expectedStream.str(), actualString); } + +void HTTPServerTest::testConvertToWithForwardedClientIP() +{ + // Test a forwarded IP which is not allowed to use convert-to feature + { + const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_"); + std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri)); + session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds. + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to"); + CPPUNIT_ASSERT(!request.has("X-Forwarded-For")); + request.add("X-Forwarded-For", getNotAllowedTestServerURI().getHost() + ", " + _uri.getHost()); + Poco::Net::HTMLForm form; + form.setEncoding(Poco::Net::HTMLForm::ENCODING_MULTIPART); + form.set("format", "txt"); + form.addPart("data", new Poco::Net::FilePartSource(srcPath)); + form.prepareSubmit(request); + form.write(session->sendRequest(request)); + + Poco::Net::HTTPResponse response; + std::stringstream actualStream; + std::istream& responseStream = session->receiveResponse(response); + Poco::StreamCopier::copyStream(responseStream, actualStream); + + // Remove the temp files. + FileUtil::removeFile(srcPath); + + std::string actualString = actualStream.str(); + CPPUNIT_ASSERT(actualString.empty()); // <- we did not get the converted file + } + + // Test a forwarded IP which is allowed to use convert-to feature + { + const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_"); + std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri)); + session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds. + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to"); + CPPUNIT_ASSERT(!request.has("X-Forwarded-For")); + request.add("X-Forwarded-For", _uri.getHost() + ", " + _uri.getHost()); + Poco::Net::HTMLForm form; + form.setEncoding(Poco::Net::HTMLForm::ENCODING_MULTIPART); + form.set("format", "txt"); + form.addPart("data", new Poco::Net::FilePartSource(srcPath)); + form.prepareSubmit(request); + form.write(session->sendRequest(request)); + + Poco::Net::HTTPResponse response; + std::stringstream actualStream; + std::istream& responseStream = session->receiveResponse(response); + Poco::StreamCopier::copyStream(responseStream, actualStream); + + std::ifstream fileStream(TDOC "/hello.txt"); + std::stringstream expectedStream; + expectedStream << fileStream.rdbuf(); + + // Remove the temp files. + FileUtil::removeFile(srcPath); + + // In some cases the result is prefixed with (the UTF-8 encoding of) the Unicode BOM + // (U+FEFF). Skip that. + std::string actualString = actualStream.str(); + if (actualString.size() > 3 && actualString[0] == '\xEF' && actualString[1] == '\xBB' && actualString[2] == '\xBF') + actualString = actualString.substr(3); + CPPUNIT_ASSERT_EQUAL(expectedStream.str(), actualString); // <- we got the converted file + } + + // Test a forwarded header with three IPs, one is not allowed -> request is denied. + { + const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "testConvertToWithForwardedClientIP_"); + std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri)); + session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds. + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to"); + CPPUNIT_ASSERT(!request.has("X-Forwarded-For")); + request.add("X-Forwarded-For", _uri.getHost() + ", " + + getNotAllowedTestServerURI().getHost() + ", " + + _uri.getHost()); + Poco::Net::HTMLForm form; + form.setEncoding(Poco::Net::HTMLForm::ENCODING_MULTIPART); + form.set("format", "txt"); + form.addPart("data", new Poco::Net::FilePartSource(srcPath)); + form.prepareSubmit(request); + form.write(session->sendRequest(request)); + + Poco::Net::HTTPResponse response; + std::stringstream actualStream; + std::istream& responseStream = session->receiveResponse(response); + Poco::StreamCopier::copyStream(responseStream, actualStream); + + // Remove the temp files. + FileUtil::removeFile(srcPath); + + std::string actualString = actualStream.str(); + CPPUNIT_ASSERT(actualString.empty()); // <- we did not get the converted file + } +} + CPPUNIT_TEST_SUITE_REGISTRATION(HTTPServerTest); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 7d0e0eb33..b46ac2232 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -70,6 +70,8 @@ #include <Poco/Net/NetException.h> #include <Poco/Net/PartHandler.h> #include <Poco/Net/SocketAddress.h> +#include <Poco/Net/DNS.h> +#include <Poco/Net/HostEntry.h> #include <Poco/Path.h> #include <Poco/Pipe.h> #include <Poco/Process.h> @@ -1829,6 +1831,48 @@ public: } return hosts.match(address); } + bool allowConvertTo(const std::string &address, const Poco::Net::HTTPRequest& request, bool report = false) + { + std::string addressToCheck = address; + std::string hostToCheck = request.getHost(); + bool allow = allowPostFrom(addressToCheck) || StorageBase::allowedWopiHost(hostToCheck); + + if(!allow) + { + if(report) + LOG_ERR("Requesting address is denied: " << addressToCheck); + return false; + } + + // Handle forwarded header and make sure all participating IPs are allowed + if(request.has("X-Forwarded-For")) + { + std::string fowardedData = request.get("X-Forwarded-For"); + std::vector<std::string> tokens = LOOLProtocol::tokenize(fowardedData, ','); + for(std::string& token : tokens) + { + addressToCheck = Util::trim(token); + try + { + hostToCheck = Poco::Net::DNS::resolve(addressToCheck).name(); + allow &= allowPostFrom(addressToCheck) || StorageBase::allowedWopiHost(hostToCheck); + } + catch (const Poco::Exception& exc) + { + LOG_WRN("Poco::Net::DNS::resolve(\"" << addressToCheck << "\") failed: " << exc.displayText()); + // We can't find out the hostname, check the IP only + allow &= allowPostFrom(addressToCheck); + } + if(!allow) + { + if(report) + LOG_ERR("Requesting address is denied: " << addressToCheck); + return false; + } + } + } + return allow; + } private: @@ -2053,7 +2097,7 @@ private: { LOG_DBG("Wopi capabilities request: " << request.getURI()); - std::string capabilities = getCapabilitiesJson(request.getHost()); + std::string capabilities = getCapabilitiesJson(request); std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" @@ -2139,9 +2183,8 @@ private: std::string format = (form.has("format") ? form.get("format") : ""); - if (!allowPostFrom(socket->clientAddress()) && !StorageBase::allowedWopiHost(request.getHost()) ) + if (!allowConvertTo(socket->clientAddress(), request, true)) { - LOG_ERR("client address DENY: " << socket->clientAddress().c_str()); std::ostringstream oss; oss << "HTTP/1.1 403\r\n" << "Date: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" @@ -2567,7 +2610,7 @@ private: } /// Process the capabilities.json file and return as string. - std::string getCapabilitiesJson(const std::string& host) + std::string getCapabilitiesJson(const Poco::Net::HTTPRequest& request) { std::shared_ptr<StreamSocket> socket = _socket.lock(); @@ -2591,7 +2634,7 @@ private: Poco::JSON::Object::Ptr features = jsonFile.extract<Poco::JSON::Object::Ptr>(); Poco::JSON::Object::Ptr convert_to = features->get("convert-to").extract<Poco::JSON::Object::Ptr>(); - Poco::Dynamic::Var available = allowPostFrom(socket->clientAddress()) || StorageBase::allowedWopiHost(host); + Poco::Dynamic::Var available = allowConvertTo(socket->clientAddress(), request); convert_to->set("available", available); std::ostringstream ostrJSON; _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
