From f078395411559297198a1ec8987715161a42c797 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Wed, 25 Jan 2023 16:42:23 +0100 Subject: [PATCH] drop default chrony.keys config (#2104918) --- chrony-keyaccess.patch | 191 +++++++++++++++++++++++++++++++++++++++++ chrony.spec | 13 ++- 2 files changed, 197 insertions(+), 7 deletions(-) create mode 100644 chrony-keyaccess.patch diff --git a/chrony-keyaccess.patch b/chrony-keyaccess.patch new file mode 100644 index 0000000..4beedd8 --- /dev/null +++ b/chrony-keyaccess.patch @@ -0,0 +1,191 @@ +commit 9cba9c8585bc5ebf19bafece118fb2362090547c +Author: Miroslav Lichvar +Date: Thu Jan 19 16:09:40 2023 +0100 + + keys+nts: warn if loading world-readable/writable key + + Log a warning message if the file specified by the keyfile or + ntsserverkey directive is world-readable or writable, which is likely + an insecure misconfiguration. There is no check of directories + containing the file. + +diff --git a/keys.c b/keys.c +index 11f8b761..9225e6cd 100644 +--- a/keys.c ++++ b/keys.c +@@ -182,6 +182,9 @@ KEY_Reload(void) + if (!key_file) + return; + ++ if (!UTI_CheckFilePermissions(key_file, 0771)) ++ ; ++ + in = UTI_OpenFile(NULL, key_file, NULL, 'r', 0); + if (!in) { + LOG(LOGS_WARN, "Could not open keyfile %s", key_file); +diff --git a/nts_ke_session.c b/nts_ke_session.c +index dfcd18ab..2ae1e915 100644 +--- a/nts_ke_session.c ++++ b/nts_ke_session.c +@@ -667,6 +667,8 @@ create_credentials(const char **certs, const char **keys, int n_certs_keys, + assert(0); + + for (i = 0; i < n_certs_keys; i++) { ++ if (!UTI_CheckFilePermissions(keys[i], 0771)) ++ ; + r = gnutls_certificate_set_x509_key_file(credentials, certs[i], keys[i], + GNUTLS_X509_FMT_PEM); + if (r < 0) +diff --git a/util.c b/util.c +index 064292ce..4b9d30ee 100644 +--- a/util.c ++++ b/util.c +@@ -1248,6 +1248,29 @@ UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid) + + /* ================================================== */ + ++int ++UTI_CheckFilePermissions(const char *path, mode_t perm) ++{ ++ mode_t extra_perm; ++ struct stat buf; ++ ++ if (stat(path, &buf) < 0 || !S_ISREG(buf.st_mode)) { ++ /* Not considered an error */ ++ return 1; ++ } ++ ++ extra_perm = (buf.st_mode & 0777) & ~perm; ++ if (extra_perm != 0) { ++ LOG(LOGS_WARN, "%s permissions on %s", extra_perm & 0006 ? ++ (extra_perm & 0004 ? "World-readable" : "World-writable") : "Wrong", path); ++ return 0; ++ } ++ ++ return 1; ++} ++ ++/* ================================================== */ ++ + static int + join_path(const char *basedir, const char *name, const char *suffix, + char *buffer, size_t length, LOG_Severity severity) +diff --git a/util.h b/util.h +index 4655e537..6844798c 100644 +--- a/util.h ++++ b/util.h +@@ -196,6 +196,10 @@ extern int UTI_CreateDirAndParents(const char *path, mode_t mode, uid_t uid, gid + permissions and its uid/gid must match the specified values. */ + extern int UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid); + ++/* Check and log a warning message if a file has more permissions than ++ specified. It does not return error if it is not an accessible file. */ ++extern int UTI_CheckFilePermissions(const char *path, mode_t perm); ++ + /* Open a file. The full path of the file is constructed from the basedir + (may be NULL), '/' (if basedir is not NULL), name, and suffix (may be NULL). + Created files have specified permissions (umasked). Returns NULL on error. + +commit 883b0dde946105e0910456a0bebb24d57fecb0fc +Author: Miroslav Lichvar +Date: Wed Jan 25 14:29:06 2023 +0100 + + conf: warn if not having read-only access to keys + + After dropping root privileges, log a warning message if chronyd + doesn't have read access or has (unnecessary) write access to the + files containing symmetric and server NTS keys. + +diff --git a/conf.c b/conf.c +index 9f42a426..0597836d 100644 +--- a/conf.c ++++ b/conf.c +@@ -1774,6 +1774,19 @@ CNF_CreateDirs(uid_t uid, gid_t gid) + + /* ================================================== */ + ++void ++CNF_CheckReadOnlyAccess(void) ++{ ++ unsigned int i; ++ ++ if (keys_file) ++ UTI_CheckReadOnlyAccess(keys_file); ++ for (i = 0; i < ARR_GetSize(nts_server_key_files); i++) ++ UTI_CheckReadOnlyAccess(*(char **)ARR_GetElement(nts_server_key_files, i)); ++} ++ ++/* ================================================== */ ++ + void + CNF_AddInitSources(void) + { +diff --git a/conf.h b/conf.h +index 11fd11df..d7acb4fd 100644 +--- a/conf.h ++++ b/conf.h +@@ -44,6 +44,8 @@ extern void CNF_ParseLine(const char *filename, int number, char *line); + + extern void CNF_CreateDirs(uid_t uid, gid_t gid); + ++extern void CNF_CheckReadOnlyAccess(void); ++ + extern void CNF_AddInitSources(void); + extern void CNF_AddSources(void); + extern void CNF_AddBroadcasts(void); +diff --git a/main.c b/main.c +index c40b5e4b..31e3c8f0 100644 +--- a/main.c ++++ b/main.c +@@ -637,9 +637,13 @@ int main + } + + /* Drop root privileges if the specified user has a non-zero UID */ +- if (!geteuid() && (pw->pw_uid || pw->pw_gid)) ++ if (!geteuid() && (pw->pw_uid || pw->pw_gid)) { + SYS_DropRoot(pw->pw_uid, pw->pw_gid, SYS_MAIN_PROCESS); + ++ /* Warn if missing read access or having write access to keys */ ++ CNF_CheckReadOnlyAccess(); ++ } ++ + if (!geteuid()) + LOG(LOGS_WARN, "Running with root privileges"); + +diff --git a/util.c b/util.c +index 4b9d30ee..0321720e 100644 +--- a/util.c ++++ b/util.c +@@ -1271,6 +1271,17 @@ UTI_CheckFilePermissions(const char *path, mode_t perm) + + /* ================================================== */ + ++void ++UTI_CheckReadOnlyAccess(const char *path) ++{ ++ if (access(path, R_OK) != 0 && errno != ENOENT) ++ LOG(LOGS_WARN, "Missing read access to %s : %s", path, strerror(errno)); ++ if (access(path, W_OK) == 0) ++ LOG(LOGS_WARN, "Having write access to %s", path); ++} ++ ++/* ================================================== */ ++ + static int + join_path(const char *basedir, const char *name, const char *suffix, + char *buffer, size_t length, LOG_Severity severity) +diff --git a/util.h b/util.h +index 6844798c..d8e25dee 100644 +--- a/util.h ++++ b/util.h +@@ -200,6 +200,10 @@ extern int UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid + specified. It does not return error if it is not an accessible file. */ + extern int UTI_CheckFilePermissions(const char *path, mode_t perm); + ++/* Log a warning message if not having read access or having write access ++ to a file/directory */ ++extern void UTI_CheckReadOnlyAccess(const char *path); ++ + /* Open a file. The full path of the file is constructed from the basedir + (may be NULL), '/' (if basedir is not NULL), name, and suffix (may be NULL). + Created files have specified permissions (umasked). Returns NULL on error. diff --git a/chrony.spec b/chrony.spec index 01f924d..3744814 100644 --- a/chrony.spec +++ b/chrony.spec @@ -27,6 +27,8 @@ Source10: https://github.com/mlichvar/clknetsim/archive/%{clknetsim_ver}/c Patch1: chrony-nm-dispatcher-dhcp.patch # add chronyd-restricted service Patch2: chrony-restricted.patch +# warn if keys are world-accessible or chronyd doesn't have read-only access +Patch3: chrony-keyaccess.patch BuildRequires: libcap-devel libedit-devel nettle-devel pps-tools-devel BuildRequires: gcc gcc-c++ make bison systemd gnupg2 @@ -59,6 +61,7 @@ service to other computers in the network. %{?gitpatch:%patch0 -p1} %patch1 -p1 -b .nm-dispatcher-dhcp %patch2 -p1 -b .restricted +%patch3 -p1 -b .keyaccess %{?gitpatch: echo %{version}-%{gitpatch} > version.txt} @@ -66,7 +69,6 @@ service to other computers in the network. md5sum -c <<-EOF | (! grep -v 'OK$') b40117b4aac846d31e4ad196dc44cda3 examples/chrony-wait.service 2d01b94bc1a7b7fb70cbee831488d121 examples/chrony.conf.example2 - 96999221eeef476bd49fe97b97503126 examples/chrony.keys.example 6a3178c4670de7de393d9365e2793740 examples/chrony.logrotate c3992e2f985550739cd1cd95f98c9548 examples/chrony.nm-dispatcher.dhcp 2b81c60c020626165ac655b2633608eb examples/chrony.nm-dispatcher.onoffline @@ -80,11 +82,9 @@ test -n "%{vendorzone}" # use example chrony.conf as the default config with some modifications: # - use our vendor zone (2.*pool.ntp.org names include IPv6 addresses) # - enable leapsectz to get TAI-UTC offset and leap seconds from tzdata -# - enable keyfile # - use NTP servers from DHCP sed -e 's|^\(pool \)\(pool.ntp.org\)|\12.%{vendorzone}\2|' \ -e 's|#\(leapsectz\)|\1|' \ - -e 's|#\(keyfile\)|\1|' \ -e 's|^pool.*pool.ntp.org.*|&\n\n# Use NTP servers from DHCP.\nsourcedir /run/chrony-dhcp|' \ < examples/chrony.conf.example2 > chrony.conf @@ -125,8 +125,6 @@ mkdir -p $RPM_BUILD_ROOT{%{_unitdir},%{_prefix}/lib/systemd/ntp-units.d} install -m 644 -p chrony.conf $RPM_BUILD_ROOT%{_sysconfdir}/chrony.conf -install -m 640 -p examples/chrony.keys.example \ - $RPM_BUILD_ROOT%{_sysconfdir}/chrony.keys install -m 755 -p %{SOURCE3} \ $RPM_BUILD_ROOT%{_sysconfdir}/dhcp/dhclient.d/chrony.sh install -m 644 -p examples/chrony.logrotate \ @@ -150,6 +148,7 @@ cat > $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/chronyd < \ @@ -186,9 +185,9 @@ fi %files %{!?_licensedir:%global license %%doc} %license COPYING -%doc FAQ NEWS README +%doc FAQ NEWS README examples/chrony.keys.example %config(noreplace) %{_sysconfdir}/chrony.conf -%config(noreplace) %verify(not md5 size mtime) %attr(640,root,chrony) %{_sysconfdir}/chrony.keys +%ghost %config %attr(640,root,chrony) %{_sysconfdir}/chrony.keys %config(noreplace) %{_sysconfdir}/logrotate.d/chrony %config(noreplace) %{_sysconfdir}/sysconfig/chronyd %{_sysconfdir}/dhcp/dhclient.d/chrony.sh