From 8a09b3e22cc4d59a18414f4be8f69d2ba65f7d0d Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 30 Aug 2024 13:51:50 +0200 Subject: [PATCH] confd: fix audit logs using proper facility The LOG_SECURITY facility was set wrong (1 << 13) instead of (13 << 3), see https://github.com/kernelkit/sysklogd/blob/0fc6656/src/syslog.h#L120 for details. This caused all audit log messages to be logged in LOG_USER. Also, rename LOG_SECURITY -> LOG_AUDIT and log macro SECURITY() -> AUDIT() to match RFC5424 terminology. Similar fix to sysrepo, LOG_AUDIT facility instead of daemon + WARNING. Additionally, drop the leading [severity] prefix to sysrepo logs. Only needed when logging to stdout. Follow-up to issue #521 Signed-off-by: Joachim Wiberg --- ...-new-log-level-SEC-for-audit-trails.patch} | 54 +++++++++++++++---- ...ail-for-high-priority-system-changes.patch | 6 +-- ...sr_shmsub_listen_thread-exit-process.patch | 2 +- src/confd/src/ietf-system.c | 54 +++++++++---------- src/libsrx/src/common.h | 6 +-- 5 files changed, 77 insertions(+), 45 deletions(-) rename patches/sysrepo/2.10.1/{0007-Introduce-new-log-level-SEC.patch => 0007-Introduce-new-log-level-SEC-for-audit-trails.patch} (61%) diff --git a/patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC.patch b/patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC-for-audit-trails.patch similarity index 61% rename from patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC.patch rename to patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC-for-audit-trails.patch index 6ae89318c..a87b31128 100644 --- a/patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC.patch +++ b/patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC-for-audit-trails.patch @@ -1,11 +1,12 @@ -From da765b90bca45b91f72fd6525e680040eebd2d4b Mon Sep 17 00:00:00 2001 +From d128686fb15833e815ac3dd04bd87d3725c881ac Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Wed, 21 Aug 2024 16:00:35 +0200 -Subject: [PATCH 7/9] Introduce new log level [SEC] +Subject: [PATCH 7/9] Introduce new log level [SEC] for audit trails Organization: Addiva Elektronik This adds a new log level for security and audit trail related log -messages. E.g., nacm user applied a change, copied a ds, etc. +messages, see LOG_AUDIT defined in RFC5424. E.g., nacm user applied +a change, copied a datastore, etc. The new log level is added last to not affect the advertised command line log levels. A security notice has higher actual priorty than @@ -13,30 +14,61 @@ DBG, of course, so we remap it to WRN. The construct allows us to have another [label] than [WRN], which might otherwise be read as a bug/problem rather than just a high-priority-notification. +When logging to syslog() we let delegate labling and filtering to the +system log daemon, dropping any [SEVERITY] prefix. Also, \n is most +often dropped by log daemons. + Signed-off-by: Joachim Wiberg --- - src/log.c | 5 +++++ - src/log.h | 1 + - src/sysrepo_types.h | 3 ++- - tests/tcommon.c | 3 +++ - 4 files changed, 11 insertions(+), 1 deletion(-) + src/log.c | 18 +++++++++++++++++- + src/log.h | 1 + + src/sysrepo_types.h | 3 ++- + tests/tcommon.c | 3 +++ + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/log.c b/src/log.c -index e15055ac..b89ffacf 100644 +index e15055ac..25eab8fa 100644 --- a/src/log.c +++ b/src/log.c -@@ -122,6 +122,11 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg) +@@ -30,6 +30,10 @@ + + #include "config.h" + ++#ifndef LOG_AUDIT ++#define LOG_AUDIT (13<<3) /* Log audit, for audit trails */ ++#endif ++ + sr_log_level_t sr_stderr_ll = SR_LL_NONE; /**< stderr log level */ + sr_log_level_t sr_syslog_ll = SR_LL_NONE; /**< syslog log level */ + int syslog_open; /**< Whether syslog was opened */ +@@ -122,6 +126,11 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg) priority = LOG_INFO; severity = "INF"; break; + case SR_LL_SEC: -+ priority = LOG_WARNING; ++ priority = LOG_AUDIT | LOG_NOTICE; + severity = "SEC"; + ll = SR_LL_WRN; /* remap to higher level. */ + break; case SR_LL_DBG: priority = LOG_DEBUG; severity = "DBG"; +@@ -138,7 +147,14 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg) + + /* syslog logging */ + if (ll <= sr_syslog_ll) { +- syslog(priority | (plugin ? LOG_DAEMON : 0), "[%s] %s\n", severity, msg); ++ int facility; ++ ++ if (priority & ~LOG_PRIMASK) ++ facility = 0; /* audit */ ++ else ++ facility = plugin ? LOG_DAEMON : 0; ++ ++ syslog(facility | priority, "%s", msg); + } + + /* logging callback */ diff --git a/src/log.h b/src/log.h index d7e65b88..8722e51d 100644 --- a/src/log.h diff --git a/patches/sysrepo/2.10.1/0008-Add-audit-trail-for-high-priority-system-changes.patch b/patches/sysrepo/2.10.1/0008-Add-audit-trail-for-high-priority-system-changes.patch index 68ca5c8b1..86f6fe48e 100644 --- a/patches/sysrepo/2.10.1/0008-Add-audit-trail-for-high-priority-system-changes.patch +++ b/patches/sysrepo/2.10.1/0008-Add-audit-trail-for-high-priority-system-changes.patch @@ -1,4 +1,4 @@ -From 7e49f394e0afedc0259d4364f5a9a83296fe2b72 Mon Sep 17 00:00:00 2001 +From b5db2b36c06d28918f0d00dda945f7e943b470af Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Wed, 21 Aug 2024 16:04:43 +0200 Subject: [PATCH 8/9] Add audit trail for high priority system changes @@ -18,7 +18,7 @@ Signed-off-by: Joachim Wiberg 1 file changed, 12 insertions(+) diff --git a/src/sysrepo.c b/src/sysrepo.c -index 86d694e5..c7b97e53 100644 +index 86d694e5..dcea51e8 100644 --- a/src/sysrepo.c +++ b/src/sysrepo.c @@ -3946,6 +3946,9 @@ store: @@ -45,7 +45,7 @@ index 86d694e5..c7b97e53 100644 } } -+ if (session->nacm_user) ++ if (session->nacm_user && src_datastore != SR_DS_CANDIDATE) + SR_LOG_SEC("user \"%s\" copied %s to %s", session->nacm_user, sr_ds2str(src_datastore), sr_ds2str(session->ds)); + cleanup: diff --git a/patches/sysrepo/2.10.1/0009-On-error-in-sr_shmsub_listen_thread-exit-process.patch b/patches/sysrepo/2.10.1/0009-On-error-in-sr_shmsub_listen_thread-exit-process.patch index 1962449d1..c42aa5a21 100644 --- a/patches/sysrepo/2.10.1/0009-On-error-in-sr_shmsub_listen_thread-exit-process.patch +++ b/patches/sysrepo/2.10.1/0009-On-error-in-sr_shmsub_listen_thread-exit-process.patch @@ -1,4 +1,4 @@ -From 5c4df9f535f6fbdc6dc0573a47c47be2d02f1774 Mon Sep 17 00:00:00 2001 +From 13d54101eb2c3506237cebc5be99edef2ad669cd Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 23 Aug 2024 12:22:06 +0200 Subject: [PATCH 9/9] On error in sr_shmsub_listen_thread(), exit process diff --git a/src/confd/src/ietf-system.c b/src/confd/src/ietf-system.c index fb67e6e95..5a3647b59 100644 --- a/src/confd/src/ietf-system.c +++ b/src/confd/src/ietf-system.c @@ -607,9 +607,9 @@ static void add_group(const char *user, const char *group) return; /* already group member */ if (systemf("adduser %s %s", user, group)) - SECURITY("Failed giving user %s UNIX %s permissions.", user, group); + AUDIT("Failed giving user \"%s\" UNIX %s permissions.", user, group); else - SECURITY("User %s added to UNIX %s group.", user, group); + AUDIT("User \"%s\" added to UNIX \"%s\" group.", user, group); } static void del_group(const char *user, const char *group) @@ -620,9 +620,9 @@ static void del_group(const char *user, const char *group) return; /* not member of group */ if (systemf("delgroup %s %s", user, group)) - SECURITY("Failed removing user %s from UNIX %s group.", user, group); + AUDIT("Failed removing user \"%s\" from UNIX \"%s\" group.", user, group); else - SECURITY("User %s removed from UNIX %s group.", user, group); + AUDIT("User \"%s\" removed from UNIX \"%s\" group.", user, group); } /* Users with a valid shell are also allowed CLI access */ @@ -786,7 +786,7 @@ static int sys_call_adduser(sr_session_ctx_t *sess, char *name, uid_t uid, gid_t char **args; int err; - DEBUG("Adding new user %s, cleaning up any stale group.", name); + DEBUG("Adding new user \"%s\", cleaning up any stale group.", name); systemf("delgroup %s 2>/dev/null", name); /* reusing existing uid:gid from $HOME */ @@ -840,15 +840,15 @@ static int sys_add_user(sr_session_ctx_t *sess, char *name) /* Verify IDs aren't already used, like BusyBox adduser */ if (getpwuid(st.st_uid) || getgrgid(st.st_uid) || getgrgid(st.st_gid)) { /* Exists but owned by someone else. */ - SECURITY("Failed mapping user %s to /home/%s, uid:gid (%d:%d) already exists.", + AUDIT("Failed mapping user \"%s\" to /home/%s, uid:gid (%d:%d) already exists.", name, name, st.st_uid, st.st_gid); err = sys_call_adduser(sess, name, 0, 0); } else { - SECURITY("Reusing uid:gid %d:%d and /home/%s for new user %s", + AUDIT("Reusing uid:gid %d:%d and /home/%s for new user \"%s\"", st.st_uid, st.st_gid, name, name); err = sys_call_adduser(sess, name, st.st_uid, st.st_gid); if (err) { - SECURITY("Failed reusing uid:gid from /home/%s, retrying create user ...", name); + AUDIT("Failed reusing uid:gid from /home/%s, retrying create user ...", name); err = sys_call_adduser(sess, name, 0, 0); } else reused = true; @@ -857,11 +857,11 @@ static int sys_add_user(sr_session_ctx_t *sess, char *name) err = sys_call_adduser(sess, name, 0, 0); if (err) { - SECURITY("Failed creating new user \"%s\"", name); + AUDIT("Failed creating new user \"%s\"", name); return SR_ERR_SYS; } - SECURITY("User \"%s\" created%s.", name, reused ? ", mapped to existing home directory" : ""); + AUDIT("User \"%s\" created%s.", name, reused ? ", mapped to existing home directory" : ""); /* * OpenSSH in Infix has been set up to use /var/run/sshd/%s.keys @@ -930,7 +930,7 @@ static int set_shell(const char *user, const char *shell) if (!strcmp(pw->pw_name, user)) { if (strcmp(pw->pw_shell, shell)) - NOTE("Updating login shell for user %s to %s", user, shell); + AUDIT("Updating login shell for user \"%s\" to %s", user, shell); upw = *pw; upw.pw_shell = (char *)shell; @@ -953,7 +953,7 @@ static int set_shell(const char *user, const char *shell) if (fp) fclose(fp); endpwent(); - ERRNO("Failed setting user %s shell %s", user, shell); + ERRNO("Failed setting user \"%s\" shell %s", user, shell); return -1; } @@ -971,7 +971,7 @@ static int set_password(const char *user, const char *hash, bool lock) fp = fopen(_PATH_SHADOW "+", "w"); if (!fp) { - ERRNO("Failed opening %s+ for %s", _PATH_SHADOW, user); + ERRNO("Failed opening %s+ for user \"%s\"", _PATH_SHADOW, user); goto fail; } fd = fileno(fp); @@ -1016,7 +1016,7 @@ static int set_password(const char *user, const char *hash, bool lock) endspent(); ulckpwdf(); exit: - SECURITY("Failed setting password for %s", user); + AUDIT("Failed setting password for user \"%s\"", user); return -1; } @@ -1045,7 +1045,7 @@ static const char *is_valid_hash(struct confd *confd, const char *user, const ch pwd = json_object_get(confd->root, "factory-password-hash"); if (!json_is_string(pwd)) { - EMERG("Cannot find factory-default password hash for user %s!", user); + EMERG("Cannot find factory-default password hash for user \"%s\"!", user); return NULL; } @@ -1072,7 +1072,7 @@ static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *conf assert(change->new); if (change->new->type != SR_STRING_T) { - SECURITY("Internal error, expected user %s password to be string type.", user); + AUDIT("Internal error, expected user \"%s\" password to be string type.", user); err = SR_ERR_INTERNAL; break; } @@ -1090,17 +1090,17 @@ static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *conf if (set_password(user, hash, lock)) err = SR_ERR_SYS; else if (lock) - NOTE("User account %s locked.", user); + NOTE("User account \"%s\" locked.", user); else if (!strcmp(hash, "*")) - NOTE("Password login disabled for user %s", user); + NOTE("Password login disabled for user \"%s\"", user); else - SECURITY("Password updated for user %s", user); + AUDIT("Password updated for user \"%s\"", user); break; case SR_OP_DELETED: if (set_password(user, "*", false)) err = SR_ERR_SYS; else - NOTE("Password login disabled for user %s", user); + NOTE("Password login disabled for user \"%s\"", user); break; case SR_OP_MOVED: break; @@ -1125,10 +1125,10 @@ static sr_error_t handle_sr_shell_update(sr_session_ctx_t *sess, struct confd *c shell = sys_find_usable_shell(sess, (char *)user, is_admin_user(sess, user)); if (set_shell(user, shell)) { - SECURITY("Failed updating shell to %s for user %s", shell, user); + AUDIT("Failed updating shell to %s for user \"%s\"", shell, user); err = SR_ERR_SYS; } else { - SECURITY("Login shell updated for user %s", user); + AUDIT("Login shell updated for user \"%s\"", user); err = SR_ERR_OK; } free(shell); @@ -1148,7 +1148,7 @@ static sr_error_t check_sr_user_update(sr_session_ctx_t *, struct confd *, struc name = sr_xpath_key_value(val->xpath, "user", "name", &state); if (!is_valid_username(name)) { - SECURITY("Invalid username \"%s\"", name); + AUDIT("Invalid username \"%s\"", name); return SR_ERR_VALIDATION_FAILED; } NOTE("Username \"%s\" is valid", name); @@ -1232,7 +1232,7 @@ static sr_error_t generate_auth_keys(sr_session_ctx_t *session, const char *xpat fp = fopenf("w", "/var/run/sshd/%s.keys", username); if (!fp) { - ERROR("failed opening %s authorized_keys file: %s", username, strerror(errno)); + ERROR("failed opening user \"%s\" authorized_keys file: %s", username, strerror(errno)); continue; } @@ -1361,7 +1361,7 @@ static sr_error_t change_auth_done(struct confd *confd, sr_session_ctx_t *sessio err = generate_auth_keys(session, XPATH_AUTH_"/user//."); if (err) { - SECURITY("failed saving authorized-key data."); + AUDIT("failed saving authorized-key data."); goto cleanup; } @@ -1432,11 +1432,11 @@ static int change_nacm(sr_session_ctx_t *session, uint32_t sub_id, const char *m for (size_t i = 0; i < user_count; i++) { const char *user = users[i].data.string_val; bool is_admin = is_admin_user(session, user); - char *shell; + const char *shell; shell = sys_find_usable_shell(session, (char *)user, is_admin); if (set_shell(user, shell)) - SECURITY("Failed adjusting shell for user %s", user); + AUDIT("Failed adjusting shell for user \"%s\"", user); if (is_admin) add_group(user, "wheel"); diff --git a/src/libsrx/src/common.h b/src/libsrx/src/common.h index d208f042a..cb6fdaabb 100644 --- a/src/libsrx/src/common.h +++ b/src/libsrx/src/common.h @@ -11,8 +11,8 @@ extern int debug; /* In IETF referred to LOG_AUDIT */ -#ifndef LOG_SECURITY -#define LOG_SECURITY (1 << 13) +#ifndef LOG_AUDIT +#define LOG_AUDIT (13<<3) /* Log audit, for audit trails */ #endif #ifndef HAVE_VASPRINTF @@ -33,6 +33,6 @@ int asprintf(char **strp, const char *fmt, ...); #define EMERG(fmt, ...) syslog(LOG_EMERG, fmt, ##__VA_ARGS__) #define ERROR(fmt, ...) syslog(LOG_ERR, fmt, ##__VA_ARGS__) #define ERRNO(fmt, ...) syslog(LOG_ERR, fmt ": %s", ##__VA_ARGS__, strerror(errno)) -#define SECURITY(fmt, ...) syslog(LOG_SECURITY | LOG_NOTICE, fmt, ##__VA_ARGS__) +#define AUDIT(fmt, ...) syslog(LOG_AUDIT | LOG_NOTICE, fmt, ##__VA_ARGS__) #endif /* CONFD_COMMON_H_ */