From: Arran Cudbard-Bell Date: Fri, 25 Apr 2025 15:33:49 +0000 (-0400) Subject: Nuke chroot, it's likely not used and causes clang scan to complain X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=988212cb083c11dc870fd27be42bf37c9ba6e73f;p=thirdparty%2Ffreeradius-server.git Nuke chroot, it's likely not used and causes clang scan to complain --- diff --git a/doc/antora/modules/howto/pages/protocols/radius/radsec_with_haproxy.adoc b/doc/antora/modules/howto/pages/protocols/radius/radsec_with_haproxy.adoc index 1fb3a0866f..b978cf7421 100644 --- a/doc/antora/modules/howto/pages/protocols/radius/radsec_with_haproxy.adoc +++ b/doc/antora/modules/howto/pages/protocols/radius/radsec_with_haproxy.adoc @@ -49,7 +49,7 @@ decode the RADIUS protocol. ==== The above example is a minimal configuration. In practise you will want to retain many of the HAproxy configuration items already present in the -configuration (e.g. `log`, `chroot`, `user`, `group`), but these vary across +configuration (e.g. `log`, `group`), but these vary across distributions. Other HTTP-related options that may already exist in the configuration will conflict with `mode tcp` (Layer 4 proxying) and should be removed if HAproxy complains about them. @@ -131,4 +131,3 @@ before continuing. Once proxied connections are working we are ready to xref:protocols/radius/enable_proxy_protocol.adoc[enable the PROXY Protocol] on both HAproxy and the RadSec server. - diff --git a/doc/antora/modules/reference/pages/raddb/radiusd.conf.adoc b/doc/antora/modules/reference/pages/raddb/radiusd.conf.adoc index 13384ddf0b..ab819c9102 100644 --- a/doc/antora/modules/reference/pages/raddb/radiusd.conf.adoc +++ b/doc/antora/modules/reference/pages/raddb/radiusd.conf.adoc @@ -396,37 +396,6 @@ There may be multiple methods of attacking on the server. This section holds the configuration items which minimize the impact of those attacks - -chroot: directory where the server does "chroot". - -The chroot is done very early in the process of starting -the server. After the chroot has been performed it -switches to the `user` listed below (which MUST be -specified). If `group` is specified, it switches to that -group, too. Any other groups listed for the specified -`user` in `/etc/group` are also added as part of this -process. - -The current working directory (chdir / cd) is left - *outside* of the `chroot` until all of the modules have been -initialized. This allows the `raddb` directory to be left -outside of the `chroot`. Once the modules have been -initialized, it does a `chdir` to `${logdir}`. This means -that it should be impossible to break out of the chroot. - -If you are worried about security issues related to this -use of chdir, then simply ensure that the `raddb` directory -is inside of the chroot, and be sure to do `cd raddb` -BEFORE starting the server. - -If the server is statically linked, then the only files -that have to exist in the chroot are `${run_dir}` and -`${logdir}`. If you do the `cd raddb` as discussed above, -then the `raddb` directory has to be inside of the `chroot` -directory, too. - - - user:: group:: @@ -760,7 +729,6 @@ templates { $INCLUDE template.d/ } security { -# chroot = /path/to/chroot/directory # user = radius # group = radius allow_core_dumps = no diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index b162181385..0443d9d394 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -503,37 +503,6 @@ templates { # of those attacks # security { - # - # chroot: directory where the server does "chroot". - # - # The chroot is done very early in the process of starting - # the server. After the chroot has been performed it - # switches to the `user` listed below (which MUST be - # specified). If `group` is specified, it switches to that - # group, too. Any other groups listed for the specified - # `user` in `/etc/group` are also added as part of this - # process. - # - # The current working directory (chdir / cd) is left - # *outside* of the `chroot` until all of the modules have been - # initialized. This allows the `raddb` directory to be left - # outside of the `chroot`. Once the modules have been - # initialized, it does a `chdir` to `${logdir}`. This means - # that it should be impossible to break out of the chroot. - # - # If you are worried about security issues related to this - # use of chdir, then simply ensure that the `raddb` directory - # is inside of the chroot, and be sure to do `cd raddb` - # BEFORE starting the server. - # - # If the server is statically linked, then the only files - # that have to exist in the chroot are `${run_dir}` and - # `${logdir}`. If you do the `cd raddb` as discussed above, - # then the `raddb` directory has to be inside of the `chroot` - # directory, too. - # -# chroot = /path/to/chroot/directory - # # user:: # group:: diff --git a/scripts/ci/dovecot/fr_dovecot.conf b/scripts/ci/dovecot/fr_dovecot.conf index 33a1669060..9521805bbc 100644 --- a/scripts/ci/dovecot/fr_dovecot.conf +++ b/scripts/ci/dovecot/fr_dovecot.conf @@ -14,11 +14,9 @@ service anvil { unix_listener anvil-auth-penalty { mode = 0 } - chroot = } service stats { - chroot = } # ALL CODE BELOW THIS IS ADDED BY THE SHELL SCRIPT scripts/ci/imap-setup.sh diff --git a/scripts/ci/imap-setup.sh b/scripts/ci/imap-setup.sh index b3f39dcc38..f94e117ad6 100755 --- a/scripts/ci/imap-setup.sh +++ b/scripts/ci/imap-setup.sh @@ -14,13 +14,13 @@ BUILDDIR="${BASEDIR}/build/ci/dovecot" CERTDIR="${BASEDIR}/raddb/certs/rsa" ETCDIR="${BUILDDIR}/etc" -# Directories for running dovecot +# Directories for running dovecot RUNDIR="${BUILDDIR}/run" TLSRUNDIR="${BUILDDIR}/tls_run" MAILDIR="${ETCDIR}/dovecot_mail" LOGDIR="${BUILDDIR}/log" -# Important files for running dovecot +# Important files for running dovecot CONF="${ETCDIR}/dovecot.conf" TLSCONF="${ETCDIR}/tls_dovecot.conf" PASSPATH="${ETCDIR}/dovecot.passwd" @@ -51,7 +51,7 @@ touch "${PASSPATH}" # Make sure there are log files touch "${LOGPATH}" -touch "${LOGINFOPATH}" +touch "${LOGINFOPATH}" # Get primary group name if [ -z "${USER}" ]; then @@ -72,7 +72,7 @@ openssl rsa -in "${BASEDIR}/raddb/certs/rsa/server.key" -passin 'pass:whatever' rm -f ${PASSPATH} for i in {1..3}; do PASS=$(doveadm -o stats_writer_socket_path= pw -p test${i} -s CRYPT) - echo "user${i}:${PASS}:::::: + echo "user${i}:${PASS}:::::: " >> "${PASSPATH}" done @@ -95,7 +95,6 @@ base_dir = ${RUNDIR} service imap-login { process_min_avail = 16 user = ${USER:-dovecot} - chroot = inet_listener imap { port = 1430 } @@ -112,7 +111,6 @@ base_dir = ${TLSRUNDIR} service imap-login { process_min_avail = 16 user = ${USER:-dovecot} - chroot = inet_listener imap { port = 1431 } diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c index 206a2265d0..afe06ee91a 100644 --- a/src/lib/server/main_config.c +++ b/src/lib/server/main_config.c @@ -267,7 +267,6 @@ static const conf_parser_t security_config[] = { { FR_CONF_OFFSET_IS_SET("user", FR_TYPE_VOID, 0, main_config_t, uid), .func = cf_parse_uid }, { FR_CONF_OFFSET_IS_SET("group", FR_TYPE_VOID, 0, main_config_t, gid), .func = cf_parse_gid }, #endif - { FR_CONF_OFFSET("chroot", main_config_t, chroot_dir) }, { FR_CONF_OFFSET("allow_core_dumps", main_config_t, allow_core_dumps), .dflt = "no" }, #ifdef ENABLE_OPENSSL_VERSION_CHECK @@ -610,8 +609,6 @@ static int mkdir_chown(int fd, char const *path, void *uctx) return ret; } /* - * Do chroot, if requested. - * * Switch UID and GID to what is specified in the config file */ static int switch_users(main_config_t *config, CONF_SECTION *cs) @@ -635,7 +632,7 @@ static int switch_users(main_config_t *config, CONF_SECTION *cs) return -1; } - DEBUG("Parsing security rules to bootstrap UID / GID / chroot / etc."); + DEBUG("Parsing security rules to bootstrap UID / GID / etc."); if (cf_section_parse(config, config, cs) < 0) { fprintf(stderr, "%s: Error: Failed to parse user/group information.\n", config->name); @@ -643,11 +640,11 @@ static int switch_users(main_config_t *config, CONF_SECTION *cs) } /* - * Don't do chroot/setuid/setgid if we're in debugging + * Don't do setuid/setgid if we're in debugging * as non-root. */ if (DEBUG_ENABLED && (getuid() != 0)) { - WARN("Ignoring configured UID / GID / chroot as we're running in debug mode"); + WARN("Ignoring configured UID / GID as we're running in debug mode"); return 0; } #ifdef HAVE_GRP_H @@ -702,33 +699,6 @@ static int switch_users(main_config_t *config, CONF_SECTION *cs) cf_file_check_user(config->server_uid ? config->server_uid : (uid_t)-1, config->server_gid ? config->server_gid : (gid_t)-1); - /* - * Do chroot BEFORE changing UIDs. - */ - if (config->chroot_dir) { - if (chroot(config->chroot_dir) < 0) { - fprintf(stderr, "%s: Failed to perform chroot %s: %s", - config->name, config->chroot_dir, fr_syserror(errno)); - return -1; - } - - /* - * Note that we leave chdir alone. It may be - * OUTSIDE of the root. This allows us to read - * the configuration from "-d ./etc/raddb", with - * the chroot as "./chroot/" for example. After - * the server has been loaded, it does a "cd - * ${logdir}" below, so that core files (if any) - * go to a logging directory. - * - * This also allows the configuration of the - * server to be outside of the chroot. If the - * server is statically linked, then the only - * things needed inside of the chroot are the - * logging directories. - */ - } - #ifdef HAVE_GRP_H /* * Set the GID. Don't bother checking it. @@ -1388,16 +1358,6 @@ int main_config_init(main_config_t *config) xlat_func_args_set(xlat, xlat_config_args); xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); - /* - * Ensure cwd is inside the chroot. - */ - if (config->chroot_dir) { - if (chdir(config->log_dir) < 0) { - ERROR("Failed to 'chdir %s' after chroot: %s", config->log_dir, fr_syserror(errno)); - goto failure; - } - } - config->root_cs = cs; /* Do this last to avoid dangling pointers on error */ /* Clear any unprocessed configuration errors */ diff --git a/src/lib/server/main_config.h b/src/lib/server/main_config.h index 5a8b30b0d5..fe8901b3ee 100644 --- a/src/lib/server/main_config.h +++ b/src/lib/server/main_config.h @@ -65,7 +65,6 @@ struct main_config_s { char const *log_dir; char const *local_state_dir; - char const *chroot_dir; bool reverse_lookups; bool hostname_lookups;