[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3' - kit/ForKit.cpp kit/Kit.cpp kit/Kit.hpp loolwsd.xml.in Makefile.am wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp
Michael Meeks
michael.meeks at collabora.com
Tue Mar 20 12:27:41 UTC 2018
Makefile.am | 4 ----
kit/ForKit.cpp | 17 +++++++++++++----
kit/Kit.cpp | 13 ++++++++++---
kit/Kit.hpp | 1 +
loolwsd.xml.in | 5 +++++
wsd/LOOLWSD.cpp | 36 +++++++++++++++++++++---------------
wsd/LOOLWSD.hpp | 1 +
7 files changed, 51 insertions(+), 26 deletions(-)
New commits:
commit 527b95de1425fd2969099f78c0a526a9fcdac2b4
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Mar 19 15:20:10 2018 +0000
Allow running without seccomp and capabilities.
There are some significant security trade-offs here which are now
at least configurable.
Change-Id: I1d879d69e91392f4ccf5db250a2277f53df60db7
Reviewed-on: https://gerrit.libreoffice.org/51590
Reviewed-by: Jan Holesovsky <kendy at collabora.com>
Tested-by: Jan Holesovsky <kendy at collabora.com>
diff --git a/Makefile.am b/Makefile.am
index e4c773253..e811e0010 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -86,7 +86,6 @@ loolwsd_SOURCES = $(loolwsd_sources) \
noinst_PROGRAMS = clientnb \
connect \
lokitclient \
- loolforkit-nocaps \
loolwsd_fuzzer
connect_SOURCES = tools/Connect.cpp \
@@ -116,9 +115,6 @@ clientnb_SOURCES = net/clientnb.cpp \
common/Log.cpp \
common/Util.cpp
-# build a binary with no caps to help debugging
-loolforkit_nocaps_SOURCES = $(loolforkit_SOURCES)
-
loolmount_SOURCES = tools/mount.cpp
loolmap_SOURCES = tools/map.cpp
diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index b3ca8c64e..ca2defaf6 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -49,6 +49,7 @@ using Poco::Util::Application;
#ifndef KIT_IN_PROCESS
static bool NoCapsForKit = false;
+static bool NoSeccomp = false;
#endif
static bool DisplayVersion = false;
static std::string UnitTestLibrary;
@@ -270,9 +271,9 @@ static int createLibreOfficeKit(const std::string& childRoot,
}
#ifndef KIT_IN_PROCESS
- lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, NoCapsForKit, queryVersion, DisplayVersion);
+ lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, NoCapsForKit, NoSeccomp, queryVersion, DisplayVersion);
#else
- lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, true, queryVersion, DisplayVersion);
+ lokit_main(childRoot, jailId, sysTemplate, loTemplate, loSubPath, true, true, queryVersion, DisplayVersion);
#endif
}
else
@@ -455,12 +456,20 @@ int main(int argc, char** argv)
eq = std::strchr(cmd, '=');
UnitTestLibrary = std::string(eq+1);
}
- // we are running in no-privilege mode - with no chroot etc.
+#endif
+ // we are running in a lower-privilege mode - with no chroot
else if (std::strstr(cmd, "--nocaps") == cmd)
{
+ LOG_ERR("Security: Running without the capability to enter a chroot jail is ill advised.");
NoCapsForKit = true;
}
-#endif
+
+ // we are running without seccomp protection
+ else if (std::strstr(cmd, "--noseccomp") == cmd)
+ {
+ LOG_ERR("Security :Running without the ability to filter system calls is ill advised.");
+ NoSeccomp = true;
+ }
}
if (loSubPath.empty() || sysTemplate.empty() ||
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index b99d7e413..f7b169c0b 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -1925,6 +1925,7 @@ void lokit_main(const std::string& childRoot,
const std::string& loTemplate,
const std::string& loSubPath,
bool noCapabilities,
+ bool noSeccomp,
bool queryVersion,
bool displayVersion)
{
@@ -2070,7 +2071,7 @@ void lokit_main(const std::string& childRoot,
}
else // noCapabilities set
{
- LOG_INF("Using template " << loTemplate << " as install subpath - skipping jail setup");
+ LOG_ERR("Security warning - using template " << loTemplate << " as install subpath - skipping chroot jail setup");
userdir_url = "file:///" + jailPath.toString() + "/user";
instdir_path = "/" + loTemplate + "/program";
}
@@ -2107,8 +2108,14 @@ void lokit_main(const std::string& childRoot,
// Lock down the syscalls that can be used
if (!Seccomp::lockdown(Seccomp::Type::KIT))
{
- LOG_ERR("LibreOfficeKit security lockdown failed. Exiting.");
- std::_Exit(Application::EXIT_SOFTWARE);
+ if (!noSeccomp)
+ {
+ LOG_ERR("LibreOfficeKit seccomp security lockdown failed. Exiting.");
+ std::_Exit(Application::EXIT_SOFTWARE);
+ }
+
+ LOG_ERR("LibreOfficeKit seccomp security lockdown failed, but configured to continue. "
+ "You are running in a significantly less secure mode.");
}
rlimit rlim = { 0, 0 };
diff --git a/kit/Kit.hpp b/kit/Kit.hpp
index 829ab2e2e..ac80b4869 100644
--- a/kit/Kit.hpp
+++ b/kit/Kit.hpp
@@ -20,6 +20,7 @@ void lokit_main(const std::string& childRoot,
const std::string& loTemplate,
const std::string& loSubPath,
bool noCapabilities,
+ bool noSeccomp,
bool queryVersionInfo,
bool displayVersion);
diff --git a/loolwsd.xml.in b/loolwsd.xml.in
index 9103395d8..38e81a2b0 100644
--- a/loolwsd.xml.in
+++ b/loolwsd.xml.in
@@ -82,6 +82,11 @@
</hpkp>
</ssl>
+ <security desc="Altering these defaults potentially opens you to significant risk">
+ <seccomp desc="Should we use the seccomp system call filtering." type="bool" default="true">true</seccomp>
+ <capabilities desc="Should we require capabilities to isolate processes into chroot jails" type="bool" default="true">true</capabilities>
+ </security>
+
<storage desc="Backend storage">
<filesystem allow="false" />
<wopi desc="Allow/deny wopi storage. Mutually exclusive with webdav." allow="true">
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 2118a0dd2..8d0668c18 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -569,8 +569,9 @@ std::atomic<unsigned> LOOLWSD::NextSessionId;
#ifndef KIT_IN_PROCESS
std::atomic<int> LOOLWSD::ForKitWritePipe(-1);
std::atomic<int> LOOLWSD::ForKitProcId(-1);
-bool LOOLWSD::NoCapsForKit = false;
#endif
+bool LOOLWSD::NoSeccomp = false;
+bool LOOLWSD::NoCapsForKit = false;
#ifdef FUZZER
bool LOOLWSD::DummyLOK = false;
std::string LOOLWSD::FuzzFileName;
@@ -687,6 +688,8 @@ void LOOLWSD::initialize(Application& self)
{ "ssl.hpkp[@report_only]", "false" },
{ "ssl.hpkp.max_age[@enable]", "true" },
{ "ssl.hpkp.report_uri[@enable]", "false" },
+ { "security.seccomp", "true" },
+ { "security.capabilities", "true" },
{ "storage.filesystem[@allow]", "false" },
{ "storage.wopi[@allow]", "true" },
{ "storage.wopi.host[0][@allow]", "true" },
@@ -845,6 +848,9 @@ void LOOLWSD::initialize(Application& self)
LOOLWSD::MaxConnections = MAX_CONNECTIONS;
LOOLWSD::MaxDocuments = MAX_DOCUMENTS;
+ NoSeccomp = !getConfigValue<bool>(conf, "security.seccomp", true);
+ NoCapsForKit = !getConfigValue<bool>(conf, "security.capabilities", true);
+
#if ENABLE_SUPPORT_KEY
const std::string supportKeyString = getConfigValue<std::string>(conf, "support_key", "");
@@ -1076,14 +1082,14 @@ void LOOLWSD::defineOptions(OptionSet& optionSet)
.repeatable(false)
.argument("unitlib"));
- optionSet.addOption(Option("nocaps", "", "Use a non-privileged forkit for valgrinding.")
- .required(false)
- .repeatable(false));
-
optionSet.addOption(Option("careerspan", "", "How many seconds to run.")
.required(false)
.repeatable(false)
.argument("seconds"));
+
+ optionSet.addOption(Option("nocaps", "", "Use a non-privileged forkit, with increase in security problems.")
+ .required(false)
+ .repeatable(false));
#endif
#ifdef FUZZER
@@ -1127,12 +1133,12 @@ void LOOLWSD::handleOption(const std::string& optionName,
#if ENABLE_DEBUG
else if (optionName == "unitlib")
UnitTestLibrary = value;
+ else if (optionName == "careerspan")
+ careerSpanMs = std::stoi(value) * 1000; // Convert second to ms
#ifndef KIT_IN_PROCESS
else if (optionName == "nocaps")
NoCapsForKit = true;
#endif
- else if (optionName == "careerspan")
- careerSpanMs = std::stoi(value) * 1000; // Convert second to ms
static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
if (clientPort)
@@ -1345,21 +1351,19 @@ bool LOOLWSD::createForKit()
args.push_back("--rlimits=" + ossRLimits.str());
if (UnitWSD::get().hasKitHooks())
- {
args.push_back("--unitlib=" + UnitTestLibrary);
- }
if (DisplayVersion)
- {
args.push_back("--version");
- }
- std::string forKitPath = Path(Application::instance().commandPath()).parent().toString() + "loolforkit";
if (NoCapsForKit)
- {
- forKitPath = forKitPath + std::string("-nocaps");
args.push_back("--nocaps");
- }
+
+ if (NoSeccomp)
+ args.push_back("--noseccomp");
+
+ std::string forKitPath = Path(Application::instance().commandPath()).parent().toString() + "loolforkit";
+
// Always reap first, in case we haven't done so yet.
if (ForKitProcId != -1)
@@ -2545,6 +2549,8 @@ public:
<< " prisoner " << MasterPortNumber << "\n"
<< " SSL: " << (LOOLWSD::isSSLEnabled() ? "https" : "http") << "\n"
<< " SSL-Termination: " << (LOOLWSD::isSSLTermination() ? "yes" : "no") << "\n"
+ << " Security " << (LOOLWSD::NoCapsForKit ? "no" : "") << " chroot, "
+ << (LOOLWSD::NoSeccomp ? "no" : "") << " api lockdown\n"
<< " TerminationFlag: " << TerminationFlag << "\n"
<< " isShuttingDown: " << ShutdownRequestFlag << "\n"
<< " NewChildren: " << NewChildren.size() << "\n"
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index c16ecc37b..603e01ab4 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -43,6 +43,7 @@ public:
static std::atomic<unsigned> NextSessionId;
static unsigned int NumPreSpawnedChildren;
static bool NoCapsForKit;
+ static bool NoSeccomp;
static std::atomic<int> ForKitWritePipe;
static std::atomic<int> ForKitProcId;
static bool DummyLOK;
More information about the Libreoffice-commits
mailing list