From 938dd4670bee27bd46fbf7d64cd0526eeb24669f Mon Sep 17 00:00:00 2001 From: Kay Marquardt Date: Mon, 10 Apr 2017 13:38:16 +0200 Subject: [PATCH] extend plugin password to avoid sudo (use ssh instead calling chpasswd) (#5654) Use proc_open() instead of popen() to catch error messages from called wrapper. Create new wrapper chpass-wrapper-expect.py --- plugins/password/README | 19 ++- plugins/password/config.inc.php.dist | 26 +++- plugins/password/drivers/chpasswd.php | 45 +++++- .../password/helpers/chpass-wrapper-expect.py | 143 ++++++++++++++++++ plugins/password/helpers/chpass-wrapper.py | 93 ++++++++++-- 5 files changed, 299 insertions(+), 27 deletions(-) create mode 100644 plugins/password/helpers/chpass-wrapper-expect.py diff --git a/plugins/password/README b/plugins/password/README index 8f3305a03..a27082b68 100644 --- a/plugins/password/README +++ b/plugins/password/README @@ -262,6 +262,9 @@ Attached wrapper script (helpers/chpass-wrapper.py) restricts password changes to uids >= 1000 and can deny requests based on a blacklist. + + For a more secure setup see the notes in section sudo or use the new wrapper + chpass-wrapper-expect.py, for usage see there. 2.12. LDAP - no PEAR (ldap_simple) @@ -317,7 +320,9 @@ Driver to change user password via the 'expect' command. See config.inc.php.dist file for configuration description. - + + For easyer and more secure of the expect script a new wrapper is + is provided: chpass-wrapper-expect.py, see there and sudo for setup 2.18. Samba (smb) ----------------------------------- @@ -375,11 +380,21 @@ 4. Sudo setup ------------- + try to be more secure and use dovecot or pam methods + if this is not possible in your setup you can increase security by + sudo to a wrapper, where you can implement some security meassures + + 1. two wrappers are provided by this plugin: chpass-wrapper.py / chpass-wrapper_expect.py + 2. move wrapper out of the default location to a random place + 3. change permissons of wrapper to root:www 770 to avoid changes by user or webserver + 4. add some security meassures, i.e. limit userids where password can be changed + 5. allow webserver sudo for wrapper only (se below) + Some drivers that execute system commands (like chpasswd) require use of sudo command. Here's a sample for CentOS 7: # cat </etc/sudoers.d/99-roundcubemail - apache ALL=NOPASSWD:/usr/sbin/chpasswd + apache ALL=NOPASSWD://roundcube/wrapper/chpass-wrapper.py Defaults:apache !requiretty < +// allowing sudo chpasswd directly IMHO opens a security hole! +// any script on the webserver can change password for every user. +// $config['password_chpasswd_cmd'] = 'sudo /usr/sbin/chpasswd 2>/dev/null'; + +// try to be more secure and use other methods +// if this is not possible in your setup, you can increase security by +// sudo to a wrapper, where you can implement some security meassures: +// 1. a simple wraper is provided by this plugin: helpers/chpasswrapper.py +// 2. move wrapper out of default location to a random place +// 3. change permissons of wrapper to root:www 770 to avoid changes by user or webserver +// 4. add some security meassures, i.e. limit userids where password can be changed +// 5. allow webserver sudo for wrapper only (see README) +// $config['password_chpasswd_cmd'] = 'sudo //roundcube/wrapper/chpass-wrapper.py'; + +// IMHO the most flexible and secure method for users with interactive shell access is to use ssh with expect +// I modifed the chpasss driver to provide the old password needed, additionally it pass the script response in case of error. + +// 1. use my wrapper for the nice expect script provided by this plugin: helpers/chpass-wrapper-expect.py +// 2. move wrapper out of default location to a random place +// 3. change permissons of wrapper to root:www 770 to avoid changes by user or webserver +// 4. edit arguments to provide loginmethod and hostname. +// 5. remove sudo rules you may have applied (see README) +$config['password_chpasswd_cmd'] = '//roundcube/wrapper/chpass-wrapper-expect.py -ssh -host localhost'; // XMail Driver options diff --git a/plugins/password/drivers/chpasswd.php b/plugins/password/drivers/chpasswd.php index 45c56dba3..7c6b34b25 100644 --- a/plugins/password/drivers/chpasswd.php +++ b/plugins/password/drivers/chpasswd.php @@ -8,8 +8,9 @@ * * For installation instructions please read the README file. * - * @version 2.0 + * @version 3.0 * @author Alex Cartwright + * @author rewrtitten by KaM * * Copyright (C) 2005-2013, The Roundcube Dev Team * @@ -34,12 +35,43 @@ class rcube_chpasswd_password $cmd = rcmail::get_instance()->config->get('password_chpasswd_cmd'); $username = $_SESSION['username']; - $handle = popen($cmd, "w"); - fwrite($handle, "$username:$newpass\n"); + // change popen to proc_open to chatch responses from command + $desc = array( + 0 => array('pipe', 'r'), // 0 is STDIN for process + 1 => array('pipe', 'w'), // 1 is STDOUT for process + 2 => array('file', strtok(cmd, ' ') . '-error.log', 'a') // 2 is STDERR for process + ); - if (pclose($handle) == 0) { - return PASSWORD_SUCCESS; + // spawn the sub process + $process = proc_open($cmd, $desc, $pipes); + + // does sub prescess exist? + if (is_resource($process)) { + // send send username and new pass to chpasswd command / warpper + fwrite($pipes[0], "$username:$newpass\n"); + // new: send old passwd for expect sript, works also with chpasswd + fwrite($pipes[0], "$currpass\n"); + fclose($pipes[0]); + + // read response from sub process + $message = stream_get_contents($pipes[1]); + + // all done! Clean up + fclose($pipes[1]); + fclose($pipes[2]); + + if (!proc_close($process)) { + return PASSWORD_SUCCESS; + } + else { + // return response in case of error + return array( + 'code' => PASSWORD_ERROR, + 'message' => "
" . $message + ); + } } + // sub process failed! else { rcube::raise_error(array( 'code' => 600, @@ -48,7 +80,4 @@ class rcube_chpasswd_password 'message' => "Password plugin: Unable to execute $cmd" ), true, false); } - - return PASSWORD_ERROR; - } } diff --git a/plugins/password/helpers/chpass-wrapper-expect.py b/plugins/password/helpers/chpass-wrapper-expect.py new file mode 100644 index 000000000..34cf6a57a --- /dev/null +++ b/plugins/password/helpers/chpass-wrapper-expect.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python +# +# use passwd-expect to login to host and change password +# you need to install expect on the webserver host +# i.e. "apt-get install expect" or "yast -i expect" +# +# you need an writeable .ssh dir in webserver homedir +# you can test this by do ssh user@host as webserver +# +# Parameter: +# -ssh use ssh for connetion to host (default) +# -host hostname connect to hostname (default localhost) +# -timeout # 0 - 99 time in s to wait for response +# +# all other parameters are passed to passwd-expect, see there. +# +""" +// 2017-02-13: Remarks by Kay Marquardt kay@rrr.de +// allowing sudo chpasswd directly opens a security hole! +// any script on the webserver can change password for every user, incl. root + +// IMHO the most flexible and secure method for users with interactive shell access is to use ssh with an expect script +// I modifed the chpasss driver to provide the old password needed, additionally it pass the script response in case of error. + +// 1. I wrote a wrapper (this script) for expect script provided by this plugin. +// 2. move wrapper out of default location to a random place +// 3. change permissons of wrapper to root:www 770 to avoid changes by user or webserver +// 4. I add some security meassures(see README) +// 5. remove sudo rules you may have applied (see README) +""" + +# path to ecpect and script name (has to be in the same dir as this script) +# "which expect" show the path to expect programm +expect = '/usr/bin/expect' +script = 'passwd-expect' + +TIMEOUT= 10 + +import os, sys, pwd, re +import subprocess, signal + + +# get args for script and extract hostname for us +hostname='localhost' +scriptargs = '' +count=1 +while(count < len(sys.argv)): + # get hostname + if sys.argv[count] == '-host': + hostname = sys.argv[count+1] + # local only args, do not pass + try: + if sys.argv[count] == '-timeout': + count += 2 + TIMEOUT=int(sys.argv[count-1]) + continue + except ValueError: + continue + # pass all other args + scriptargs += ' ' + sys.argv[count] + count += 1 + + +# read username:password\noldpasswd with timeout +class TimeoutException(Exception): # Custom exception class + pass +def timeout_handler(signum, frame): # Custom signal handler + raise TimeoutException +signal.signal(signal.SIGALRM, timeout_handler) + +# set timeout +signal.alarm(TIMEOUT) +try: + try: + username, password = sys.stdin.readline().split(':', 1) + oldpassw = sys.stdin.readline() + except ValueError, e: + sys.exit('Malformed input') + +except TimeoutException: + sys.exit('Timeout while reading input') +else: + # clear timeout + signal.alarm(0) + + +# add user to BLACKLIST and/or /etc/ftpusers to disable password change +BLACKLIST = [ + # add blacklisted users here + 'ftp', +] + +# add /etc/ftpusers to BLACKLIST if exist +try: + with open("/etc/ftpusers", "r") as ins: + for line in ins: + if line.startswith('#'): + continue + BLACKLIST.append(line.rstrip('\n')) + +except IOError: + # only catch error and continue + pass + + +# check if user is blacklisted for password change +if username in BLACKLIST: + sys.exit('Changing password for user %s is forbidden (user blacklisted)!' % + username) + + +# check if user exit and is allowed to chage password +if hostname == 'localhost': + try: + user = pwd.getpwnam(username) + except KeyError, e: + sys.exit('No such user: %s' % username) + + if user.pw_uid < 1000: + sys.exit('Changing the password for user %s is forbidden (system user)!' % + username) + + +path= os.path.dirname(os.path.realpath(sys.argv[0])) + +# script has to be in same directory +if scriptargs == '': scriptargs = ' -ssh -host ' + hostname +cmd = expect + ' ' + os.path.dirname(os.path.realpath(sys.argv[0])) + '/' + script + scriptargs + ' -log \|cat' + +# set timeout +signal.alarm(TIMEOUT) +try: + handle = subprocess.Popen( cmd, shell=True, stdin = subprocess.PIPE) + handle.communicate('%s\n%s\n%s' % (username, oldpassw.rstrip('\r\n'), password)) +except TimeoutException: + sys.exit('Timeout while changing password (wrong old password)') +else: + # clear timeout + signal.alarm(0) + + +sys.exit(handle.returncode) + diff --git a/plugins/password/helpers/chpass-wrapper.py b/plugins/password/helpers/chpass-wrapper.py index 61bba849e..428deef99 100644 --- a/plugins/password/helpers/chpass-wrapper.py +++ b/plugins/password/helpers/chpass-wrapper.py @@ -1,32 +1,95 @@ #!/usr/bin/env python +# +# extended wrapper to /user/sbin/chpasswd +# - file based blacklist (/etc/ftpusers) +# - timeout for reading and writing -import sys -import pwd -import subprocess +TIMEOUT= 10 -BLACKLIST = ( +import os, sys, pwd, re +import subprocess, signal + + +# get args for script +scriptargs = '' +count=1 +while(count < len(sys.argv)): + # local only args, do not pass + try: + if sys.argv[count] == '-timeout': + count += 2 + TIMEOUT=int(sys.argv[count-1]) + continue + except ValueError: + continue + # pass all other args + scriptargs += ' ' + sys.argv[count] + count += 1 + +# read username:password\noldpasswd with timeout +class TimeoutException(Exception): # Custom exception class + pass +def timeout_handler(signum, frame): # Custom signal handler + raise TimeoutException +signal.signal(signal.SIGALRM, timeout_handler) + +# set timeout +signal.alarm(TIMEOUT) +try: + try: + username, password = sys.stdin.readline().split(':', 1) + except ValueError, e: + sys.exit('Malformed input') + +except TimeoutException: + sys.exit('Timeout while reading input') +else: + # clear timeout + signal.alarm(0) + +# add user to BLACKLIST and/or /etc/ftpusers to disable password change +BLACKLIST = [ # add blacklisted users here - #'user1', -) + 'ftp', +] +# add /etc/ftpusers to BLACKLIST if exist try: - username, password = sys.stdin.readline().split(':', 1) -except ValueError, e: - sys.exit('Malformed input') + with open("/etc/ftpusers", "r") as ins: + for line in ins: + if line.startswith('#'): + continue + BLACKLIST.append(line.rstrip('\n')) + +except IOError: + # only catch error and continue + pass +# check if user is blacklisted for password change +if username in BLACKLIST: + sys.exit('Changing password for user %s is forbidden (user blacklisted)!' % + username) + + +# check if user exit and is allowed to chage password try: user = pwd.getpwnam(username) except KeyError, e: sys.exit('No such user: %s' % username) if user.pw_uid < 1000: - sys.exit('Changing the password for user id < 1000 is forbidden') - -if username in BLACKLIST: - sys.exit('Changing password for user %s is forbidden (user blacklisted)' % + sys.exit('Changing the password for user %s is forbidden (system user)!' % username) -handle = subprocess.Popen('/usr/sbin/chpasswd', stdin = subprocess.PIPE) -handle.communicate('%s:%s' % (username, password)) +# set timeout +signal.alarm(TIMEOUT) +try: + handle = subprocess.Popen('/usr/sbin/chpasswd', stdin = subprocess.PIPE) + handle.communicate('%s:%s' % (username, password)) +except TimeoutException: + sys.exit('Timeout while changing password') +else: + # clear timeout + signal.alarm(0) sys.exit(handle.returncode)