[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3' - test/integration-http-server.cpp wsd/LOOLWSD.cpp
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Thu Nov 8 21:20:31 UTC 2018
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 <tamas.zolnai at collabora.com>
AuthorDate: Thu Nov 8 21:16:57 2018 +0000
Commit: Michael Meeks <michael.meeks at collabora.com>
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 <michael.meeks at collabora.com>
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;
More information about the Libreoffice-commits
mailing list