Commit fdc9120a by Jérome Perrin

ProFTPd: fixes to support running in a "system level" slapos

This SR was developed and tested in environments where both software and instance where owned by the same unix user. These are fixes for problems discovered when running on a "system level" slapos.

This cannot easily be tested without actually using different users (especially the mistake on the mode of the executable script), but this MR introduce some tests to check that service running in instance does not open files or unix sockets outside of instance. It also introduce a utility method to access the supervisor RPC API, it can be useful if tests needs to start/stop a process or get process PID to inspect this process.

I ran into problems, because proftpd sets [`PR_SET_DUMPABLE`](http://man7.org/linux/man-pages/man2/prctl.2.html) to 0, which disables any possibility of inspecting a running program. See 18e28bbb  for details about the chosen approach.

/cc @rafael @Nicolas @luke @Eteri @vpelletier 

/reviewed-on !334
2 parents 46a771a6 61b8e972
From ec7c3929df82855cc708f2074eb163590ecaeeaf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Wed, 23 May 2018 08:55:35 +0200
Subject: [PATCH] mod_rlimit: don't change PR_SET_DUMPABLE flag
In our case we don't really care if the process leaves a coredump and
are interested in being able to inspect running processes for tests.
---
modules/mod_rlimit.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/modules/mod_rlimit.c b/modules/mod_rlimit.c
index 6788e4cee..2584260af 100644
--- a/modules/mod_rlimit.c
+++ b/modules/mod_rlimit.c
@@ -568,6 +568,8 @@ static int rlimit_set_core(int scope) {
#if !defined(PR_DEVEL_COREDUMP) && \
defined(HAVE_PRCTL) && \
defined(PR_SET_DUMPABLE)
+
+ #if 0 /* SlapOS patch: keep PR_SET_DUMPABLE for tests */
if (max == 0) {
/* Really, no core dumps please. On Linux, there are exceptions made
* even when setting RLIMIT_CORE = 0; see:
@@ -582,6 +584,7 @@ static int rlimit_set_core(int scope) {
strerror(errno));
}
}
+#endif /* SlapOS patch */
#endif /* no --enable-devel=coredump and HAVE_PRCTL and PR_SET_DUMPABLE */
errno = xerrno;
--
2.11.0
......@@ -11,6 +11,7 @@ extends =
../curl/buildout.cfg
../libtool/buildout.cfg
../git/buildout.cfg
../patch/buildout.cfg
# proftpd server
[proftpd-environment]
......@@ -37,7 +38,10 @@ environment =
install_group=${proftpd-grp:GROUP}
cppflags=-I${zlib:location}/include -I${openssl:location}/include
ldflags=-L${zlib:location}/lib -Wl,-rpath=${zlib:location}/lib -L${openssl:location}/lib -Wl,-rpath=${openssl:location}/lib
patch-binary = ${patch:location}/bin/patch
patch-options = -p1
patches =
${:_profile_base_location_}/0001-mod_rlimit-don-t-change-PR_SET_DUMPABLE-flag.patch#6e58a7a429ff96a51812dc9835e4c227
# mod_auth_web: a proftpd module to authenticate users against an HTTP service
[proftpd-mod_auth_web-repository]
......@@ -63,13 +67,13 @@ command =
location=${proftpd:location}/libexec/mod_auth_web.so
# ftpasswod: a perl script to manage a proftpd AuthUserFile
# ftpasswd: a perl script to manage a proftpd AuthUserFile
[ftpasswd]
recipe = slapos.recipe.build:download
url = https://raw.githubusercontent.com/proftpd/proftpd/v1.3.6/contrib/ftpasswd
md5sum = 4a47df2cab86d8de7077a445bb416f31
download-only = true
mode = 0700
mode = 0755
[proftpd-output]
......
......@@ -20,9 +20,9 @@ md5sum = 8ed5b4a7940db47ccb386c2f4e3e7273
[instance-default]
filename = instance-default.cfg.in
md5sum = 89e145188fe2a223f22f57e5c0c242e4
md5sum = e2e67390753ec63babcc8d4ce9fc6230
[proftpd-config-file]
filename = proftpd-config-file.cfg.in
md5sum = 46cbe4ea65b445822d3da32c76c7d67d
md5sum = a7c0f4607c378b640379cc258a8aadfa
......@@ -62,6 +62,8 @@ url = sftp://[${:host}]:${:sftp-port}
data-dir = ${directory:proftpd-dir}
user=${proftpd-userinfo:pw-name}
group=${proftpd-userinfo:gr-name}
scoreboard-file=${directory:var}/proftpd.scoreboard
pid-file=${directory:var}/proftpd.pid
sftp-log=${directory:log}/proftpd-sftp.log
xfer-log=${directory:log}/proftpd-xfer.log
ban-log=${directory:log}/proftpd-ban.log
......@@ -70,6 +72,7 @@ ssh-host-dsa-key=${ssh-host-dsa-key:output}
ssh-host-ecdsa-key=${ssh-host-ecdsa-key:output}
ssh-authorized-keys-dir = ${directory:ssh-authorized-keys-dir}
ban-table=${directory:srv}/proftpd-ban-table
control-socket=${directory:var}/proftpd.sock
auth-user-file=${auth-user-file:output}
recipe = slapos.cookbook:wrapper
......
......@@ -9,6 +9,9 @@ Port {{ proftpd['sftp-port'] }}
User {{ proftpd['user'] }}
Group {{ proftpd['group'] }}
ScoreboardFile {{ proftpd['scoreboard-file'] }}
PidFile {{ proftpd['pid-file'] }}
Umask 022
AllowOverwrite on
......@@ -63,5 +66,6 @@ BanTable {{ proftpd['ban-table'] }}
# 5 failed login attemps in 5 minutes -> ban for 20 minutes
BanOnEvent MaxLoginAttempts 5/00:05:00 00:20:00 "Too many Failed Login Attempts"
BanControlsACLs all allow user {{ proftpd['user'] }}
# This depends on a control socket
ControlsSocket {{ proftpd['control-socket'] }}
ControlsSocketOwner {{ proftpd['user'] }} {{ proftpd['group'] }}
......@@ -45,6 +45,8 @@ setup(name=name,
'slapos.core',
'erp5.util',
'pysftp',
'supervisor',
'psutil',
],
zip_safe=True,
test_suite='test',
......
......@@ -33,6 +33,7 @@ import StringIO
import subprocess
import pysftp
import psutil
from paramiko.ssh_exception import SSHException
from paramiko.ssh_exception import AuthenticationException
......@@ -204,3 +205,29 @@ class TestInstanceParameterPort(ProFTPdTestCase):
self.assertTrue(self._getConnection())
class TestFilesAndSocketsInInstanceDir(ProFTPdTestCase):
"""Make sure the instance only have files and socket in software dir.
"""
def setUp(self):
"""sets `self.proftpdProcess` to `psutil.Process` for the running proftpd in the instance.
"""
all_process_info = self.getSupervisorRPCServer().supervisor.getAllProcessInfo()
# there is only one process in this instance
process_info, = [p for p in all_process_info if p['name'] != 'watchdog']
process = psutil.Process(process_info['pid'])
self.assertEqual('proftpd', process.name()) # sanity check
self.proftpdProcess = process
def test_only_write_file_in_instance_dir(self):
self.assertEqual(
[],
[f for f in self.proftpdProcess.open_files()
if f.mode != 'r'
if not f.path.startswith(self.computer_partition_root_path)])
def test_only_unix_socket_in_instance_dir(self):
self.assertEqual(
[],
[s for s in self.proftpdProcess.connections('unix')
if not s.laddr.startswith(self.computer_partition_root_path)])
......@@ -31,6 +31,8 @@ import socket
from contextlib import closing
import logging
import xmlrpclib
import supervisor.xmlrpc
from erp5.util.testnode.SlapOSControler import SlapOSControler
from erp5.util.testnode.ProcessManager import ProcessManager
import slapos
......@@ -175,3 +177,18 @@ class SlapOSInstanceTestCase(unittest.TestCase):
@classmethod
def tearDownClass(cls):
cls.stopSlapOSProcesses()
# utility methods
def getSupervisorRPCServer(self):
"""Returns a XML-RPC connection to the supervisor used by slapos node
Refer to http://supervisord.org/api.html for details of available methods.
"""
# xmlrpc over unix socket https://stackoverflow.com/a/11746051/7294664
return xmlrpclib.ServerProxy(
'http://slapos-supervisor',
transport=supervisor.xmlrpc.SupervisorTransport(
None,
None,
# XXX hardcoded socket path
serverurl="unix://{working_directory}/inst/supervisord.socket".format(**self.config)))
Styling with Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!