loolwsd.xml.in | 6 +- wsd/FileServer.cpp | 158 ++++++++++++++++++++++++++++++++--------------------- wsd/LOOLWSD.cpp | 2 3 files changed, 101 insertions(+), 65 deletions(-)
New commits: commit 8c976a01edf51cdced6190d4bc5c457ad8b9c005 Author: Jan Holesovsky <ke...@collabora.com> Date: Wed May 2 12:48:41 2018 +0200 Improve readability of the admin console password check. Also disable PAM by default. Change-Id: Id1197f0d049ce56f698952b87d2c4760412eb8ec Reviewed-on: https://gerrit.libreoffice.org/53727 Reviewed-by: Michael Meeks <michael.me...@collabora.com> Tested-by: Michael Meeks <michael.me...@collabora.com> diff --git a/loolwsd.xml.in b/loolwsd.xml.in index 456790005..1d5ac8dc1 100644 --- a/loolwsd.xml.in +++ b/loolwsd.xml.in @@ -108,9 +108,9 @@ <admin_console desc="Web admin console settings."> <enable desc="Enable the admin console functionality" type="bool" default="true">true</enable> - <enable_pam desc="Enable admin user authentication with PAM" type="bool" default="true">true</enable_pam> - <username desc="The username of the admin console. Must be set, if PAM is not enabled, otherwise it's optional."></username> - <password desc="The password of the admin console. Deprecated on most platforms. Instead, use loolconfig to set up a secure password."></password> + <enable_pam desc="Enable admin user authentication with PAM" type="bool" default="false">false</enable_pam> + <username desc="The username of the admin console. Ignored if PAM is enabled."></username> + <password desc="The password of the admin console. Deprecated on most platforms. Instead, use PAM or loolconfig to set up a secure password."></password> </admin_console> </config> diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp index c7dd9a884..db02203a2 100644 --- a/wsd/FileServer.cpp +++ b/wsd/FileServer.cpp @@ -66,15 +66,16 @@ int functionConversation(int /*num_msg*/, const struct pam_message** /*msg*/, return PAM_SUCCESS; } -bool isPamAuthOk(const std::string user, const std::string pass) +/// Use PAM to check for user / password. +bool isPamAuthOk(const std::string& userProvidedUsr, const std::string& userProvidedPwd) { struct pam_conv localConversation { functionConversation, nullptr }; pam_handle_t *localAuthHandle = NULL; int retval; - localConversation.appdata_ptr = const_cast<char *>(pass.c_str()); + localConversation.appdata_ptr = const_cast<char *>(userProvidedPwd.c_str()); - retval = pam_start("loolwsd", user.c_str(), &localConversation, &localAuthHandle); + retval = pam_start("loolwsd", userProvidedUsr.c_str(), &localConversation, &localAuthHandle); if (retval != PAM_SUCCESS) { @@ -88,7 +89,7 @@ bool isPamAuthOk(const std::string user, const std::string pass) { if (retval == PAM_AUTH_ERR) { - LOG_ERR("PAM authentication failure for user \"" << user << "\"."); + LOG_ERR("PAM authentication failure for user \"" << userProvidedUsr << "\"."); } else { @@ -97,7 +98,7 @@ bool isPamAuthOk(const std::string user, const std::string pass) return false; } - LOG_INF("PAM authentication success for user \"" << user << "\"."); + LOG_INF("PAM authentication success for user \"" << userProvidedUsr << "\"."); retval = pam_end(localAuthHandle, retval); @@ -108,59 +109,48 @@ bool isPamAuthOk(const std::string user, const std::string pass) return true; } -} -bool FileServerRequestHandler::isAdminLoggedIn(const HTTPRequest& request, - HTTPResponse &response) +/// Check for user / password set in loolwsd.xml. +bool isConfigAuthOk(const std::string& userProvidedUsr, const std::string& userProvidedPwd) { - assert(LOOLWSD::AdminEnabled); - const auto& config = Application::instance().config(); - const auto sslKeyPath = config.getString("ssl.key_file_path", ""); + const std::string user = config.getString("admin_console.username", ""); - NameValueCollection cookies; - request.getCookies(cookies); - try + // Check for the username + if (user.empty()) { - const std::string jwtToken = cookies.get("jwt"); - LOG_INF("Verifying JWT token: " << jwtToken); - JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); - if (authAgent.verify(jwtToken)) - { - LOG_TRC("JWT token is valid"); - return true; - } - - LOG_INF("Invalid JWT token, let the administrator re-login"); + LOG_ERR("Admin Console username missing, admin console disabled."); + return false; } - catch (const Poco::Exception& exc) + else if (user != userProvidedUsr) { - LOG_INF("No existing JWT cookie found"); + LOG_ERR("Admin Console wrong username."); + return false; } - HTTPBasicCredentials credentials(request); - std::string userProvidedPwd = credentials.getPassword(); - std::string userProvidedUsr = credentials.getUsername(); - - // If no cookie found, or is invalid, let admin re-login - const std::string user = config.getString("admin_console.username", ""); - std::string pass = config.getString("admin_console.password", ""); - const bool pam = config.getBool("admin_console.enable_pam", "true"); + const char useLoolconfig[] = " Use loolconfig to configure the admin password."; + // do we have secure_password? if (config.has("admin_console.secure_password")) { + const std::string securePass = config.getString("admin_console.secure_password", ""); + if (securePass.empty()) + { + LOG_ERR("Admin Console secure password is empty, denying access." << useLoolconfig); + return false; + } + #if HAVE_PKCS5_PBKDF2_HMAC - pass = config.getString("admin_console.secure_password"); // Extract the salt from the config std::vector<unsigned char> saltData; - std::vector<std::string> tokens = LOOLProtocol::tokenize(pass, '.'); + std::vector<std::string> tokens = LOOLProtocol::tokenize(securePass, '.'); if (tokens.size() != 5 || tokens[0] != "pbkdf2" || tokens[1] != "sha512" || !Util::dataFromHexString(tokens[3], saltData)) { - LOG_ERR("Incorrect format detected for secure_password in config file." - << "Use loolconfig to configure admin password."); + LOG_ERR("Incorrect format detected for secure_password in config file." << useLoolconfig); + return false; } unsigned char userProvidedPwdHash[tokens[4].size() / 2]; @@ -174,52 +164,98 @@ bool FileServerRequestHandler::isAdminLoggedIn(const HTTPRequest& request, for (unsigned long j = 0; j < sizeof userProvidedPwdHash; ++j) stream << std::hex << std::setw(2) << std::setfill('0') << static_cast<int>(userProvidedPwdHash[j]); - userProvidedPwd = stream.str(); - pass = tokens[4]; + // now compare the hashed user-provided pwd against the stored hash + return stream.str() == tokens[4]; #else + const std::string pass = config.getString("admin_console.password", ""); LOG_ERR("The config file has admin_console.secure_password setting, " << "but this application was compiled with old OpenSSL version, " - << "and this setting cannot be used. Falling back to plain text password or to PAM, if it is set."); + << "and this setting cannot be used." << (!pass.empty()? " Falling back to plain text password.": "")); + + // careful, a fall-through! #endif } - if (!pam && (user.empty() || pass.empty())) + const std::string pass = config.getString("admin_console.password", ""); + if (pass.empty()) { - LOG_ERR("Admin Console credentials missing. Denying access until set."); + LOG_ERR("Admin Console password is empty, denying access." << useLoolconfig); return false; } - bool authenticated = false; + return pass == userProvidedPwd; +} + +} + +bool FileServerRequestHandler::isAdminLoggedIn(const HTTPRequest& request, + HTTPResponse &response) +{ + assert(LOOLWSD::AdminEnabled); + + const auto& config = Application::instance().config(); + const auto sslKeyPath = config.getString("ssl.key_file_path", ""); - if (userProvidedUsr == user && userProvidedPwd == pass) + NameValueCollection cookies; + request.getCookies(cookies); + try { - authenticated = true; + const std::string jwtToken = cookies.get("jwt"); + LOG_INF("Verifying JWT token: " << jwtToken); + JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); + if (authAgent.verify(jwtToken)) + { + LOG_TRC("JWT token is valid"); + return true; + } + + LOG_INF("Invalid JWT token, let the administrator re-login"); + } + catch (const Poco::Exception& exc) + { + LOG_INF("No existing JWT cookie found"); } - if (!authenticated && pam) + // If no cookie found, or is invalid, let the admin re-login + HTTPBasicCredentials credentials(request); + std::string userProvidedUsr = credentials.getUsername(); + std::string userProvidedPwd = credentials.getPassword(); + + // Deny attempts to login without providing a username / pwd and fail right away + // We don't even want to allow a password-less PAM module to be used here, + // or anything. + if (userProvidedUsr.empty() || userProvidedPwd.empty()) { - authenticated = isPamAuthOk(userProvidedUsr, userProvidedPwd); + LOG_WRN("An attempt to log into Admin Console without username or password."); + return false; } - if (authenticated) + // Check if the user is allowed to use the admin console + if (config.getBool("admin_console.enable_pam", "false")) { - // generate and set the cookie - JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); - const std::string jwtToken = authAgent.getAccessToken(); - - Poco::Net::HTTPCookie cookie("jwt", jwtToken); - // bundlify appears to add an extra /dist -> dist/dist/admin - cookie.setPath("/loleaflet/dist/"); - cookie.setSecure(LOOLWSD::isSSLEnabled() || - LOOLWSD::isSSLTermination()); - response.addCookie(cookie); + // use PAM - it needs the username too + if (!isPamAuthOk(userProvidedUsr, userProvidedPwd)) + return false; } else { - LOG_INF("Wrong admin credentials."); + // use the hash or password in the config file + if (!isConfigAuthOk(userProvidedUsr, userProvidedPwd)) + return false; } - return authenticated; + // authentication passed, generate and set the cookie + JWTAuth authAgent(sslKeyPath, "admin", "admin", "admin"); + const std::string jwtToken = authAgent.getAccessToken(); + + Poco::Net::HTTPCookie cookie("jwt", jwtToken); + // bundlify appears to add an extra /dist -> dist/dist/admin + cookie.setPath("/loleaflet/dist/"); + cookie.setSecure(LOOLWSD::isSSLEnabled() || + LOOLWSD::isSSLTermination()); + response.addCookie(cookie); + + return true; } void FileServerRequestHandler::handleRequest(const HTTPRequest& request, Poco::MemoryInputStream& message, diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 28fcc0dd0..d329c4e3f 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -709,7 +709,7 @@ void LOOLWSD::initialize(Application& self) { "trace[@enable]", "false" }, { "trace.path[@compress]", "true" }, { "trace.path[@snapshot]", "false" }, - { "admin_console.enable_pam", "true"} }; + { "admin_console.enable_pam", "false"} }; // Set default values, in case they are missing from the config file. AutoPtr<AppConfigMap> defConfig(new AppConfigMap(DefAppConfig)); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits