From 6c82e2af92c741b52585e67054a67862211875b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Apr 2020 15:58:45 +0200 Subject: [PATCH 1/4] Don't change effective uid when we already dropped privileges When running on Linux and system capabilities are available, named will drop the extra capabilities before loading the configuration. This led to spurious warnings from `seteuid()` because named already dropped CAP_SETUID and CAP_GETUID capabilities. The fix removes setting the effective uid/gid when capabilities are available, and adds a check that we are running under the user we were requested to run. --- bin/named/unix/os.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bin/named/unix/os.c b/bin/named/unix/os.c index b99b0d3d00..c9a9441a84 100644 --- a/bin/named/unix/os.c +++ b/bin/named/unix/os.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -414,7 +415,6 @@ named_os_chroot(const char *root) { void named_os_inituserinfo(const char *username) { - char strbuf[ISC_STRERRORSIZE]; if (username == NULL) { return; } @@ -431,6 +431,7 @@ named_os_inituserinfo(const char *username) { } if (getuid() == 0) { + char strbuf[ISC_STRERRORSIZE]; if (initgroups(runas_pw->pw_name, runas_pw->pw_gid) < 0) { strerror_r(errno, strbuf, sizeof(strbuf)); named_main_earlyfatal("initgroups(): %s", strbuf); @@ -696,14 +697,21 @@ named_os_openfile(const char *filename, mode_t mode, bool switch_user) { free(f); if (switch_user && runas_pw != NULL) { + uid_t olduid = getuid(); gid_t oldgid = getgid(); +#if HAVE_SYS_CAPABILITY_H + REQUIRE(olduid == runas_pw->pw_uid); + REQUIRE(oldgid == runas_pw->pw_gid); +#else /* HAVE_SYS_CAPABILITY_H */ /* Set UID/GID to the one we'll be running with eventually */ setperms(runas_pw->pw_uid, runas_pw->pw_gid); - +#endif fd = safe_open(filename, mode, false); - /* Restore UID/GID to root */ - setperms(0, oldgid); +#if !HAVE_SYS_CAPABILITY_H + /* Restore UID/GID to previous uid/gid */ + setperms(olduid, oldgid); +#endif if (fd == -1) { fd = safe_open(filename, mode, false); From 071bc29962ec5d7117b5a54b9e5e0c2d4081474b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 28 Apr 2020 08:13:02 +1000 Subject: [PATCH 2/4] Propagate first_time to named_os_openfile in generate_session_key. named_os_openfile was being called with switch_user set to true unconditionally leading to log messages about being unable to switch user identity from named when regenerating the key. --- bin/named/server.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c1851eded8..0ff543e238 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7313,7 +7313,8 @@ static isc_result_t generate_session_key(const char *filename, const char *keynamestr, const dns_name_t *keyname, const char *algstr, const dns_name_t *algname, unsigned int algtype, - uint16_t bits, isc_mem_t *mctx, dns_tsigkey_t **tsigkeyp) { + uint16_t bits, isc_mem_t *mctx, bool first_time, + dns_tsigkey_t **tsigkeyp) { isc_result_t result = ISC_R_SUCCESS; dst_key_t *key = NULL; isc_buffer_t key_txtbuffer; @@ -7354,7 +7355,7 @@ generate_session_key(const char *filename, const char *keynamestr, NULL, now, now, mctx, NULL, &tsigkey)); /* Dump the key to the key file. */ - fp = named_os_openfile(filename, S_IRUSR | S_IWUSR, true); + fp = named_os_openfile(filename, S_IRUSR | S_IWUSR, first_time); if (fp == NULL) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -7405,7 +7406,7 @@ cleanup: static isc_result_t configure_session_key(const cfg_obj_t **maps, named_server_t *server, - isc_mem_t *mctx) { + isc_mem_t *mctx, bool first_time) { const char *keyfile, *keynamestr, *algstr; unsigned int algtype; dns_fixedname_t fname; @@ -7501,7 +7502,7 @@ configure_session_key(const cfg_obj_t **maps, named_server_t *server, CHECK(generate_session_key(keyfile, keynamestr, keyname, algstr, algname, algtype, bits, mctx, - &server->sessionkey)); + first_time, &server->sessionkey)); } return (result); @@ -8882,7 +8883,7 @@ load_configuration(const char *filename, named_server_t *server, * turns out that a session key is really needed but doesn't exist, * we'll treat it as a fatal error then. */ - (void)configure_session_key(maps, server, named_g_mctx); + (void)configure_session_key(maps, server, named_g_mctx, first_time); /* * Create the DNSSEC key and signing policies (KASP). From a0a5eab31c206af31ad4ac4a400d9551ce97d330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Apr 2020 16:23:07 +0200 Subject: [PATCH 3/4] Add CHANGES for #1042, #1090 --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index cec94bbd5c..e269ea9396 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5394. [cleanup] Don't change effective uid/gid in named_os_openfile() + if named is already running under specified uid/gid. + [GL #1042] [GL #1090] + 5393. [cleanup] Unused or redundant APIs were removed from libirs. [GL #1758] From 60b608b65b4b38d94965074b43a3ae9416a398da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Apr 2020 16:27:05 +0200 Subject: [PATCH 4/4] Add release notes for GL #1042, #1090 --- doc/arm/notes-9.17.2.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/arm/notes-9.17.2.xml b/doc/arm/notes-9.17.2.xml index ed276bdcc0..0d48dd78dd 100644 --- a/doc/arm/notes-9.17.2.xml +++ b/doc/arm/notes-9.17.2.xml @@ -66,7 +66,11 @@ - None. + When running on a system with Linux capabilities support, + named drops root privileges very soon after system + startup. This was causing a spurious log message, unable to set + effective uid to 0: Operation not permitted, which has now been + silenced. [GL #1042] [GL #1090]