drop default chrony.keys config (#2104918)

This commit is contained in:
Miroslav Lichvar 2023-01-25 16:42:23 +01:00
parent 906e072deb
commit f078395411
2 changed files with 197 additions and 7 deletions

191
chrony-keyaccess.patch Normal file
View File

@ -0,0 +1,191 @@
commit 9cba9c8585bc5ebf19bafece118fb2362090547c
Author: Miroslav Lichvar <mlichvar@redhat.com>
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 <mlichvar@redhat.com>
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.

View File

@ -27,6 +27,8 @@ Source10: https://github.com/mlichvar/clknetsim/archive/%{clknetsim_ver}/c
Patch1: chrony-nm-dispatcher-dhcp.patch Patch1: chrony-nm-dispatcher-dhcp.patch
# add chronyd-restricted service # add chronyd-restricted service
Patch2: chrony-restricted.patch 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: libcap-devel libedit-devel nettle-devel pps-tools-devel
BuildRequires: gcc gcc-c++ make bison systemd gnupg2 BuildRequires: gcc gcc-c++ make bison systemd gnupg2
@ -59,6 +61,7 @@ service to other computers in the network.
%{?gitpatch:%patch0 -p1} %{?gitpatch:%patch0 -p1}
%patch1 -p1 -b .nm-dispatcher-dhcp %patch1 -p1 -b .nm-dispatcher-dhcp
%patch2 -p1 -b .restricted %patch2 -p1 -b .restricted
%patch3 -p1 -b .keyaccess
%{?gitpatch: echo %{version}-%{gitpatch} > version.txt} %{?gitpatch: echo %{version}-%{gitpatch} > version.txt}
@ -66,7 +69,6 @@ service to other computers in the network.
md5sum -c <<-EOF | (! grep -v 'OK$') md5sum -c <<-EOF | (! grep -v 'OK$')
b40117b4aac846d31e4ad196dc44cda3 examples/chrony-wait.service b40117b4aac846d31e4ad196dc44cda3 examples/chrony-wait.service
2d01b94bc1a7b7fb70cbee831488d121 examples/chrony.conf.example2 2d01b94bc1a7b7fb70cbee831488d121 examples/chrony.conf.example2
96999221eeef476bd49fe97b97503126 examples/chrony.keys.example
6a3178c4670de7de393d9365e2793740 examples/chrony.logrotate 6a3178c4670de7de393d9365e2793740 examples/chrony.logrotate
c3992e2f985550739cd1cd95f98c9548 examples/chrony.nm-dispatcher.dhcp c3992e2f985550739cd1cd95f98c9548 examples/chrony.nm-dispatcher.dhcp
2b81c60c020626165ac655b2633608eb examples/chrony.nm-dispatcher.onoffline 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 example chrony.conf as the default config with some modifications:
# - use our vendor zone (2.*pool.ntp.org names include IPv6 addresses) # - 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 leapsectz to get TAI-UTC offset and leap seconds from tzdata
# - enable keyfile
# - use NTP servers from DHCP # - use NTP servers from DHCP
sed -e 's|^\(pool \)\(pool.ntp.org\)|\12.%{vendorzone}\2|' \ sed -e 's|^\(pool \)\(pool.ntp.org\)|\12.%{vendorzone}\2|' \
-e 's|#\(leapsectz\)|\1|' \ -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|' \ -e 's|^pool.*pool.ntp.org.*|&\n\n# Use NTP servers from DHCP.\nsourcedir /run/chrony-dhcp|' \
< examples/chrony.conf.example2 > chrony.conf < 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 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} \ install -m 755 -p %{SOURCE3} \
$RPM_BUILD_ROOT%{_sysconfdir}/dhcp/dhclient.d/chrony.sh $RPM_BUILD_ROOT%{_sysconfdir}/dhcp/dhclient.d/chrony.sh
install -m 644 -p examples/chrony.logrotate \ install -m 644 -p examples/chrony.logrotate \
@ -150,6 +148,7 @@ cat > $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/chronyd <<EOF
OPTIONS="%{?with_seccomp:-F 2}" OPTIONS="%{?with_seccomp:-F 2}"
EOF EOF
touch $RPM_BUILD_ROOT%{_sysconfdir}/chrony.keys
touch $RPM_BUILD_ROOT%{_localstatedir}/lib/chrony/{drift,rtc} touch $RPM_BUILD_ROOT%{_localstatedir}/lib/chrony/{drift,rtc}
echo 'chronyd.service' > \ echo 'chronyd.service' > \
@ -186,9 +185,9 @@ fi
%files %files
%{!?_licensedir:%global license %%doc} %{!?_licensedir:%global license %%doc}
%license COPYING %license COPYING
%doc FAQ NEWS README %doc FAQ NEWS README examples/chrony.keys.example
%config(noreplace) %{_sysconfdir}/chrony.conf %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}/logrotate.d/chrony
%config(noreplace) %{_sysconfdir}/sysconfig/chronyd %config(noreplace) %{_sysconfdir}/sysconfig/chronyd
%{_sysconfdir}/dhcp/dhclient.d/chrony.sh %{_sysconfdir}/dhcp/dhclient.d/chrony.sh