[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3' - loolwsd.xml.in wsd/FileServer.cpp wsd/LOOLWSD.cpp
Jan Holesovsky
kendy at collabora.com
Wed May 2 14:19:59 UTC 2018
loolwsd.xml.in | 6 +-
wsd/FileServer.cpp | 158 ++++++++++++++++++++++++++++++++---------------------
wsd/LOOLWSD.cpp | 2
3 files changed, 101 insertions(+), 65 deletions(-)
New commits:
commit f12049919764fe0af2563ba71ef4308b2a929e49
Author: Jan Holesovsky <kendy at 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.meeks at collabora.com>
Tested-by: Michael Meeks <michael.meeks at collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/53732
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));
More information about the Libreoffice-commits
mailing list