From 78841c94876af992c50d00fdefe244b6d7479f93 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Thu, 28 May 2009 00:00:00 +0200 Subject: [PATCH] Updated to v2.1.1 --- Changelog | 21 +++++++++++++++++++++ ftpdataio.c | 2 ++ ftppolicy.c | 12 ++++++++---- main.c | 2 +- oneprocess.c | 6 ++++-- parseconf.c | 1 + prelogin.c | 1 + privsock.c | 4 ++-- readwrite.c | 8 +++++++- secutil.c | 4 ++++ secutil.h | 2 ++ sslslave.c | 3 ++- standalone.c | 9 ++++++++- sysdeputil.c | 43 +++++++++++++++++++++++++++++++++++++++++++ sysdeputil.h | 4 ++++ sysutil.c | 29 +++++++++++++++++++++++++---- sysutil.h | 2 ++ tunables.c | 2 ++ tunables.h | 1 + twoprocess.c | 12 ++++++++---- vsf_findlibs.sh | 2 ++ vsftpd.conf.5 | 7 +++++-- 22 files changed, 155 insertions(+), 22 deletions(-) diff --git a/Changelog b/Changelog index 7cdec1d..97bfd19 100644 --- a/Changelog +++ b/Changelog @@ -1105,3 +1105,24 @@ unprivileged child. receives data transfer fd's via recvmsg(). It's a total shame because doing the SSL handshake under even lower privs would be a real boost. (v2.1.1pre1 here) +- Fix some declarations occuring in the middle of a block; broke older more +strict compilers. +- Handle the case where libcap is now libcap.so.2; fixes build on my new +Ubuntu 9.04. +- Enhance 522 error message to point to require_ssl_reuse option. +- Fix NASTY regression whereby data transfer timeouts would fire incorrectly +under SSL transfers. In addition, the transfer rate caps were not working +under SSL transfers. Reported by several people. +- Use the login delay machinery for userlist-based denials too. Thanks to +Tomas Hoger for the patch. +- Fix another tedious regression whereby absent per-user config files were +causing a session fail rather than being gracefully ignored. +- Use the somewhat new CLONE_NEWPID / CLONE_NEWIPC to provide more isolation +in the vsftpd low-priv processes (CLONE_NEWNET pending). +- Use RLIMIT_NPROC to disallow fork()ing etc. in processes that do not need +to create new ones. +- Add "isolate" config flag to disable the new weird clone() flags if +necessary. + +At this point: v2.1.1 released! +=============================== diff --git a/ftpdataio.c b/ftpdataio.c index 482399a..1177680 100644 --- a/ftpdataio.c +++ b/ftpdataio.c @@ -199,6 +199,8 @@ vsf_ftpdataio_post_mark_connect(struct vsf_session* p_sess) if (tunable_require_ssl_reuse) { str_append_text(&s_err_msg, "; session reuse required"); + str_append_text( + &s_err_msg, ": see require_ssl_reuse option in vsftpd.conf man page"); } vsf_cmdio_write_str(p_sess, FTP_DATATLSBAD, &s_err_msg); } diff --git a/ftppolicy.c b/ftppolicy.c index 32fd4be..db282c9 100644 --- a/ftppolicy.c +++ b/ftppolicy.c @@ -162,6 +162,7 @@ policy_setup(struct pt_sandbox* p_sandbox, const struct vsf_session* p_sess) static int socket_validator(struct pt_sandbox* p_sandbox, void* p_arg) { + int ret; struct vsf_session* p_sess = (struct vsf_session*) p_arg; unsigned long arg1; unsigned long arg2; @@ -170,7 +171,7 @@ socket_validator(struct pt_sandbox* p_sandbox, void* p_arg) { expected_family = AF_INET6; } - int ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 0, &arg1); + ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 0, &arg1); if (ret != 0) { return ret; @@ -190,6 +191,7 @@ socket_validator(struct pt_sandbox* p_sandbox, void* p_arg) static int connect_validator(struct pt_sandbox* p_sandbox, void* p_arg) { + int ret; struct vsf_session* p_sess = (struct vsf_session*) p_arg; unsigned long arg2; unsigned long arg3; @@ -203,7 +205,7 @@ connect_validator(struct pt_sandbox* p_sandbox, void* p_arg) expected_family = AF_INET6; expected_len = sizeof(struct sockaddr_in6); } - int ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); + ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); if (ret != 0) { return ret; @@ -258,10 +260,11 @@ connect_validator(struct pt_sandbox* p_sandbox, void* p_arg) static int getsockopt_validator(struct pt_sandbox* p_sandbox, void* p_arg) { + int ret; unsigned long arg2; unsigned long arg3; (void) p_arg; - int ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); + ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); if (ret != 0) { return ret; @@ -281,10 +284,11 @@ getsockopt_validator(struct pt_sandbox* p_sandbox, void* p_arg) static int setsockopt_validator(struct pt_sandbox* p_sandbox, void* p_arg) { + int ret; unsigned long arg2; unsigned long arg3; (void) p_arg; - int ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); + ret = ptrace_sandbox_get_socketcall_arg(p_sandbox, 1, &arg2); if (ret != 0) { return ret; diff --git a/main.c b/main.c index 1849bf8..221093b 100644 --- a/main.c +++ b/main.c @@ -35,7 +35,6 @@ static void limits_init(void); int main(int argc, const char* argv[]) { - tunables_load_defaults(); struct vsf_session the_session = { /* Control connection */ @@ -69,6 +68,7 @@ main(int argc, const char* argv[]) }; int config_specified = 0; const char* p_config_name = VSFTP_DEFAULT_CONFIG; + tunables_load_defaults(); /* Zero or one argument supported. If one argument is passed, it is the * path to the config file */ diff --git a/oneprocess.c b/oneprocess.c index ae582cc..4e857b4 100644 --- a/oneprocess.c +++ b/oneprocess.c @@ -36,7 +36,7 @@ vsf_one_process_start(struct vsf_session* p_sess) struct pt_sandbox* p_sandbox = ptrace_sandbox_alloc(); if (p_sandbox == 0) { - die("could not allocate sandbox"); + die("could not allocate sandbox (only works for 32-bit builds)"); } policy_setup(p_sandbox, p_sess); if (ptrace_sandbox_launch_process(p_sandbox, @@ -90,7 +90,9 @@ one_process_start(void* p_arg) else { vsf_secutil_change_credentials(&user_name, 0, &chdir_str, caps, - VSF_SECUTIL_OPTION_CHROOT | VSF_SECUTIL_OPTION_USE_GROUPS); + VSF_SECUTIL_OPTION_CHROOT | + VSF_SECUTIL_OPTION_USE_GROUPS | + VSF_SECUTIL_OPTION_NO_PROCS); } str_free(&user_name); str_free(&chdir_str); diff --git a/parseconf.c b/parseconf.c index e1c6f80..5dd0379 100644 --- a/parseconf.c +++ b/parseconf.c @@ -106,6 +106,7 @@ parseconf_bool_array[] = { "implicit_ssl", &tunable_implicit_ssl }, { "sandbox", &tunable_sandbox }, { "require_ssl_reuse", &tunable_require_ssl_reuse }, + { "isolate", &tunable_isolate }, { 0, 0 } }; diff --git a/prelogin.c b/prelogin.c index b92cf85..ddbe2fe 100644 --- a/prelogin.c +++ b/prelogin.c @@ -217,6 +217,7 @@ handle_user_command(struct vsf_session* p_sess) if ((located && tunable_userlist_deny) || (!located && !tunable_userlist_deny)) { + check_login_delay(); vsf_cmdio_write(p_sess, FTP_LOGINERR, "Permission denied."); check_login_fails(p_sess); str_empty(&p_sess->user_str); diff --git a/privsock.c b/privsock.c index 0e9b2b3..17a2cec 100644 --- a/privsock.c +++ b/privsock.c @@ -23,6 +23,7 @@ void priv_sock_init(struct vsf_session* p_sess) { + struct vsf_sysutil_socketpair_retval retval; if (p_sess->parent_fd != -1) { bug("parent_fd active"); @@ -31,8 +32,7 @@ priv_sock_init(struct vsf_session* p_sess) { bug("child_fd active"); } - const struct vsf_sysutil_socketpair_retval retval = - vsf_sysutil_unix_stream_socketpair(); + retval = vsf_sysutil_unix_stream_socketpair(); p_sess->parent_fd = retval.socket_one; p_sess->child_fd = retval.socket_two; } diff --git a/readwrite.c b/readwrite.c index a0062c5..bf44565 100644 --- a/readwrite.c +++ b/readwrite.c @@ -85,6 +85,8 @@ ftp_read_data(struct vsf_session* p_sess, char* p_buf, unsigned int len) priv_sock_send_cmd(p_sess->ssl_consumer_fd, PRIV_SOCK_DO_SSL_READ); ret = priv_sock_get_int(p_sess->ssl_consumer_fd); priv_sock_recv_buf(p_sess->ssl_consumer_fd, p_buf, len); + // Need to do this here too because it is useless in the slave process. + vsf_sysutil_check_pending_actions(kVSFSysUtilIO, ret, p_sess->data_fd); return ret; } else if (p_sess->data_use_ssl) @@ -103,9 +105,13 @@ ftp_write_data(const struct vsf_session* p_sess, const char* p_buf, { if (p_sess->data_use_ssl && p_sess->ssl_slave_active) { + int ret; priv_sock_send_cmd(p_sess->ssl_consumer_fd, PRIV_SOCK_DO_SSL_WRITE); priv_sock_send_buf(p_sess->ssl_consumer_fd, p_buf, len); - return priv_sock_get_int(p_sess->ssl_consumer_fd); + ret = priv_sock_get_int(p_sess->ssl_consumer_fd); + // Need to do this here too because it is useless in the slave process. + vsf_sysutil_check_pending_actions(kVSFSysUtilIO, ret, p_sess->data_fd); + return ret; } else if (p_sess->data_use_ssl) { diff --git a/secutil.c b/secutil.c index 70ca337..1cd9d58 100644 --- a/secutil.c +++ b/secutil.c @@ -125,5 +125,9 @@ vsf_secutil_change_credentials(const struct mystr* p_user_str, { vsf_sysdep_adopt_capabilities(caps); } + if (options & VSF_SECUTIL_OPTION_NO_PROCS) + { + vsf_sysutil_set_no_procs(); + } } diff --git a/secutil.h b/secutil.h index 6f06d00..8f08a94 100644 --- a/secutil.h +++ b/secutil.h @@ -30,6 +30,8 @@ struct mystr; #define VSF_SECUTIL_OPTION_CHANGE_EUID 4 /* Use RLIMIT_NOFILE to prevent the opening of new fds */ #define VSF_SECUTIL_OPTION_NO_FDS 8 +/* Use RLIMIT_NPROC to prevent the launching of new processes */ +#define VSF_SECUTIL_OPTION_NO_PROCS 16 void vsf_secutil_change_credentials(const struct mystr* p_user_str, const struct mystr* p_dir_str, diff --git a/sslslave.c b/sslslave.c index af74357..916f9c8 100644 --- a/sslslave.c +++ b/sslslave.c @@ -67,8 +67,9 @@ ssl_slave(struct vsf_session* p_sess) } else if (cmd == PRIV_SOCK_DO_SSL_READ) { + int ret; str_trunc(&data_str, VSFTP_DATA_BUFSIZE); - int ret = ssl_read_into_str(p_sess, p_sess->p_data_ssl, &data_str); + ret = ssl_read_into_str(p_sess, p_sess->p_data_ssl, &data_str); priv_sock_send_int(p_sess->ssl_slave_fd, ret); priv_sock_send_str(p_sess->ssl_slave_fd, &data_str); } diff --git a/standalone.c b/standalone.c index e295cdc..d62cf23 100644 --- a/standalone.c +++ b/standalone.c @@ -151,7 +151,14 @@ vsf_standalone_main(void) child_info.num_this_ip = 0; p_raw_addr = vsf_sysutil_sockaddr_get_raw_addr(p_accept_addr); child_info.num_this_ip = handle_ip_count(p_raw_addr); - new_child = vsf_sysutil_fork_failok(); + if (tunable_isolate) + { + new_child = vsf_sysutil_fork_isolate_failok(); + } + else + { + new_child = vsf_sysutil_fork_failok(); + } if (new_child != 0) { /* Parent context */ diff --git a/sysdeputil.c b/sysdeputil.c index 04a5cf8..ec03329 100644 --- a/sysdeputil.c +++ b/sysdeputil.c @@ -51,6 +51,7 @@ #undef VSF_SYSDEP_HAVE_HPUX_SETPROCTITLE #undef VSF_SYSDEP_HAVE_MAP_ANON #undef VSF_SYSDEP_NEED_OLD_FD_PASSING +#undef VSF_SYSDEP_HAVE_LINUX_CLONE #ifdef VSF_BUILD_PAM #define VSF_SYSDEP_HAVE_PAM #endif @@ -63,6 +64,17 @@ #include /* BEGIN config */ +#if defined(__linux__) + #define VSF_SYSDEP_HAVE_LINUX_CLONE + #include + #ifndef CLONE_NEWPID + #define CLONE_NEWPID 0x20000000 + #endif + #ifndef CLONE_NEWIPC + #define CLONE_NEWIPC 0x08000000 + #endif +#endif + #if defined(__linux__) && !defined(__ia64__) && !defined(__s390__) #define VSF_SYSDEP_TRY_LINUX_SETPROCTITLE_HACK #include @@ -1220,3 +1232,34 @@ vsf_set_term_if_parent_dies() } #endif } + +int +vsf_sysutil_fork_isolate_failok() +{ +#ifdef VSF_SYSDEP_HAVE_LINUX_CLONE + static int cloneflags_work = 1; + if (cloneflags_work) + { + int ret = syscall(__NR_clone, CLONE_NEWPID | CLONE_NEWIPC | SIGCHLD, NULL); + if (ret != -1 || errno != EINVAL) + { + vsf_sysutil_clear_pid_cache(); + return ret; + } + cloneflags_work = 0; + } +#endif + return vsf_sysutil_fork_failok(); +} + +int +vsf_sysutil_getpid_nocache(void) +{ +#ifdef VSF_SYSDEP_HAVE_LINUX_CLONE + // Need to defeat the glibc pid caching because we need to hit a raw + // sys_clone() above. + return syscall(__NR_getpid); +#else + return getpid(); +#endif +} diff --git a/sysdeputil.h b/sysdeputil.h index a09425d..69c31ed 100644 --- a/sysdeputil.h +++ b/sysdeputil.h @@ -60,5 +60,9 @@ void vsf_set_die_if_parent_dies(); /* Or a softer version delivering SIGTERM. */ void vsf_set_term_if_parent_dies(); +/* If supported, the ability to fork into different secure namespaces. */ +int vsf_sysutil_fork_isolate_failok(); +int vsf_sysutil_getpid_nocache(); + #endif /* VSF_SYSDEPUTIL_H */ diff --git a/sysutil.c b/sysutil.c index 3a8376e..bada832 100644 --- a/sysutil.c +++ b/sysutil.c @@ -15,6 +15,7 @@ #include "sysutil.h" #include "utility.h" #include "tunables.h" +#include "sysdeputil.h" /* Activate 64-bit file support on Linux/32bit plus others */ #define _FILE_OFFSET_BITS 64 @@ -533,7 +534,7 @@ vsf_sysutil_getpid(void) { if (s_current_pid == -1) { - s_current_pid = getpid(); + s_current_pid = vsf_sysutil_getpid_nocache(); } return (unsigned int) s_current_pid; } @@ -563,7 +564,7 @@ vsf_sysutil_fork_failok(void) int retval = fork(); if (retval == 0) { - s_current_pid = -1; + vsf_sysutil_clear_pid_cache(); } return retval; } @@ -687,7 +688,7 @@ vsf_sysutil_set_nodelay(int fd) void vsf_sysutil_activate_sigurg(int fd) { - int retval = fcntl(fd, F_SETOWN, getpid()); + int retval = fcntl(fd, F_SETOWN, vsf_sysutil_getpid()); if (retval != 0) { die("fcntl"); @@ -2483,7 +2484,7 @@ vsf_sysutil_make_session_leader(void) /* This makes us the leader if we are not already */ (void) setsid(); /* Check we're the leader */ - if (getpid() != getpgrp()) + if ((int) vsf_sysutil_getpid() != getpgrp()) { die("not session leader"); } @@ -2774,3 +2775,23 @@ vsf_sysutil_set_no_fds() die("setrlimit NOFILE"); } } + +void +vsf_sysutil_set_no_procs() +{ + int ret; + struct rlimit rlim; + rlim.rlim_cur = 0; + rlim.rlim_max = 0; + ret = setrlimit(RLIMIT_NPROC, &rlim); + if (ret != 0) + { + die("setrlimit NPROC"); + } +} + +void +vsf_sysutil_clear_pid_cache() +{ + s_current_pid = -1; +} diff --git a/sysutil.h b/sysutil.h index cf86506..e4ad452 100644 --- a/sysutil.h +++ b/sysutil.h @@ -164,6 +164,7 @@ void vsf_sysutil_free(void* p_ptr); /* Process creation/exit/process handling */ unsigned int vsf_sysutil_getpid(void); +void vsf_sysutil_clear_pid_cache(void); int vsf_sysutil_fork(void); int vsf_sysutil_fork_failok(void); void vsf_sysutil_exit(int exit_code); @@ -340,6 +341,7 @@ int vsf_sysutil_setmodtime(const char* p_file, long the_time, int is_localtime); /* Limits */ void vsf_sysutil_set_address_space_limit(long bytes); void vsf_sysutil_set_no_fds(void); +void vsf_sysutil_set_no_procs(void); #endif /* VSF_SYSUTIL_H */ diff --git a/tunables.c b/tunables.c index fa716b2..fdd7986 100644 --- a/tunables.c +++ b/tunables.c @@ -82,6 +82,7 @@ int tunable_delete_failed_uploads; int tunable_implicit_ssl; int tunable_sandbox; int tunable_require_ssl_reuse; +int tunable_isolate; unsigned int tunable_accept_timeout; unsigned int tunable_connect_timeout; @@ -216,6 +217,7 @@ tunables_load_defaults() tunable_implicit_ssl = 0; tunable_sandbox = 0; tunable_require_ssl_reuse = 1; + tunable_isolate = 1; tunable_accept_timeout = 60; tunable_connect_timeout = 60; diff --git a/tunables.h b/tunables.h index b47f96a..b14da1a 100644 --- a/tunables.h +++ b/tunables.h @@ -83,6 +83,7 @@ extern int tunable_delete_failed_uploads; /* Delete an upload that failed */ extern int tunable_implicit_ssl; /* Use implicit SSL protocol */ extern int tunable_sandbox; /* Deploy ptrace sandbox */ extern int tunable_require_ssl_reuse; /* Require re-used data conn */ +extern int tunable_isolate; /* Use container clone() flags */ /* Integer/numeric defines */ extern unsigned int tunable_accept_timeout; diff --git a/twoprocess.c b/twoprocess.c index 23097ba..dff01ae 100644 --- a/twoprocess.c +++ b/twoprocess.c @@ -133,7 +133,7 @@ drop_all_privs(void) { struct mystr user_str = INIT_MYSTR; struct mystr dir_str = INIT_MYSTR; - int option = VSF_SECUTIL_OPTION_CHROOT; + int option = VSF_SECUTIL_OPTION_CHROOT | VSF_SECUTIL_OPTION_NO_PROCS; if (!tunable_ssl_enable) { /* Unfortunately, can only enable this if we can be sure of not using SSL. @@ -331,7 +331,8 @@ common_do_login(struct vsf_session* p_sess, const struct mystr* p_user_str, struct mystr chroot_str = INIT_MYSTR; struct mystr chdir_str = INIT_MYSTR; struct mystr userdir_str = INIT_MYSTR; - unsigned int secutil_option = VSF_SECUTIL_OPTION_USE_GROUPS; + unsigned int secutil_option = VSF_SECUTIL_OPTION_USE_GROUPS | + VSF_SECUTIL_OPTION_NO_PROCS; /* Child - drop privs and start proper FTP! */ /* This PR_SET_PDEATHSIG doesn't work for all possible process tree setups. * The other cases are taken care of by a shutdown() of the command @@ -413,8 +414,11 @@ handle_per_user_config(const struct mystr* p_user_str) str_append_char(&filename_str, '/'); str_append_str(&filename_str, p_user_str); retval = str_stat(&filename_str, &p_statbuf); - /* Security - file ownership check now in vsf_parseconf_load_file() */ - vsf_parseconf_load_file(str_getbuf(&filename_str), 1); + if (!vsf_sysutil_retval_is_error(retval)) + { + /* Security - file ownership check now in vsf_parseconf_load_file() */ + vsf_parseconf_load_file(str_getbuf(&filename_str), 1); + } str_free(&filename_str); vsf_sysutil_free(p_statbuf); } diff --git a/vsf_findlibs.sh b/vsf_findlibs.sh index 31647b3..3014f00 100755 --- a/vsf_findlibs.sh +++ b/vsf_findlibs.sh @@ -45,6 +45,8 @@ locate_library /usr/lib/libsec.sl && echo "-lsec"; # Look for libcap (capabilities) if locate_library /lib/libcap.so.1; then echo "/lib/libcap.so.1"; +elif locate_library /lib/libcap.so.2; then + echo "/lib/libcap.so.2"; else locate_library /usr/lib/libcap.so && echo "-lcap"; locate_library /lib/libcap.so && echo "-lcap"; diff --git a/vsftpd.conf.5 b/vsftpd.conf.5 index 3551ae7..e2e4c1d 100644 --- a/vsftpd.conf.5 +++ b/vsftpd.conf.5 @@ -119,7 +119,7 @@ vsftpd will not check /etc/shells for a valid user shell for local logins. Default: YES .TP .B chmod_enable -When enables, allows use of the SITE CHMOD command. NOTE! This only applies +When enabled, allows use of the SITE CHMOD command. NOTE! This only applies to local users. Anonymous users never get to use SITE CHMOD. Default: YES @@ -401,7 +401,10 @@ Default: NO .B require_ssl_reuse If set to yes, all SSL data connections are required to exhibit SSL session reuse (which proves that they know the same master secret as the control -channel). (Added in v2.1.0). +channel). Although this is a secure default, it may break many FTP clients, +so you may want to disable it. For a discussion of the consequences, see +http://scarybeastsecurity.blogspot.com/2009/02/vsftpd-210-released.html +(Added in v2.1.0). Default: YES .TP