loleaflet/src/admin/AdminSocketAnalytics.js | 3 loleaflet/src/admin/AdminSocketBase.js | 5 - loleaflet/src/admin/AdminSocketOverview.js | 3 loleaflet/src/admin/AdminSocketSettings.js | 3 loleaflet/src/admin/Util.js | 12 +++ loolwsd/Admin.cpp | 34 ++++++++- loolwsd/Admin.hpp | 1 loolwsd/FileServer.cpp | 58 ++++++++-------- loolwsd/FileServer.hpp | 19 +++-- loolwsd/LOOLWSD.cpp | 13 --- loolwsd/protocol.txt | 16 ++++ loolwsd/test/UnitAdmin.cpp | 101 ++++++++++++++++------------ 12 files changed, 173 insertions(+), 95 deletions(-)
New commits: commit b8ef0176103f61a3d548ff9ad7ee001537aaff3a Author: Pranav Kant <pran...@collabora.co.uk> Date: Thu Jul 28 17:58:40 2016 +0530 Move JWT auth to inside websocket to prevent CSRF ... instead of setting a httpOnly cookie for admin websocket endpoint which can be CSRFed. With this, we remove the httpOnly tag on jwt cookie so that scripts in admin html pages can access the jwt cookie and authenticates after opening websocket endpoint. Until authenticated using 'auth authToken' command, admin will not respond to any admin command. Also, adapt admin test these changes diff --git a/loleaflet/src/admin/AdminSocketAnalytics.js b/loleaflet/src/admin/AdminSocketAnalytics.js index c8815b7..a5cbcde 100644 --- a/loleaflet/src/admin/AdminSocketAnalytics.js +++ b/loleaflet/src/admin/AdminSocketAnalytics.js @@ -38,6 +38,9 @@ var AdminSocketAnalytics = AdminSocketBase.extend({ }, onSocketOpen: function() { + // Base class' onSocketOpen handles authentication + this.base.call(this); + this.socket.send('subscribe mem_stats cpu_stats settings'); this.socket.send('settings'); this.socket.send('mem_stats'); diff --git a/loleaflet/src/admin/AdminSocketBase.js b/loleaflet/src/admin/AdminSocketBase.js index 48720df..cb8a5a8 100644 --- a/loleaflet/src/admin/AdminSocketBase.js +++ b/loleaflet/src/admin/AdminSocketBase.js @@ -2,7 +2,7 @@ Abstract class */ -/* global vex Base */ +/* global Util vex Base */ /* eslint no-unused-vars:0 */ var AdminSocketBase = Base.extend({ socket: null, @@ -26,7 +26,8 @@ var AdminSocketBase = Base.extend({ }, onSocketOpen: function() { - /* Implemented by child */ + // Authenticate + this.socket.send('auth ' + Util.getCookie('jwt')); }, onSocketMessage: function() { diff --git a/loleaflet/src/admin/AdminSocketOverview.js b/loleaflet/src/admin/AdminSocketOverview.js index e58ec9e..89a17a2 100644 --- a/loleaflet/src/admin/AdminSocketOverview.js +++ b/loleaflet/src/admin/AdminSocketOverview.js @@ -19,6 +19,9 @@ var AdminSocketOverview = AdminSocketBase.extend({ }, onSocketOpen: function() { + // Base class' onSocketOpen handles authentication + this.base.call(this); + this.socket.send('documents'); this.socket.send('subscribe adddoc rmdoc'); diff --git a/loleaflet/src/admin/AdminSocketSettings.js b/loleaflet/src/admin/AdminSocketSettings.js index 094cfeb..5b71b63 100644 --- a/loleaflet/src/admin/AdminSocketSettings.js +++ b/loleaflet/src/admin/AdminSocketSettings.js @@ -29,6 +29,9 @@ var AdminSocketSettings = AdminSocketBase.extend({ }, onSocketOpen: function() { + // Base class' onSocketOpen handles authentication + this.base.call(this); + this.socket.send('subscribe settings'); this.socket.send('settings'); }, diff --git a/loleaflet/src/admin/Util.js b/loleaflet/src/admin/Util.js index c208763..acc5db5 100644 --- a/loleaflet/src/admin/Util.js +++ b/loleaflet/src/admin/Util.js @@ -48,5 +48,17 @@ var Util = Base.extend({ } return res; + }, + + getCookie: function(name) { + var cookies = document.cookie.split(';'); + for (var i = 0; i < cookies.length; i++) { + var cookie = cookies[i].trim(); + if (cookie.indexOf(name) === 0) { + return cookie; + } + } + + return ''; } }); diff --git a/loolwsd/Admin.cpp b/loolwsd/Admin.cpp index c8405b2..d03eca5 100644 --- a/loolwsd/Admin.cpp +++ b/loolwsd/Admin.cpp @@ -48,6 +48,7 @@ using Poco::Net::HTTPResponse; using Poco::Net::HTTPServerRequest; using Poco::Net::HTTPServerResponse; using Poco::Net::WebSocket; +using Poco::Util::Application; bool AdminRequestHandler::adminCommandHandler(const std::vector<char>& payload) { @@ -61,7 +62,32 @@ bool AdminRequestHandler::adminCommandHandler(const std::vector<char>& payload) std::unique_lock<std::mutex> modelLock(_admin->getLock()); AdminModel& model = _admin->getModel(); - if (tokens[0] == "documents" || + if (tokens.count() > 1 && tokens[0] == "auth") { + std::string jwtToken; + LOOLProtocol::getTokenString(tokens[1], "jwt", jwtToken); + const auto& config = Application::instance().config(); + const auto sslKeyPath = config.getString("ssl.key_file_path", ""); + + Log::info("Verifying JWT token: " + jwtToken); + JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); + if (authAgent.verify(jwtToken)) + { + Log::trace("JWT token is valid"); + _isAuthenticated = true; + } + else + { + sendTextFrame("InvalidAuthToken"); + return false; + } + } + + if (!_isAuthenticated) + { + sendTextFrame("NotAuthenticated"); + return false; + } + else if (tokens[0] == "documents" || tokens[0] == "active_users_count" || tokens[0] == "active_docs_count" || tokens[0] == "mem_stats" || @@ -203,7 +229,8 @@ void AdminRequestHandler::handleWSRequests(HTTPServerRequest& request, HTTPServe } AdminRequestHandler::AdminRequestHandler(Admin* adminManager) - : _admin(adminManager) + : _admin(adminManager), + _isAuthenticated(false) { } void AdminRequestHandler::sendTextFrame(const std::string& message) @@ -228,9 +255,6 @@ void AdminRequestHandler::handleRequest(HTTPServerRequest& request, HTTPServerRe if (request.find("Upgrade") != request.end() && Poco::icompare(request["Upgrade"], "websocket") == 0) { - if (!FileServerRequestHandler::isAdminLoggedIn(request, response)) - throw Poco::Net::NotAuthenticatedException("Invalid admin login"); - handleWSRequests(request, response, sessionId); } } diff --git a/loolwsd/Admin.hpp b/loolwsd/Admin.hpp index 4fb7d95..118d562 100644 --- a/loolwsd/Admin.hpp +++ b/loolwsd/Admin.hpp @@ -44,6 +44,7 @@ private: Admin* _admin; std::shared_ptr<Poco::Net::WebSocket> _adminWs; int _sessionId; + bool _isAuthenticated; }; /// An admin command processor. diff --git a/loolwsd/FileServer.cpp b/loolwsd/FileServer.cpp index 54c7133..eef1b48 100644 --- a/loolwsd/FileServer.cpp +++ b/loolwsd/FileServer.cpp @@ -93,9 +93,8 @@ bool FileServerRequestHandler::isAdminLoggedIn(HTTPServerRequest& request, HTTPS JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); const std::string jwtToken = authAgent.getAccessToken(); Poco::Net::HTTPCookie cookie("jwt", jwtToken); - cookie.setPath("/lool/adminws/"); + cookie.setPath("/loleaflet/dist/admin/"); cookie.setSecure(true); - cookie.setHttpOnly(true); response.addCookie(cookie); return true; diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt index 2db04d4..607bfc7 100644 --- a/loolwsd/protocol.txt +++ b/loolwsd/protocol.txt @@ -374,9 +374,17 @@ Admin console can also opt to get notified of various events on the server. For example, getting notified when a new document is opened or closed. Notifications are commands sent from server to the client over established websocket. +Before doing anything, clients must authenticate by providing an auth token with +'auth' command. + client -> admin ============== +auth jwt=<jwtToken> + + Authenticate the client with provided jwtToken. This is necessary before any + other command. + subscribe <space seperated list of commands> Where list of commands are the ones that client wants to get notified @@ -452,6 +460,14 @@ section). Others are just response messages to some client command. <memory consumed> in kilobytes sent from admin -> client after every mem_stats_interval (see `set` command for list of settings) +InvalidAuthToken + + This is sent when invalid auth token is provided in 'auth' command. See + above. + +NotAuthenticated + + When client sends an admin command that requires authentication. documents <pid> <filename> <number of views> <memory consumed> <elapsed time> <pid> <filename> .... diff --git a/loolwsd/test/UnitAdmin.cpp b/loolwsd/test/UnitAdmin.cpp index edd6bb1..af78344 100644 --- a/loolwsd/test/UnitAdmin.cpp +++ b/loolwsd/test/UnitAdmin.cpp @@ -109,83 +109,98 @@ private: // Check cookie properties std::string cookiePath = cookies[0].getPath(); bool secure = cookies[0].getSecure(); - bool httpOnly = cookies[0].getHttpOnly(); std::string value = cookies[0].getValue(); TestResult res = TestResult::TEST_FAILED; - if (cookiePath.find_first_of("/lool/adminws/") == 0 && - secure && - httpOnly && - value != "") + if (cookiePath.find_first_of("/loleaflet/dist/admin/") == 0 && + value != "" && + secure) { // Set JWT cookie to be used for subsequent tests _jwtCookie = value; res = TestResult::TEST_OK; } + else + { + Log::info("testCorrectPassword: Invalid cookie properties"); + } Log::info(std::string("testCorrectPassword: ") + (res == TestResult::TEST_OK ? "OK" : "FAIL")); return res; } - TestResult testWebSocketWithoutCookie() + TestResult testWebSocketWithoutAuthToken() { - // try connecting without cookie; should result in exception + // try connecting without authentication; should result in NotAuthenticated HTTPResponse response; - HTTPRequest request(HTTPRequest::HTTP_GET, "/adminws/"); + HTTPRequest request(HTTPRequest::HTTP_GET, "/lool/adminws/"); std::unique_ptr<HTTPClientSession> session(helpers::createSession(_uri)); - bool authorized = true; - try + + _adminWs = std::make_shared<Poco::Net::WebSocket>(*session, request, response); + const std::string testMessage = "documents"; + std::unique_lock<std::mutex> lock(_messageReceivedMutex); + _messageReceived.clear(); + _adminWs->sendFrame(testMessage.data(), testMessage.size()); + if (_messageReceivedCV.wait_for(lock, std::chrono::milliseconds(_messageTimeoutMilliSeconds)) == std::cv_status::timeout) { - _adminWs = std::make_shared<Poco::Net::WebSocket>(*session, request, response); + Log::info("testWebSocketWithoutAuth: Timed out waiting for admin console message"); + return TestResult::TEST_TIMED_OUT; } - catch (const Poco::Net::WebSocketException& exc) + lock.unlock(); + + StringTokenizer tokens(_messageReceived, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM); + if (tokens.count() != 1 || + tokens[0] != "NotAuthenticated") { - Log::info() << "Admin websocket: Not authorized " << Log::end; - authorized = false; + Log::info("testWebSocketWithoutAuth: Unrecognized message format"); + return TestResult::TEST_FAILED; } - // no cookie -> should result in not authorized exception - TestResult res = TestResult::TEST_FAILED; - if (!authorized) - res = TestResult::TEST_OK; - - Log::info(std::string("testWebSocketWithoutCookie: ") + (res == TestResult::TEST_OK ? "OK" : "FAIL")); - return res; + Log::info(std::string("testWebSocketWithoutAuth: OK")); + return TestResult::TEST_OK; } - TestResult testWebSocketWithCookie() + TestResult testWebSocketWithIncorrectAuthToken() { + // try connecting with incorrect auth token; should result in InvalidToken HTTPResponse response; HTTPRequest request(HTTPRequest::HTTP_GET, "/lool/adminws/"); std::unique_ptr<HTTPClientSession> session(helpers::createSession(_uri)); - // set cookie - assert(_jwtCookie != ""); - HTTPCookie cookie("jwt", _jwtCookie); - Poco::Net::NameValueCollection nvc; - nvc.add("jwt", _jwtCookie); - request.setCookies(nvc); - - bool authorized = true; - try + _adminWs = std::make_shared<Poco::Net::WebSocket>(*session, request, response); + const std::string testMessage = "auth jwt=incorrectJWT"; + std::unique_lock<std::mutex> lock(_messageReceivedMutex); + _messageReceived.clear(); + _adminWs->sendFrame(testMessage.data(), testMessage.size()); + if (_messageReceivedCV.wait_for(lock, std::chrono::milliseconds(_messageTimeoutMilliSeconds)) == std::cv_status::timeout) { - _adminWs = std::make_shared<Poco::Net::WebSocket>(*session, request, response); + Log::info("testWebSocketWithIncorrectAuthToken: Timed out waiting for admin console message"); + return TestResult::TEST_TIMED_OUT; } - catch (const Poco::Net::WebSocketException& exc) + lock.unlock(); + + StringTokenizer tokens(_messageReceived, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM); + if (tokens.count() != 1 || + tokens[0] != "InvalidAuthToken") { - Log::info() << "Admin websocket: Not authorized " << Log::end; - authorized = false; + Log::info("testWebSocketWithIncorrectAuthToken: Unrecognized message format"); + return TestResult::TEST_FAILED; } - TestResult res = TestResult::TEST_FAILED; - if (authorized) - res = TestResult::TEST_OK; - - Log::info(std::string("testWebSocketWithCookie: ") + (res == TestResult::TEST_OK ? "OK" : "FAIL")); - return res; + Log::info(std::string("testWebSocketWithIncorrectAuthToken: OK")); + return TestResult::TEST_OK; } TestResult testAddDocNotify() { + // Authenticate first + HTTPResponse response; + HTTPRequest request(HTTPRequest::HTTP_GET, "/lool/adminws/"); + std::unique_ptr<HTTPClientSession> session(helpers::createSession(_uri)); + + _adminWs = std::make_shared<Poco::Net::WebSocket>(*session, request, response); + const std::string authMessage = "auth jwt=" + _jwtCookie; + _adminWs->sendFrame(authMessage.data(), authMessage.size()); + // subscribe notification on admin websocket const std::string subscribeMessage = "subscribe adddoc"; _adminWs->sendFrame(subscribeMessage.data(), subscribeMessage.size()); @@ -399,8 +414,8 @@ public: // Register tests here. _tests.push_back(&UnitAdmin::testIncorrectPassword); _tests.push_back(&UnitAdmin::testCorrectPassword); - _tests.push_back(&UnitAdmin::testWebSocketWithoutCookie); - _tests.push_back(&UnitAdmin::testWebSocketWithCookie); + _tests.push_back(&UnitAdmin::testWebSocketWithoutAuthToken); + _tests.push_back(&UnitAdmin::testWebSocketWithIncorrectAuthToken); _tests.push_back(&UnitAdmin::testAddDocNotify); _tests.push_back(&UnitAdmin::testUsersCount); _tests.push_back(&UnitAdmin::testDocCount); commit a97b4f52b883b62c1bedab02cc71139d2c575d62 Author: Pranav Kant <pran...@collabora.co.uk> Date: Thu Jul 28 13:54:25 2016 +0530 loolwsd: Make FileServer a singleton diff --git a/loolwsd/FileServer.cpp b/loolwsd/FileServer.cpp index c0a0805..54c7133 100644 --- a/loolwsd/FileServer.cpp +++ b/loolwsd/FileServer.cpp @@ -244,4 +244,9 @@ void FileServerRequestHandler::preprocessFile(HTTPServerRequest& request, HTTPSe ostr << preprocess; } +FileServer::FileServer() +{ + Log::info("FileServer ctor."); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/loolwsd/FileServer.hpp b/loolwsd/FileServer.hpp index ac8d44d..e2afc47 100644 --- a/loolwsd/FileServer.hpp +++ b/loolwsd/FileServer.hpp @@ -37,23 +37,26 @@ public: void handleRequest(Poco::Net::HTTPServerRequest& request, Poco::Net::HTTPServerResponse& response) override; }; +// Singleton class FileServer { public: - FileServer() + static FileServer& instance() { - Log::info("File server ctor."); + static FileServer fileServer; + return fileServer; } - ~FileServer() - { - Log::info("File Server dtor."); - } - - FileServerRequestHandler* createRequestHandler() + static FileServerRequestHandler* createRequestHandler() { return new FileServerRequestHandler(); } + + FileServer(FileServer const&) = delete; + void operator=(FileServer const&) = delete; + +private: + FileServer(); }; #endif diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index daa8946..6654f1f 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -1152,8 +1152,7 @@ public: class ClientRequestHandlerFactory: public HTTPRequestHandlerFactory { public: - ClientRequestHandlerFactory(FileServer& fileServer) - : _fileServer(fileServer) + ClientRequestHandlerFactory() { } HTTPRequestHandler* createRequestHandler(const HTTPServerRequest& request) override @@ -1182,7 +1181,7 @@ public: // File server if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "loleaflet") { - requestHandler = _fileServer.createRequestHandler(); + requestHandler = FileServer::createRequestHandler(); } // Admin WebSocket Connections else if (reqPathSegs.size() >= 2 && reqPathSegs[0] == "lool" && reqPathSegs[1] == "adminws") @@ -1197,9 +1196,6 @@ public: return requestHandler; } - -private: - FileServer& _fileServer; }; class PrisonerRequestHandlerFactory: public HTTPRequestHandlerFactory @@ -1638,9 +1634,6 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/) return Application::EXIT_SOFTWARE; } - // Init the file server - FileServer fileServer; - // Configure the Server. // Note: TCPServer internally uses a ThreadPool to // dispatch connections (the default if not given). @@ -1657,7 +1650,7 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/) std::unique_ptr<ServerSocket> psvs(lcl_getServerSocket(ClientPortNumber)); ThreadPool threadPool(NumPreSpawnedChildren*6, MAX_SESSIONS * 2); - HTTPServer srv(new ClientRequestHandlerFactory(fileServer), threadPool, *psvs, params1); + HTTPServer srv(new ClientRequestHandlerFactory(), threadPool, *psvs, params1); Log::info("Starting master server listening on " + std::to_string(ClientPortNumber)); srv.start(); commit fee967e510afe63ccb0790d0fc064d888d925d84 Author: Pranav Kant <pran...@collabora.co.uk> Date: Thu Jul 28 10:26:09 2016 +0530 loolwsd: Fix indentation diff --git a/loolwsd/FileServer.cpp b/loolwsd/FileServer.cpp index 60cb58e..c0a0805 100644 --- a/loolwsd/FileServer.cpp +++ b/loolwsd/FileServer.cpp @@ -210,38 +210,38 @@ void FileServerRequestHandler::preprocessFile(HTTPServerRequest& request, HTTPSe { HTMLForm form(request, request.stream()); - const auto host = (LOOLWSD::isSSLEnabled() ? "wss://" : "ws://") + (LOOLWSD::ServerName.empty() ? request.getHost() : LOOLWSD::ServerName); - const auto path = Poco::Path(LOOLWSD::FileServerRoot, getRequestPathname(request)); + const auto host = (LOOLWSD::isSSLEnabled() ? "wss://" : "ws://") + (LOOLWSD::ServerName.empty() ? request.getHost() : LOOLWSD::ServerName); + const auto path = Poco::Path(LOOLWSD::FileServerRoot, getRequestPathname(request)); - Log::debug("Preprocessing file: " + path.toString()); + Log::debug("Preprocessing file: " + path.toString()); - std::string preprocess; - FileInputStream file(path.toString()); - StreamCopier::copyToString(file, preprocess); - file.close(); + std::string preprocess; + FileInputStream file(path.toString()); + StreamCopier::copyToString(file, preprocess); + file.close(); - const std::string& accessToken = form.get("access_token", ""); - const std::string& accessTokenTtl = form.get("access_token_ttl", ""); + const std::string& accessToken = form.get("access_token", ""); + const std::string& accessTokenTtl = form.get("access_token_ttl", ""); - // As of now only alphanumeric characters are allowed in access token - // Sanitize user input before replacing - Poco::RegularExpression re("[a-zA-Z0-9_]*", Poco::RegularExpression::RE_ANCHORED); - if (!re.match(accessToken, 0, 0) || !re.match(accessTokenTtl, 0, 0)) - { - throw Poco::FileAccessDeniedException("Invalid access token provided. Only alphanumeric and _ are allowed "); - } + // As of now only alphanumeric characters are allowed in access token + // Sanitize user input before replacing + Poco::RegularExpression re("[a-zA-Z0-9_]*", Poco::RegularExpression::RE_ANCHORED); + if (!re.match(accessToken, 0, 0) || !re.match(accessTokenTtl, 0, 0)) + { + throw Poco::FileAccessDeniedException("Invalid access token provided. Only alphanumeric and _ are allowed "); + } - Poco::replaceInPlace(preprocess, std::string("%ACCESS_TOKEN%"), accessToken); - Poco::replaceInPlace(preprocess, std::string("%ACCESS_TOKEN_TTL%"), accessTokenTtl); - Poco::replaceInPlace(preprocess, std::string("%HOST%"), host); - Poco::replaceInPlace(preprocess, std::string("%VERSION%"), std::string(LOOLWSD_VERSION_HASH)); + Poco::replaceInPlace(preprocess, std::string("%ACCESS_TOKEN%"), accessToken); + Poco::replaceInPlace(preprocess, std::string("%ACCESS_TOKEN_TTL%"), accessTokenTtl); + Poco::replaceInPlace(preprocess, std::string("%HOST%"), host); + Poco::replaceInPlace(preprocess, std::string("%VERSION%"), std::string(LOOLWSD_VERSION_HASH)); - response.setContentType("text/html"); - response.setContentLength(preprocess.length()); - response.setChunkedTransferEncoding(false); + response.setContentType("text/html"); + response.setContentLength(preprocess.length()); + response.setChunkedTransferEncoding(false); - std::ostream& ostr = response.send(); - ostr << preprocess; + std::ostream& ostr = response.send(); + ostr << preprocess; } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits