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.