=== modified file 'INSTALL' --- INSTALL 2013-06-23 15:13:06 +0000 +++ INSTALL 2014-06-22 02:19:30 +0000 @@ -47,10 +47,11 @@ Strongly recommended: + fping 2.4b2-to-ipv6 http://www.fping.com/ + + ssh-keyscan from OpenSSH http://www.openssh.com/ Package names: python-gnutls avahi-daemon python python-avahi python-dbus - python-gobject python-urwid + python-gobject python-urwid ssh-client *** Mandos Client + initramfs-tools 0.85i @@ -60,9 +61,12 @@ + GnuPG 1.4.9 http://www.gnupg.org/ + GPGME 1.1.6 http://www.gnupg.org/related_software/gpgme/ + Strongly recommended: + + OpenSSH http://www.openssh.com/ + Package names: initramfs-tools libgnutls-dev libavahi-core-dev gnupg - libgpgme11-dev + libgpgme11-dev ssh * Installing the Mandos server @@ -132,6 +136,6 @@ You may want to tighten or loosen the timeouts in the server configuration files; see mandos.conf(5) and mandos-clients.conf(5). - If IPsec is not used, it is suggested that a more cryptographically - secure checker program is used and configured, since without IPsec - ping packets can be faked. + If IPsec is not used and SSH is not installed, it is suggested that + a more cryptographically secure checker program is used and + configured, since, without IPsec, ping packets can be faked. === modified file 'Makefile' --- Makefile 2014-05-11 20:18:48 +0000 +++ Makefile 2014-06-22 02:19:30 +0000 @@ -264,15 +264,18 @@ @echo "# ignored. The messages are caused by not running as root, but #" @echo "# you should NOT run \"make run-client\" as root unless you also #" @echo "# unpacked and compiled Mandos as root, which is NOT recommended. #" - @echo "# From plugin-runner: setuid: Operation not permitted #" + @echo "# From plugin-runner: setgid: Operation not permitted #" + @echo "# setuid: Operation not permitted #" @echo "# From askpass-fifo: mkfifo: Permission denied #" - @echo "# From mandos-client: setuid: Operation not permitted #" - @echo "# seteuid: Operation not permitted #" - @echo "# klogctl: Operation not permitted #" + @echo "# From mandos-client: #" + @echo "# Failed to raise privileges: Operation not permitted #" + @echo "# Warning: network hook \"*\" exited with status * #" @echo "###################################################################" +# We set GNOME_KEYRING_CONTROL to block pam_gnome_keyring ./plugin-runner --plugin-dir=plugins.d \ --config-file=plugin-runner.conf \ --options-for=mandos-client:--seckey=keydir/seckey.txt,--pubkey=keydir/pubkey.txt,--network-hook-dir=network-hooks.d \ + --env-for=mandos-client:GNOME_KEYRING_CONTROL= \ $(CLIENTARGS) # Used by run-client @@ -293,7 +296,7 @@ install --directory confdir install --mode=u=rw $< $@ # Add a client password - ./mandos-keygen --dir keydir --password >> $@ + ./mandos-keygen --dir keydir --password --no-ssh >> $@ statedir: install --directory statedir === modified file 'TODO' --- TODO 2014-03-27 21:24:33 +0000 +++ TODO 2014-06-22 02:19:30 +0000 @@ -44,8 +44,6 @@ * plugin-runner ** TODO handle printing for errors for plugins *** Hook up stderr of plugins, buffer them, and prepend "Mandos Plugin [plugin name]" -** TODO [#B] use scandirat(3) instead of readdir(3) -*** Must wait until GNU libc 2.15 ** TODO [#C] use same file name rules as run-parts(8) ** kernel command line option for debug info @@ -83,7 +81,11 @@ http://standards.freedesktop.org/secret-service/ ** TODO Remove D-Bus interfaces with old domain name :2: ** TODO Remove old string_to_delta format :2: -** TODO --no-zeroconf (only valid if port or socket is set) +** TODO http://0pointer.de/blog/projects/stateless.html +*** tmpfiles snippet to create /var/lib/mandos with right user+perms +*** File in /usr/lib/sysusers.d to create user+group "_mandos" +** TODO Error handling on error parsing config files +** TODO init.d script error handling * mandos.xml ** Add mandos contact info in manual pages @@ -104,6 +106,7 @@ *** Properties popup ** Print a nice "We are sorry" message, save stack trace to log. ** Rename module "gobject" to "GObject". +** TODO Optional verbose mode to see checkers starting and succeeding * mandos-keygen ** TODO "--secfile" option === modified file 'debian/control' --- debian/control 2014-05-30 09:53:03 +0000 +++ debian/control 2014-07-12 12:54:13 +0000 @@ -21,7 +21,7 @@ python-avahi, python-gobject, avahi-daemon, adduser, python-urwid, python (>=2.7) | python-argparse, gnupg (<< 2), initscripts (>= 2.88dsf-13.3) -Recommends: fping +Recommends: ssh-client | fping Description: server giving encrypted passwords to Mandos clients This is the server part of the Mandos system, which allows computers to have encrypted root file systems and at the @@ -41,6 +41,7 @@ Architecture: linux-any Depends: ${shlibs:Depends}, ${misc:Depends}, adduser, cryptsetup, gnupg (<< 2), initramfs-tools, dpkg-dev (>=1.16.0) +Recommends: ssh Breaks: dropbear (<= 0.53.1-1) Enhances: cryptsetup Description: do unattended reboots with an encrypted root file system === modified file 'debian/mandos.postinst' --- debian/mandos.postinst 2014-01-06 17:22:30 +0000 +++ debian/mandos.postinst 2014-06-07 20:29:36 +0000 @@ -51,7 +51,7 @@ # "avahi-daemon") in its /etc/init.d script header. To make # insserv(8) happy, we edit our /etc/init.d script header to contain # the correct string before the code added by dh_installinit calls -# update.rd-c, which calls insserv. +# update.rc-d, which calls insserv. avahi_version="`dpkg-query --showformat='${Version}' --show avahi-daemon`" if dpkg --compare-versions "$avahi_version" le 0.6.31-2; then sed --in-place --expression='/^### BEGIN INIT INFO$/,/^### END INIT INFO$/s/^\(# Required-\(Stop\|Start\):.*avahi\)-daemon\>/\1/g' /etc/init.d/mandos === modified file 'intro.xml' --- intro.xml 2011-12-31 23:05:34 +0000 +++ intro.xml 2014-06-22 02:19:30 +0000 @@ -1,7 +1,7 @@ + %common; ]> @@ -215,17 +215,22 @@ - - Faking ping replies? + + Faking checker results? - The default for the server is to use + If the Mandos client does not have an SSH server, the default + is for the Mandos server to use fping, the replies to which could be faked to eliminate the timeout. But this could easily be changed to any shell command, with any security - measures you like. It could, for instance, be changed to an - SSH command with strict keychecking, which could not be faked. - Or IPsec could be used for the ping packets, making them - secure. + measures you like. If the Mandos client + has an SSH server, the default + configuration (as generated by + mandos-keygen with the + option) is for the Mandos server + to use an ssh-keyscan command with strict + keychecking, which can not be faked. Alternatively, IPsec + could be used for the ping packets, making them secure. === modified file 'mandos' --- mandos 2014-05-11 20:18:48 +0000 +++ mandos 2014-06-15 02:48:49 +0000 @@ -114,6 +114,7 @@ def initlogger(debug, level=logging.WARNING): """init logger and add loglevel""" + global syslogger syslogger = (logging.handlers.SysLogHandler (facility = logging.handlers.SysLogHandler.LOG_DAEMON, @@ -2336,6 +2337,9 @@ help="Directory to save/restore state in") parser.add_argument("--foreground", action="store_true", help="Run in foreground", default=None) + parser.add_argument("--no-zeroconf", action="store_false", + dest="zeroconf", help="Do not use Zeroconf", + default=None) options = parser.parse_args() @@ -2359,6 +2363,7 @@ "socket": "", "statedir": "/var/lib/mandos", "foreground": "False", + "zeroconf": "True", } # Parse config file for server-global settings @@ -2391,7 +2396,7 @@ for option in ("interface", "address", "port", "debug", "priority", "servicename", "configdir", "use_dbus", "use_ipv6", "debuglevel", "restore", - "statedir", "socket", "foreground"): + "statedir", "socket", "foreground", "zeroconf"): value = getattr(options, option) if value is not None: server_settings[option] = value @@ -2402,7 +2407,7 @@ server_settings[option] = unicode(server_settings[option]) # Force all boolean options to be boolean for option in ("debug", "use_dbus", "use_ipv6", "restore", - "foreground"): + "foreground", "zeroconf"): server_settings[option] = bool(server_settings[option]) # Debug implies foreground if server_settings["debug"]: @@ -2411,6 +2416,12 @@ ################################################################## + if (not server_settings["zeroconf"] and + not (server_settings["port"] + or server_settings["socket"] != "")): + parser.error("Needs port or socket to work without" + " Zeroconf") + # For convenience debug = server_settings["debug"] debuglevel = server_settings["debuglevel"] @@ -2419,6 +2430,7 @@ stored_state_path = os.path.join(server_settings["statedir"], stored_state_file) foreground = server_settings["foreground"] + zeroconf = server_settings["zeroconf"] if debug: initlogger(debug, logging.DEBUG) @@ -2445,6 +2457,9 @@ global mandos_dbus_service mandos_dbus_service = None + socketfd = None + if server_settings["socket"] != "": + socketfd = server_settings["socket"] tcp_server = MandosServer((server_settings["address"], server_settings["port"]), ClientHandler, @@ -2454,8 +2469,7 @@ gnutls_priority= server_settings["priority"], use_dbus=use_dbus, - socketfd=(server_settings["socket"] - or None)) + socketfd=socketfd) if not foreground: pidfilename = "/run/mandos.pid" if not os.path.isdir("/run/."): @@ -2531,14 +2545,15 @@ use_dbus = False server_settings["use_dbus"] = False tcp_server.use_dbus = False - protocol = avahi.PROTO_INET6 if use_ipv6 else avahi.PROTO_INET - service = AvahiServiceToSyslog(name = - server_settings["servicename"], - servicetype = "_mandos._tcp", - protocol = protocol, bus = bus) - if server_settings["interface"]: - service.interface = (if_nametoindex - (str(server_settings["interface"]))) + if zeroconf: + protocol = avahi.PROTO_INET6 if use_ipv6 else avahi.PROTO_INET + service = AvahiServiceToSyslog(name = + server_settings["servicename"], + servicetype = "_mandos._tcp", + protocol = protocol, bus = bus) + if server_settings["interface"]: + service.interface = (if_nametoindex + (str(server_settings["interface"]))) global multiprocessing_manager multiprocessing_manager = multiprocessing.Manager() @@ -2738,7 +2753,8 @@ def cleanup(): "Cleanup function; run on exit" - service.cleanup() + if zeroconf: + service.cleanup() multiprocessing.active_children() wnull.close() @@ -2823,7 +2839,8 @@ tcp_server.server_activate() # Find out what port we got - service.port = tcp_server.socket.getsockname()[1] + if zeroconf: + service.port = tcp_server.socket.getsockname()[1] if use_ipv6: logger.info("Now listening on address %r, port %d," " flowinfo %d, scope_id %d", @@ -2835,14 +2852,15 @@ #service.interface = tcp_server.socket.getsockname()[3] try: - # From the Avahi example code - try: - service.activate() - except dbus.exceptions.DBusException as error: - logger.critical("D-Bus Exception", exc_info=error) - cleanup() - sys.exit(1) - # End of Avahi example code + if zeroconf: + # From the Avahi example code + try: + service.activate() + except dbus.exceptions.DBusException as error: + logger.critical("D-Bus Exception", exc_info=error) + cleanup() + sys.exit(1) + # End of Avahi example code gobject.io_add_watch(tcp_server.fileno(), gobject.IO_IN, lambda *args, **kwargs: === modified file 'mandos-clients.conf.xml' --- mandos-clients.conf.xml 2013-10-20 15:25:09 +0000 +++ mandos-clients.conf.xml 2014-06-22 02:19:30 +0000 @@ -3,7 +3,7 @@ "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd" [ /etc/mandos/clients.conf"> - + %common; ]> @@ -177,7 +177,13 @@ PATH will be searched. The default value for the checker command is fping %%(host)s. + >-- %%(host)s. Note that + mandos-keygen, when generating output + to be inserted into this file, normally looks for an SSH + server on the Mandos client, and, if it find one, outputs + a option to check for the + client’s key fingerprint – this is more secure against + spoofing. In addition to normal start time expansion, this option === modified file 'mandos-keygen' --- mandos-keygen 2014-05-11 20:18:48 +0000 +++ mandos-keygen 2014-06-22 02:19:30 +0000 @@ -33,6 +33,7 @@ KEYCOMMENT="" KEYEXPIRE=0 FORCE=no +SSH=yes KEYCOMMENT_ORIG="$KEYCOMMENT" mode=keygen @@ -41,8 +42,8 @@ fi # Parse options -TEMP=`getopt --options vhpF:d:t:l:s:L:n:e:c:x:f \ - --longoptions version,help,password,passfile:,dir:,type:,length:,subtype:,sublength:,name:,email:,comment:,expire:,force \ +TEMP=`getopt --options vhpF:d:t:l:s:L:n:e:c:x:fS \ + --longoptions version,help,password,passfile:,dir:,type:,length:,subtype:,sublength:,name:,email:,comment:,expire:,force,no-ssh \ --name "$0" -- "$@"` help(){ @@ -85,6 +86,7 @@ Encrypt a password from FILE using the key in the key directory. All options other than --dir and --name are ignored. + -S, --no-ssh Don't get SSH key or set "checker" option. EOF } @@ -103,6 +105,7 @@ -c|--comment) KEYCOMMENT="$2"; shift 2;; -x|--expire) KEYEXPIRE="$2"; shift 2;; -f|--force) FORCE=yes; shift;; + -S|--no-ssh) SSH=no; shift;; -v|--version) echo "$0 $VERSION"; exit;; -h|--help) help; exit;; --) shift; break;; @@ -188,7 +191,7 @@ trap " set +e; \ test -n \"$SECFILE\" && shred --remove \"$SECFILE\"; \ -shred --remove \"$RINGDIR\"/sec*; +shred --remove \"$RINGDIR\"/sec* 2>/dev/null; test -n \"$BATCHFILE\" && rm --force \"$BATCHFILE\"; \ rm --recursive --force \"$RINGDIR\"; tty --quiet && stty echo; \ @@ -274,6 +277,23 @@ fi if [ "$mode" = password ]; then + + # Make SSH be 0 or 1 + case "$SSH" in + [Yy][Ee][Ss]|[Tt][Rr][Uu][Ee]) SSH=1;; + [Nn][Oo]|[Ff][Aa][Ll][Ss][Ee]|*) SSH=0;; + esac + + if [ $SSH -eq 1 ]; then + set +e + ssh_fingerprint="`ssh-keyscan localhost 2>/dev/null`" + if [ $? -ne 0 ]; then + ssh_fingerprint="" + fi + set -e + ssh_fingerprint="${ssh_fingerprint#localhost }" + fi + # Import key into temporary key rings gpg --quiet --batch --no-tty --no-options --enable-dsa2 \ --homedir "$RINGDIR" --trust-model always --armor \ @@ -342,6 +362,10 @@ /^[^-]/s/^/ /p } }' < "$SECFILE" + if [ -n "$ssh_fingerprint" ]; then + echo 'checker = ssh-keyscan %%(host)s 2>/dev/null | grep --fixed-strings --line-regexp --quiet --regexp="%%(host)s %(ssh_fingerprint)s"' + echo "ssh_fingerprint = ${ssh_fingerprint}" + fi fi trap - EXIT @@ -352,5 +376,5 @@ shred --remove "$SECFILE" fi # Remove the key rings -shred --remove "$RINGDIR"/sec* +shred --remove "$RINGDIR"/sec* 2>/dev/null rm --recursive --force "$RINGDIR" === modified file 'mandos-keygen.xml' --- mandos-keygen.xml 2013-10-22 19:24:01 +0000 +++ mandos-keygen.xml 2014-06-22 02:19:30 +0000 @@ -2,7 +2,7 @@ - + %common; ]> @@ -119,7 +119,10 @@ TIME - + + + + &COMMANDNAME; @@ -145,6 +148,10 @@ + + + + &COMMANDNAME; @@ -346,6 +353,22 @@ + + + + + + When or + is given, this option will + prevent &COMMANDNAME; from calling + ssh-keyscan to get an SSH fingerprint + for this host and, if successful, output suitable config + options to use this fingerprint as a + option in the output. This is + otherwise the default behavior. + + + @@ -502,7 +525,9 @@ mandos 8, mandos-client - 8mandos + 8mandos, + ssh-keyscan + 1 === modified file 'mandos-options.xml' --- mandos-options.xml 2013-10-24 20:21:45 +0000 +++ mandos-options.xml 2014-06-15 02:48:49 +0000 @@ -123,4 +123,11 @@ implies this option. + + This option controls whether the server will announce its + existence using Zeroconf. Default is to use Zeroconf. If + Zeroconf is not used, a number or a + is required. + + === modified file 'mandos.conf' --- mandos.conf 2013-10-22 19:24:01 +0000 +++ mandos.conf 2014-06-15 02:48:49 +0000 @@ -45,3 +45,9 @@ # Whether to run in the foreground ;foreground = False + +# File descriptor number to use for network socket +;socket = + +# Whether to use ZeroConf; if false, requires port or socket +;zeroconf = True === modified file 'mandos.xml' --- mandos.xml 2013-10-26 19:05:21 +0000 +++ mandos.xml 2014-06-15 02:48:49 +0000 @@ -2,7 +2,7 @@ - + %common; ]> @@ -106,6 +106,8 @@ FD + + &COMMANDNAME; @@ -323,6 +325,13 @@ + + + + + + + === modified file 'plugin-runner.c' --- plugin-runner.c 2014-03-29 02:38:15 +0000 +++ plugin-runner.c 2014-06-14 23:43:07 +0000 @@ -23,18 +23,17 @@ */ #define _GNU_SOURCE /* TEMP_FAILURE_RETRY(), getline(), - O_CLOEXEC */ + O_CLOEXEC, pipe2() */ #include /* size_t, NULL */ #include /* malloc(), exit(), EXIT_SUCCESS, realloc() */ #include /* bool, true, false */ #include /* fileno(), fprintf(), stderr, STDOUT_FILENO, fclose() */ -#include /* DIR, fdopendir(), fstat(), struct - stat, waitpid(), WIFEXITED(), - WEXITSTATUS(), wait(), pid_t, - uid_t, gid_t, getuid(), getgid(), - dirfd() */ +#include /* fstat(), struct stat, waitpid(), + WIFEXITED(), WEXITSTATUS(), wait(), + pid_t, uid_t, gid_t, getuid(), + getgid() */ #include /* fd_set, select(), FD_ZERO(), FD_SET(), FD_ISSET(), FD_CLR */ #include /* wait(), waitpid(), WIFEXITED(), @@ -42,17 +41,17 @@ WCOREDUMP() */ #include /* struct stat, fstat(), S_ISREG() */ #include /* and, or, not */ -#include /* DIR, struct dirent, fdopendir(), - readdir(), closedir(), dirfd() */ +#include /* struct dirent, scandirat() */ #include /* fcntl(), F_GETFD, F_SETFD, FD_CLOEXEC, write(), STDOUT_FILENO, struct stat, fstat(), close(), setgid(), setuid(), S_ISREG(), - faccessat() pipe(), fork(), + faccessat() pipe2(), fork(), _exit(), dup2(), fexecve(), read() */ #include /* fcntl(), F_GETFD, F_SETFD, - FD_CLOEXEC, openat() */ + FD_CLOEXEC, openat(), scandirat(), + pipe2() */ #include /* strsep, strlen(), strsignal(), strcmp(), strncmp() */ #include /* errno */ @@ -241,6 +240,7 @@ return add_to_char_array(def, &(p->environ), &(p->envc)); } +#ifndef O_CLOEXEC /* * Based on the example in the GNU LibC manual chapter 13.13 "File * Descriptor Flags". @@ -257,6 +257,7 @@ return (int)TEMP_FAILURE_RETRY(fcntl(fd, F_SETFD, ret | FD_CLOEXEC)); } +#endif /* not O_CLOEXEC */ /* Mark processes as completed when they exit, and save their exit @@ -348,8 +349,7 @@ char *plugindir = NULL; char *argfile = NULL; FILE *conffp; - DIR *dir = NULL; - struct dirent *dirst; + struct dirent **direntries = NULL; struct stat st; fd_set rfds_all; int ret, maxfd = 0; @@ -363,7 +363,7 @@ .sa_flags = SA_NOCLDSTOP }; char **custom_argv = NULL; int custom_argc = 0; - int dir_fd; + int dir_fd = -1; /* Establish a signal handler */ sigemptyset(&sigchld_action.sa_mask); @@ -829,90 +829,66 @@ ret = set_cloexec_flag(dir_fd); if(ret < 0){ error(0, errno, "set_cloexec_flag"); - TEMP_FAILURE_RETRY(close(dir_fd)); exitstatus = EX_OSERR; goto fallback; } #endif /* O_CLOEXEC */ - - dir = fdopendir(dir_fd); - if(dir == NULL){ - error(0, errno, "Could not open plugin dir"); - TEMP_FAILURE_RETRY(close(dir_fd)); - exitstatus = EX_OSERR; - goto fallback; + } + + int good_name(const struct dirent * const dirent){ + const char * const patterns[] = { ".*", "#*#", "*~", "*.dpkg-new", + "*.dpkg-old", "*.dpkg-bak", + "*.dpkg-divert", NULL }; +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-qual" +#endif + for(const char **pat = (const char **)patterns; + *pat != NULL; pat++){ +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif + if(fnmatch(*pat, dirent->d_name, FNM_FILE_NAME | FNM_PERIOD) + != FNM_NOMATCH){ + if(debug){ + fprintf(stderr, "Ignoring plugin dir entry \"%s\"" + " matching pattern %s\n", dirent->d_name, *pat); + } + return 0; + } } + return 1; + } + +#ifdef __GLIBC__ +#if __GLIBC_PREREQ(2, 15) + int numplugins = scandirat(dir_fd, ".", &direntries, good_name, + alphasort); +#else /* not __GLIBC_PREREQ(2, 15) */ + int numplugins = scandir(plugindir != NULL ? plugindir : PDIR, + &direntries, good_name, alphasort); +#endif /* not __GLIBC_PREREQ(2, 15) */ +#else /* not __GLIBC__ */ + int numplugins = scandir(plugindir != NULL ? plugindir : PDIR, + &direntries, good_name, alphasort); +#endif /* not __GLIBC__ */ + if(numplugins == -1){ + error(0, errno, "Could not scan plugin dir"); + direntries = NULL; + exitstatus = EX_OSERR; + goto fallback; } FD_ZERO(&rfds_all); /* Read and execute any executable in the plugin directory*/ - while(true){ - do { - dirst = readdir(dir); - } while(dirst == NULL and errno == EINTR); - - /* All directory entries have been processed */ - if(dirst == NULL){ - if(errno == EBADF){ - error(0, errno, "readdir"); - exitstatus = EX_IOERR; - goto fallback; - } - break; - } - - /* Ignore dotfiles, backup files and other junk */ - { - bool bad_name = false; - const char * const patterns[] = { ".*", "#*#", "*~", - "*.dpkg-new", "*.dpkg-old", - "*.dpkg-bak", "*.dpkg-divert", - NULL }; -#ifdef __GNUC__ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wcast-qual" -#endif - for(const char **pat = (const char **)patterns; - *pat != NULL; pat++){ -#ifdef __GNUC__ -#pragma GCC diagnostic pop -#endif - if(fnmatch(*pat, dirst->d_name, - FNM_FILE_NAME | FNM_PERIOD) != FNM_NOMATCH){ - if(debug){ - fprintf(stderr, "Ignoring plugin dir entry \"%s\"" - " matching pattern %s\n", dirst->d_name, *pat); - } - bad_name = true; - break; - } - } - if(bad_name){ - continue; - } - } - - int plugin_fd = openat(dir_fd, dirst->d_name, O_RDONLY | -#ifdef O_CLOEXEC - O_CLOEXEC -#else /* not O_CLOEXEC */ - 0 -#endif /* not O_CLOEXEC */ - ); + for(int i = 0; i < numplugins; i++){ + + int plugin_fd = openat(dir_fd, direntries[i]->d_name, O_RDONLY); if(plugin_fd == -1){ error(0, errno, "Could not open plugin"); continue; } -#ifndef O_CLOEXEC - /* Set the FD_CLOEXEC flag on the plugin FD */ - ret = set_cloexec_flag(plugin_fd); - if(ret < 0){ - error(0, errno, "set_cloexec_flag"); - TEMP_FAILURE_RETRY(close(plugin_fd)); - continue; - } -#endif /* O_CLOEXEC */ ret = (int)TEMP_FAILURE_RETRY(fstat(plugin_fd, &st)); if(ret == -1){ error(0, errno, "stat"); @@ -922,18 +898,19 @@ /* Ignore non-executable files */ if(not S_ISREG(st.st_mode) - or (TEMP_FAILURE_RETRY(faccessat(dir_fd, dirst->d_name, X_OK, - 0)) != 0)){ + or (TEMP_FAILURE_RETRY(faccessat(dir_fd, direntries[i]->d_name, + X_OK, 0)) != 0)){ if(debug){ fprintf(stderr, "Ignoring plugin dir entry \"%s/%s\"" " with bad type or mode\n", - plugindir != NULL ? plugindir : PDIR, dirst->d_name); + plugindir != NULL ? plugindir : PDIR, + direntries[i]->d_name); } TEMP_FAILURE_RETRY(close(plugin_fd)); continue; } - plugin *p = getplugin(dirst->d_name); + plugin *p = getplugin(direntries[i]->d_name); if(p == NULL){ error(0, errno, "getplugin"); TEMP_FAILURE_RETRY(close(plugin_fd)); @@ -942,7 +919,7 @@ if(p->disabled){ if(debug){ fprintf(stderr, "Ignoring disabled plugin \"%s\"\n", - dirst->d_name); + direntries[i]->d_name); } TEMP_FAILURE_RETRY(close(plugin_fd)); continue; @@ -975,25 +952,43 @@ } int pipefd[2]; +#ifndef O_CLOEXEC ret = (int)TEMP_FAILURE_RETRY(pipe(pipefd)); +#else /* O_CLOEXEC */ + ret = (int)TEMP_FAILURE_RETRY(pipe2(pipefd, O_CLOEXEC)); +#endif /* O_CLOEXEC */ if(ret == -1){ error(0, errno, "pipe"); exitstatus = EX_OSERR; goto fallback; } + if(pipefd[0] >= FD_SETSIZE){ + fprintf(stderr, "pipe()[0] (%d) >= FD_SETSIZE (%d)", pipefd[0], + FD_SETSIZE); + TEMP_FAILURE_RETRY(close(pipefd[0])); + TEMP_FAILURE_RETRY(close(pipefd[1])); + exitstatus = EX_OSERR; + goto fallback; + } +#ifndef O_CLOEXEC /* Ask OS to automatic close the pipe on exec */ ret = set_cloexec_flag(pipefd[0]); if(ret < 0){ error(0, errno, "set_cloexec_flag"); + TEMP_FAILURE_RETRY(close(pipefd[0])); + TEMP_FAILURE_RETRY(close(pipefd[1])); exitstatus = EX_OSERR; goto fallback; } ret = set_cloexec_flag(pipefd[1]); if(ret < 0){ error(0, errno, "set_cloexec_flag"); + TEMP_FAILURE_RETRY(close(pipefd[0])); + TEMP_FAILURE_RETRY(close(pipefd[1])); exitstatus = EX_OSERR; goto fallback; } +#endif /* not O_CLOEXEC */ /* Block SIGCHLD until process is safely in process list */ ret = (int)TEMP_FAILURE_RETRY(sigprocmask(SIG_BLOCK, &sigchld_action.sa_mask, @@ -1010,6 +1005,10 @@ } while(pid == -1 and errno == EINTR); if(pid == -1){ error(0, errno, "fork"); + TEMP_FAILURE_RETRY(sigprocmask(SIG_UNBLOCK, + &sigchld_action.sa_mask, NULL)); + TEMP_FAILURE_RETRY(close(pipefd[0])); + TEMP_FAILURE_RETRY(close(pipefd[1])); exitstatus = EX_OSERR; goto fallback; } @@ -1032,15 +1031,11 @@ _exit(EX_OSERR); } - if(dirfd(dir) < 0){ - /* If dir has no file descriptor, we could not set FD_CLOEXEC - above and must now close it manually here. */ - closedir(dir); - } if(fexecve(plugin_fd, p->argv, (p->environ[0] != NULL) ? p->environ : environ) < 0){ error(0, errno, "fexecve for %s/%s", - plugindir != NULL ? plugindir : PDIR, dirst->d_name); + plugindir != NULL ? plugindir : PDIR, + direntries[i]->d_name); _exit(EX_OSERR); } /* no return */ @@ -1049,7 +1044,7 @@ TEMP_FAILURE_RETRY(close(pipefd[1])); /* Close unused write end of pipe */ TEMP_FAILURE_RETRY(close(plugin_fd)); - plugin *new_plugin = getplugin(dirst->d_name); + plugin *new_plugin = getplugin(direntries[i]->d_name); if(new_plugin == NULL){ error(0, errno, "getplugin"); ret = (int)(TEMP_FAILURE_RETRY @@ -1096,8 +1091,10 @@ } } - TEMP_FAILURE_RETRY(closedir(dir)); - dir = NULL; + free(direntries); + direntries = NULL; + TEMP_FAILURE_RETRY(close(dir_fd)); + dir_fd = -1; free_plugin(getplugin(NULL)); for(plugin *p = plugin_list; p != NULL; p = p->next){ @@ -1297,8 +1294,10 @@ free(custom_argv); } - if(dir != NULL){ - closedir(dir); + free(direntries); + + if(dir_fd != -1){ + TEMP_FAILURE_RETRY(close(dir_fd)); } /* Kill the processes */ === modified file 'plugins.d/mandos-client.c' --- plugins.d/mandos-client.c 2014-03-29 05:16:37 +0000 +++ plugins.d/mandos-client.c 2014-07-12 13:13:28 +0000 @@ -32,15 +32,15 @@ /* Needed by GPGME, specifically gpgme_data_seek() */ #ifndef _LARGEFILE_SOURCE #define _LARGEFILE_SOURCE -#endif +#endif /* not _LARGEFILE_SOURCE */ #ifndef _FILE_OFFSET_BITS #define _FILE_OFFSET_BITS 64 -#endif +#endif /* not _FILE_OFFSET_BITS */ #define _GNU_SOURCE /* TEMP_FAILURE_RETRY(), asprintf() */ #include /* fprintf(), stderr, fwrite(), - stdout, ferror(), remove() */ + stdout, ferror() */ #include /* uint16_t, uint32_t, intptr_t */ #include /* NULL, size_t, ssize_t */ #include /* free(), EXIT_SUCCESS, srand(), @@ -57,7 +57,7 @@ #include /* socket(), struct sockaddr_in6, inet_pton(), connect(), getnameinfo() */ -#include /* open() */ +#include /* open(), unlinkat() */ #include /* opendir(), struct dirent, readdir() */ #include /* PRIu16, PRIdMAX, intmax_t, @@ -73,7 +73,8 @@ */ #include /* close(), SEEK_SET, off_t, write(), getuid(), getgid(), seteuid(), - setgid(), pause(), _exit() */ + setgid(), pause(), _exit(), + unlinkat() */ #include /* inet_pton(), htons() */ #include /* not, or, and */ #include /* struct argp_option, error_t, struct @@ -141,6 +142,7 @@ static const char sys_class_net[] = "/sys/class/net"; char *connect_to = NULL; const char *hookdir = HOOKDIR; +int hookdir_fd = -1; uid_t uid = 65534; gid_t gid = 65534; @@ -232,11 +234,14 @@ .af = af }; if(new_server->ip == NULL){ perror_plus("strdup"); + free(new_server); return false; } ret = clock_gettime(CLOCK_MONOTONIC, &(new_server->last_seen)); if(ret == -1){ perror_plus("clock_gettime"); + free(new_server->ip); + free(new_server); return false; } /* Special case of first server */ @@ -1335,7 +1340,7 @@ sret = strspn(direntry->d_name, "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" "0123456789" - "_-"); + "_.-"); if((direntry->d_name)[sret] != '\0'){ /* Contains non-allowed characters */ if(debug){ @@ -1345,14 +1350,7 @@ return 0; } - char *fullname = NULL; - ret = asprintf(&fullname, "%s/%s", hookdir, direntry->d_name); - if(ret < 0){ - perror_plus("asprintf"); - return 0; - } - - ret = stat(fullname, &st); + ret = fstatat(hookdir_fd, direntry->d_name, &st, 0); if(ret == -1){ if(debug){ perror_plus("Could not stat hook"); @@ -1463,7 +1461,6 @@ error_t ret_errno = 0; if(seteuid(0) == -1){ ret_errno = errno; - perror_plus("seteuid"); } errno = old_errno; return ret_errno; @@ -1480,7 +1477,6 @@ } if(setuid(0) == -1){ ret_errno = errno; - perror_plus("seteuid"); } errno = old_errno; return ret_errno; @@ -1493,7 +1489,6 @@ error_t ret_errno = 0; if(seteuid(uid) == -1){ ret_errno = errno; - perror_plus("seteuid"); } errno = old_errno; return ret_errno; @@ -1506,7 +1501,6 @@ error_t ret_errno = 0; if(setuid(uid) == -1){ ret_errno = errno; - perror_plus("setuid"); } errno = old_errno; return ret_errno; @@ -1515,151 +1509,174 @@ __attribute__((nonnull)) void run_network_hooks(const char *mode, const char *interface, const float delay){ - struct dirent **direntries; - int numhooks = scandir(hookdir, &direntries, runnable_hook, - alphasort); + struct dirent **direntries = NULL; + if(hookdir_fd == -1){ + hookdir_fd = open(hookdir, O_RDONLY); + if(hookdir_fd == -1){ + if(errno == ENOENT){ + if(debug){ + fprintf_plus(stderr, "Network hook directory \"%s\" not" + " found\n", hookdir); + } + } else { + perror_plus("open"); + } + return; + } + } +#ifdef __GLIBC__ +#if __GLIBC_PREREQ(2, 15) + int numhooks = scandirat(hookdir_fd, ".", &direntries, + runnable_hook, alphasort); +#else /* not __GLIBC_PREREQ(2, 15) */ + int numhooks = scandir(hookdir, &direntries, runnable_hook, + alphasort); +#endif /* not __GLIBC_PREREQ(2, 15) */ +#else /* not __GLIBC__ */ + int numhooks = scandir(hookdir, &direntries, runnable_hook, + alphasort); +#endif /* not __GLIBC__ */ if(numhooks == -1){ - if(errno == ENOENT){ - if(debug){ - fprintf_plus(stderr, "Network hook directory \"%s\" not" - " found\n", hookdir); - } - } else { - perror_plus("scandir"); + perror_plus("scandir"); + return; + } + struct dirent *direntry; + int ret; + int devnull = open("/dev/null", O_RDONLY); + for(int i = 0; i < numhooks; i++){ + direntry = direntries[i]; + if(debug){ + fprintf_plus(stderr, "Running network hook \"%s\"\n", + direntry->d_name); } - } else { - struct dirent *direntry; - int ret; - int devnull = open("/dev/null", O_RDONLY); - for(int i = 0; i < numhooks; i++){ - direntry = direntries[i]; - char *fullname = NULL; - ret = asprintf(&fullname, "%s/%s", hookdir, direntry->d_name); - if(ret < 0){ + pid_t hook_pid = fork(); + if(hook_pid == 0){ + /* Child */ + /* Raise privileges */ + errno = raise_privileges_permanently(); + if(errno != 0){ + perror_plus("Failed to raise privileges"); + _exit(EX_NOPERM); + } + /* Set group */ + errno = 0; + ret = setgid(0); + if(ret == -1){ + perror_plus("setgid"); + _exit(EX_NOPERM); + } + /* Reset supplementary groups */ + errno = 0; + ret = setgroups(0, NULL); + if(ret == -1){ + perror_plus("setgroups"); + _exit(EX_NOPERM); + } + ret = dup2(devnull, STDIN_FILENO); + if(ret == -1){ + perror_plus("dup2(devnull, STDIN_FILENO)"); + _exit(EX_OSERR); + } + ret = close(devnull); + if(ret == -1){ + perror_plus("close"); + _exit(EX_OSERR); + } + ret = dup2(STDERR_FILENO, STDOUT_FILENO); + if(ret == -1){ + perror_plus("dup2(STDERR_FILENO, STDOUT_FILENO)"); + _exit(EX_OSERR); + } + ret = setenv("MANDOSNETHOOKDIR", hookdir, 1); + if(ret == -1){ + perror_plus("setenv"); + _exit(EX_OSERR); + } + ret = setenv("DEVICE", interface, 1); + if(ret == -1){ + perror_plus("setenv"); + _exit(EX_OSERR); + } + ret = setenv("VERBOSITY", debug ? "1" : "0", 1); + if(ret == -1){ + perror_plus("setenv"); + _exit(EX_OSERR); + } + ret = setenv("MODE", mode, 1); + if(ret == -1){ + perror_plus("setenv"); + _exit(EX_OSERR); + } + char *delaystring; + ret = asprintf(&delaystring, "%f", (double)delay); + if(ret == -1){ perror_plus("asprintf"); - continue; - } - if(debug){ - fprintf_plus(stderr, "Running network hook \"%s\"\n", - direntry->d_name); - } - pid_t hook_pid = fork(); - if(hook_pid == 0){ - /* Child */ - /* Raise privileges */ - if(raise_privileges_permanently() != 0){ - perror_plus("Failed to raise privileges"); - _exit(EX_NOPERM); - } - /* Set group */ - errno = 0; - ret = setgid(0); - if(ret == -1){ - perror_plus("setgid"); - _exit(EX_NOPERM); - } - /* Reset supplementary groups */ - errno = 0; - ret = setgroups(0, NULL); - if(ret == -1){ - perror_plus("setgroups"); - _exit(EX_NOPERM); - } - ret = dup2(devnull, STDIN_FILENO); - if(ret == -1){ - perror_plus("dup2(devnull, STDIN_FILENO)"); - _exit(EX_OSERR); - } - ret = close(devnull); - if(ret == -1){ - perror_plus("close"); - _exit(EX_OSERR); - } - ret = dup2(STDERR_FILENO, STDOUT_FILENO); - if(ret == -1){ - perror_plus("dup2(STDERR_FILENO, STDOUT_FILENO)"); - _exit(EX_OSERR); - } - ret = setenv("MANDOSNETHOOKDIR", hookdir, 1); - if(ret == -1){ - perror_plus("setenv"); - _exit(EX_OSERR); - } - ret = setenv("DEVICE", interface, 1); - if(ret == -1){ - perror_plus("setenv"); - _exit(EX_OSERR); - } - ret = setenv("VERBOSITY", debug ? "1" : "0", 1); - if(ret == -1){ - perror_plus("setenv"); - _exit(EX_OSERR); - } - ret = setenv("MODE", mode, 1); - if(ret == -1){ - perror_plus("setenv"); - _exit(EX_OSERR); - } - char *delaystring; - ret = asprintf(&delaystring, "%f", (double)delay); - if(ret == -1){ - perror_plus("asprintf"); - _exit(EX_OSERR); - } - ret = setenv("DELAY", delaystring, 1); - if(ret == -1){ - free(delaystring); - perror_plus("setenv"); - _exit(EX_OSERR); - } + _exit(EX_OSERR); + } + ret = setenv("DELAY", delaystring, 1); + if(ret == -1){ free(delaystring); - if(connect_to != NULL){ - ret = setenv("CONNECT", connect_to, 1); - if(ret == -1){ - perror_plus("setenv"); - _exit(EX_OSERR); - } - } - if(execl(fullname, direntry->d_name, mode, NULL) == -1){ - perror_plus("execl"); - _exit(EXIT_FAILURE); - } + perror_plus("setenv"); + _exit(EX_OSERR); + } + free(delaystring); + if(connect_to != NULL){ + ret = setenv("CONNECT", connect_to, 1); + if(ret == -1){ + perror_plus("setenv"); + _exit(EX_OSERR); + } + } + int hook_fd = openat(hookdir_fd, direntry->d_name, O_RDONLY); + if(hook_fd == -1){ + perror_plus("openat"); + _exit(EXIT_FAILURE); + } + if((int)TEMP_FAILURE_RETRY(close(hookdir_fd)) == -1){ + perror_plus("close"); + _exit(EXIT_FAILURE); + } + if(fexecve(hook_fd, (char *const []){ direntry->d_name, NULL }, + environ) == -1){ + perror_plus("fexecve"); + _exit(EXIT_FAILURE); + } + } else { + int status; + if(TEMP_FAILURE_RETRY(waitpid(hook_pid, &status, 0)) == -1){ + perror_plus("waitpid"); + continue; + } + if(WIFEXITED(status)){ + if(WEXITSTATUS(status) != 0){ + fprintf_plus(stderr, "Warning: network hook \"%s\" exited" + " with status %d\n", direntry->d_name, + WEXITSTATUS(status)); + continue; + } + } else if(WIFSIGNALED(status)){ + fprintf_plus(stderr, "Warning: network hook \"%s\" died by" + " signal %d\n", direntry->d_name, + WTERMSIG(status)); + continue; } else { - int status; - if(TEMP_FAILURE_RETRY(waitpid(hook_pid, &status, 0)) == -1){ - perror_plus("waitpid"); - free(fullname); - continue; - } - if(WIFEXITED(status)){ - if(WEXITSTATUS(status) != 0){ - fprintf_plus(stderr, "Warning: network hook \"%s\" exited" - " with status %d\n", direntry->d_name, - WEXITSTATUS(status)); - free(fullname); - continue; - } - } else if(WIFSIGNALED(status)){ - fprintf_plus(stderr, "Warning: network hook \"%s\" died by" - " signal %d\n", direntry->d_name, - WTERMSIG(status)); - free(fullname); - continue; - } else { - fprintf_plus(stderr, "Warning: network hook \"%s\"" - " crashed\n", direntry->d_name); - free(fullname); - continue; - } - } - free(fullname); - if(debug){ - fprintf_plus(stderr, "Network hook \"%s\" ran successfully\n", - direntry->d_name); - } - } - close(devnull); - } + fprintf_plus(stderr, "Warning: network hook \"%s\"" + " crashed\n", direntry->d_name); + continue; + } + } + if(debug){ + fprintf_plus(stderr, "Network hook \"%s\" ran successfully\n", + direntry->d_name); + } + } + free(direntries); + if((int)TEMP_FAILURE_RETRY(close(hookdir_fd)) == -1){ + perror_plus("close"); + } else { + hookdir_fd = -1; + } + close(devnull); } __attribute__((nonnull, warn_unused_result)) @@ -1716,6 +1733,7 @@ /* Raise privileges */ ret_errno = raise_privileges(); if(ret_errno != 0){ + errno = ret_errno; perror_plus("Failed to raise privileges"); } @@ -1825,6 +1843,7 @@ /* Raise privileges */ ret_errno = raise_privileges(); if(ret_errno != 0){ + errno = ret_errno; perror_plus("Failed to raise privileges"); } @@ -2237,7 +2256,7 @@ /* If no interfaces were specified, make a list */ if(mc.interfaces == NULL){ - struct dirent **direntries; + struct dirent **direntries = NULL; /* Look for any good interfaces */ ret = scandir(sys_class_net, &direntries, good_interface, alphasort); @@ -2258,7 +2277,9 @@ } free(direntries); } else { - free(direntries); + if(ret == 0){ + free(direntries); + } fprintf_plus(stderr, "Could not find a network interface\n"); exitcode = EXIT_FAILURE; goto end; @@ -2486,7 +2507,7 @@ if(debug){ fprintf_plus(stderr, "Starting Avahi loop search\n"); } - + ret = avahi_loop_with_timeout(simple_poll, (int)(retry_interval * 1000), &mc); if(debug){ @@ -2537,6 +2558,7 @@ { ret_errno = raise_privileges(); if(ret_errno != 0){ + errno = ret_errno; perror_plus("Failed to raise privileges"); } else { @@ -2565,6 +2587,7 @@ ret_errno = lower_privileges_permanently(); if(ret_errno != 0){ + errno = ret_errno; perror_plus("Failed to lower privileges permanently"); } } @@ -2575,36 +2598,44 @@ /* Removes the GPGME temp directory and all files inside */ if(tempdir != NULL){ struct dirent **direntries = NULL; - struct dirent *direntry = NULL; - int numentries = scandir(tempdir, &direntries, notdotentries, - alphasort); - if(numentries > 0){ - for(int i = 0; i < numentries; i++){ - direntry = direntries[i]; - char *fullname = NULL; - ret = asprintf(&fullname, "%s/%s", tempdir, - direntry->d_name); - if(ret < 0){ - perror_plus("asprintf"); - continue; - } - ret = remove(fullname); - if(ret == -1){ - fprintf_plus(stderr, "remove(\"%s\"): %s\n", fullname, - strerror(errno)); - } - free(fullname); + int tempdir_fd = (int)TEMP_FAILURE_RETRY(open(tempdir, O_RDONLY | + O_NOFOLLOW)); + if(tempdir_fd == -1){ + perror_plus("open"); + } else { +#ifdef __GLIBC__ +#if __GLIBC_PREREQ(2, 15) + int numentries = scandirat(tempdir_fd, ".", &direntries, + notdotentries, alphasort); +#else /* not __GLIBC_PREREQ(2, 15) */ + int numentries = scandir(tempdir, &direntries, notdotentries, + alphasort); +#endif /* not __GLIBC_PREREQ(2, 15) */ +#else /* not __GLIBC__ */ + int numentries = scandir(tempdir, &direntries, notdotentries, + alphasort); +#endif /* not __GLIBC__ */ + if(numentries >= 0){ + for(int i = 0; i < numentries; i++){ + ret = unlinkat(tempdir_fd, direntries[i]->d_name, 0); + if(ret == -1){ + fprintf_plus(stderr, "unlinkat(open(\"%s\", O_RDONLY)," + " \"%s\", 0): %s\n", tempdir, + direntries[i]->d_name, strerror(errno)); + } + } + + /* need to clean even if 0 because man page doesn't specify */ + free(direntries); + if(numentries == -1){ + perror_plus("scandir"); + } + ret = rmdir(tempdir); + if(ret == -1 and errno != ENOENT){ + perror_plus("rmdir"); + } } - } - - /* need to clean even if 0 because man page doesn't specify */ - free(direntries); - if(numentries == -1){ - perror_plus("scandir"); - } - ret = rmdir(tempdir); - if(ret == -1 and errno != ENOENT){ - perror_plus("rmdir"); + TEMP_FAILURE_RETRY(close(tempdir_fd)); } } === modified file 'plugins.d/mandos-client.xml' --- plugins.d/mandos-client.xml 2014-03-06 02:26:04 +0000 +++ plugins.d/mandos-client.xml 2014-06-22 02:19:30 +0000 @@ -2,7 +2,7 @@ - + %common; ]> @@ -748,8 +748,9 @@ It will also help if the checker program on the server is configured to request something from the client which can not be - spoofed by someone else on the network, unlike unencrypted - ICMP echo (ping) replies. + spoofed by someone else on the network, like SSH server key + fingerprints, and unlike unencrypted ICMP + echo (ping) replies. Note: This makes it completely insecure to