Skip to content

Commit 8a09b3e

Browse files
committed
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 <[email protected]>
1 parent 7d95da7 commit 8a09b3e

File tree

5 files changed

+77
-45
lines changed

5 files changed

+77
-45
lines changed

patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC.patch renamed to patches/sysrepo/2.10.1/0007-Introduce-new-log-level-SEC-for-audit-trails.patch

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,74 @@
1-
From da765b90bca45b91f72fd6525e680040eebd2d4b Mon Sep 17 00:00:00 2001
1+
From d128686fb15833e815ac3dd04bd87d3725c881ac Mon Sep 17 00:00:00 2001
22
From: Joachim Wiberg <[email protected]>
33
Date: Wed, 21 Aug 2024 16:00:35 +0200
4-
Subject: [PATCH 7/9] Introduce new log level [SEC]
4+
Subject: [PATCH 7/9] Introduce new log level [SEC] for audit trails
55
Organization: Addiva Elektronik
66

77
This adds a new log level for security and audit trail related log
8-
messages. E.g., nacm user applied a change, copied a ds, etc.
8+
messages, see LOG_AUDIT defined in RFC5424. E.g., nacm user applied
9+
a change, copied a datastore, etc.
910

1011
The new log level is added last to not affect the advertised command
1112
line log levels. A security notice has higher actual priorty than
1213
DBG, of course, so we remap it to WRN. The construct allows us to
1314
have another [label] than [WRN], which might otherwise be read as
1415
a bug/problem rather than just a high-priority-notification.
1516

17+
When logging to syslog() we let delegate labling and filtering to the
18+
system log daemon, dropping any [SEVERITY] prefix. Also, \n is most
19+
often dropped by log daemons.
20+
1621
Signed-off-by: Joachim Wiberg <[email protected]>
1722
---
18-
src/log.c | 5 +++++
19-
src/log.h | 1 +
20-
src/sysrepo_types.h | 3 ++-
21-
tests/tcommon.c | 3 +++
22-
4 files changed, 11 insertions(+), 1 deletion(-)
23+
src/log.c | 18 +++++++++++++++++-
24+
src/log.h | 1 +
25+
src/sysrepo_types.h | 3 ++-
26+
tests/tcommon.c | 3 +++
27+
4 files changed, 23 insertions(+), 2 deletions(-)
2328

2429
diff --git a/src/log.c b/src/log.c
25-
index e15055ac..b89ffacf 100644
30+
index e15055ac..25eab8fa 100644
2631
--- a/src/log.c
2732
+++ b/src/log.c
28-
@@ -122,6 +122,11 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg)
33+
@@ -30,6 +30,10 @@
34+
35+
#include "config.h"
36+
37+
+#ifndef LOG_AUDIT
38+
+#define LOG_AUDIT (13<<3) /* Log audit, for audit trails */
39+
+#endif
40+
+
41+
sr_log_level_t sr_stderr_ll = SR_LL_NONE; /**< stderr log level */
42+
sr_log_level_t sr_syslog_ll = SR_LL_NONE; /**< syslog log level */
43+
int syslog_open; /**< Whether syslog was opened */
44+
@@ -122,6 +126,11 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg)
2945
priority = LOG_INFO;
3046
severity = "INF";
3147
break;
3248
+ case SR_LL_SEC:
33-
+ priority = LOG_WARNING;
49+
+ priority = LOG_AUDIT | LOG_NOTICE;
3450
+ severity = "SEC";
3551
+ ll = SR_LL_WRN; /* remap to higher level. */
3652
+ break;
3753
case SR_LL_DBG:
3854
priority = LOG_DEBUG;
3955
severity = "DBG";
56+
@@ -138,7 +147,14 @@ sr_log_msg(int plugin, sr_log_level_t ll, const char *msg)
57+
58+
/* syslog logging */
59+
if (ll <= sr_syslog_ll) {
60+
- syslog(priority | (plugin ? LOG_DAEMON : 0), "[%s] %s\n", severity, msg);
61+
+ int facility;
62+
+
63+
+ if (priority & ~LOG_PRIMASK)
64+
+ facility = 0; /* audit */
65+
+ else
66+
+ facility = plugin ? LOG_DAEMON : 0;
67+
+
68+
+ syslog(facility | priority, "%s", msg);
69+
}
70+
71+
/* logging callback */
4072
diff --git a/src/log.h b/src/log.h
4173
index d7e65b88..8722e51d 100644
4274
--- a/src/log.h

patches/sysrepo/2.10.1/0008-Add-audit-trail-for-high-priority-system-changes.patch

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
From 7e49f394e0afedc0259d4364f5a9a83296fe2b72 Mon Sep 17 00:00:00 2001
1+
From b5db2b36c06d28918f0d00dda945f7e943b470af Mon Sep 17 00:00:00 2001
22
From: Joachim Wiberg <[email protected]>
33
Date: Wed, 21 Aug 2024 16:04:43 +0200
44
Subject: [PATCH 8/9] Add audit trail for high priority system changes
@@ -18,7 +18,7 @@ Signed-off-by: Joachim Wiberg <[email protected]>
1818
1 file changed, 12 insertions(+)
1919

2020
diff --git a/src/sysrepo.c b/src/sysrepo.c
21-
index 86d694e5..c7b97e53 100644
21+
index 86d694e5..dcea51e8 100644
2222
--- a/src/sysrepo.c
2323
+++ b/src/sysrepo.c
2424
@@ -3946,6 +3946,9 @@ store:
@@ -45,7 +45,7 @@ index 86d694e5..c7b97e53 100644
4545
}
4646
}
4747

48-
+ if (session->nacm_user)
48+
+ if (session->nacm_user && src_datastore != SR_DS_CANDIDATE)
4949
+ SR_LOG_SEC("user \"%s\" copied %s to %s", session->nacm_user, sr_ds2str(src_datastore), sr_ds2str(session->ds));
5050
+
5151
cleanup:

patches/sysrepo/2.10.1/0009-On-error-in-sr_shmsub_listen_thread-exit-process.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
From 5c4df9f535f6fbdc6dc0573a47c47be2d02f1774 Mon Sep 17 00:00:00 2001
1+
From 13d54101eb2c3506237cebc5be99edef2ad669cd Mon Sep 17 00:00:00 2001
22
From: Joachim Wiberg <[email protected]>
33
Date: Fri, 23 Aug 2024 12:22:06 +0200
44
Subject: [PATCH 9/9] On error in sr_shmsub_listen_thread(), exit process

src/confd/src/ietf-system.c

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -607,9 +607,9 @@ static void add_group(const char *user, const char *group)
607607
return; /* already group member */
608608

609609
if (systemf("adduser %s %s", user, group))
610-
SECURITY("Failed giving user %s UNIX %s permissions.", user, group);
610+
AUDIT("Failed giving user \"%s\" UNIX %s permissions.", user, group);
611611
else
612-
SECURITY("User %s added to UNIX %s group.", user, group);
612+
AUDIT("User \"%s\" added to UNIX \"%s\" group.", user, group);
613613
}
614614

615615
static void del_group(const char *user, const char *group)
@@ -620,9 +620,9 @@ static void del_group(const char *user, const char *group)
620620
return; /* not member of group */
621621

622622
if (systemf("delgroup %s %s", user, group))
623-
SECURITY("Failed removing user %s from UNIX %s group.", user, group);
623+
AUDIT("Failed removing user \"%s\" from UNIX \"%s\" group.", user, group);
624624
else
625-
SECURITY("User %s removed from UNIX %s group.", user, group);
625+
AUDIT("User \"%s\" removed from UNIX \"%s\" group.", user, group);
626626
}
627627

628628
/* 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
786786
char **args;
787787
int err;
788788

789-
DEBUG("Adding new user %s, cleaning up any stale group.", name);
789+
DEBUG("Adding new user \"%s\", cleaning up any stale group.", name);
790790
systemf("delgroup %s 2>/dev/null", name);
791791

792792
/* reusing existing uid:gid from $HOME */
@@ -840,15 +840,15 @@ static int sys_add_user(sr_session_ctx_t *sess, char *name)
840840
/* Verify IDs aren't already used, like BusyBox adduser */
841841
if (getpwuid(st.st_uid) || getgrgid(st.st_uid) || getgrgid(st.st_gid)) {
842842
/* Exists but owned by someone else. */
843-
SECURITY("Failed mapping user %s to /home/%s, uid:gid (%d:%d) already exists.",
843+
AUDIT("Failed mapping user \"%s\" to /home/%s, uid:gid (%d:%d) already exists.",
844844
name, name, st.st_uid, st.st_gid);
845845
err = sys_call_adduser(sess, name, 0, 0);
846846
} else {
847-
SECURITY("Reusing uid:gid %d:%d and /home/%s for new user %s",
847+
AUDIT("Reusing uid:gid %d:%d and /home/%s for new user \"%s\"",
848848
st.st_uid, st.st_gid, name, name);
849849
err = sys_call_adduser(sess, name, st.st_uid, st.st_gid);
850850
if (err) {
851-
SECURITY("Failed reusing uid:gid from /home/%s, retrying create user ...", name);
851+
AUDIT("Failed reusing uid:gid from /home/%s, retrying create user ...", name);
852852
err = sys_call_adduser(sess, name, 0, 0);
853853
} else
854854
reused = true;
@@ -857,11 +857,11 @@ static int sys_add_user(sr_session_ctx_t *sess, char *name)
857857
err = sys_call_adduser(sess, name, 0, 0);
858858

859859
if (err) {
860-
SECURITY("Failed creating new user \"%s\"", name);
860+
AUDIT("Failed creating new user \"%s\"", name);
861861
return SR_ERR_SYS;
862862
}
863863

864-
SECURITY("User \"%s\" created%s.", name, reused ? ", mapped to existing home directory" : "");
864+
AUDIT("User \"%s\" created%s.", name, reused ? ", mapped to existing home directory" : "");
865865

866866
/*
867867
* 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)
930930

931931
if (!strcmp(pw->pw_name, user)) {
932932
if (strcmp(pw->pw_shell, shell))
933-
NOTE("Updating login shell for user %s to %s", user, shell);
933+
AUDIT("Updating login shell for user \"%s\" to %s", user, shell);
934934

935935
upw = *pw;
936936
upw.pw_shell = (char *)shell;
@@ -953,7 +953,7 @@ static int set_shell(const char *user, const char *shell)
953953
if (fp)
954954
fclose(fp);
955955
endpwent();
956-
ERRNO("Failed setting user %s shell %s", user, shell);
956+
ERRNO("Failed setting user \"%s\" shell %s", user, shell);
957957

958958
return -1;
959959
}
@@ -971,7 +971,7 @@ static int set_password(const char *user, const char *hash, bool lock)
971971

972972
fp = fopen(_PATH_SHADOW "+", "w");
973973
if (!fp) {
974-
ERRNO("Failed opening %s+ for %s", _PATH_SHADOW, user);
974+
ERRNO("Failed opening %s+ for user \"%s\"", _PATH_SHADOW, user);
975975
goto fail;
976976
}
977977
fd = fileno(fp);
@@ -1016,7 +1016,7 @@ static int set_password(const char *user, const char *hash, bool lock)
10161016
endspent();
10171017
ulckpwdf();
10181018
exit:
1019-
SECURITY("Failed setting password for %s", user);
1019+
AUDIT("Failed setting password for user \"%s\"", user);
10201020

10211021
return -1;
10221022
}
@@ -1045,7 +1045,7 @@ static const char *is_valid_hash(struct confd *confd, const char *user, const ch
10451045

10461046
pwd = json_object_get(confd->root, "factory-password-hash");
10471047
if (!json_is_string(pwd)) {
1048-
EMERG("Cannot find factory-default password hash for user %s!", user);
1048+
EMERG("Cannot find factory-default password hash for user \"%s\"!", user);
10491049
return NULL;
10501050
}
10511051

@@ -1072,7 +1072,7 @@ static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *conf
10721072
assert(change->new);
10731073

10741074
if (change->new->type != SR_STRING_T) {
1075-
SECURITY("Internal error, expected user %s password to be string type.", user);
1075+
AUDIT("Internal error, expected user \"%s\" password to be string type.", user);
10761076
err = SR_ERR_INTERNAL;
10771077
break;
10781078
}
@@ -1090,17 +1090,17 @@ static sr_error_t handle_sr_passwd_update(sr_session_ctx_t *, struct confd *conf
10901090
if (set_password(user, hash, lock))
10911091
err = SR_ERR_SYS;
10921092
else if (lock)
1093-
NOTE("User account %s locked.", user);
1093+
NOTE("User account \"%s\" locked.", user);
10941094
else if (!strcmp(hash, "*"))
1095-
NOTE("Password login disabled for user %s", user);
1095+
NOTE("Password login disabled for user \"%s\"", user);
10961096
else
1097-
SECURITY("Password updated for user %s", user);
1097+
AUDIT("Password updated for user \"%s\"", user);
10981098
break;
10991099
case SR_OP_DELETED:
11001100
if (set_password(user, "*", false))
11011101
err = SR_ERR_SYS;
11021102
else
1103-
NOTE("Password login disabled for user %s", user);
1103+
NOTE("Password login disabled for user \"%s\"", user);
11041104
break;
11051105
case SR_OP_MOVED:
11061106
break;
@@ -1125,10 +1125,10 @@ static sr_error_t handle_sr_shell_update(sr_session_ctx_t *sess, struct confd *c
11251125

11261126
shell = sys_find_usable_shell(sess, (char *)user, is_admin_user(sess, user));
11271127
if (set_shell(user, shell)) {
1128-
SECURITY("Failed updating shell to %s for user %s", shell, user);
1128+
AUDIT("Failed updating shell to %s for user \"%s\"", shell, user);
11291129
err = SR_ERR_SYS;
11301130
} else {
1131-
SECURITY("Login shell updated for user %s", user);
1131+
AUDIT("Login shell updated for user \"%s\"", user);
11321132
err = SR_ERR_OK;
11331133
}
11341134
free(shell);
@@ -1148,7 +1148,7 @@ static sr_error_t check_sr_user_update(sr_session_ctx_t *, struct confd *, struc
11481148

11491149
name = sr_xpath_key_value(val->xpath, "user", "name", &state);
11501150
if (!is_valid_username(name)) {
1151-
SECURITY("Invalid username \"%s\"", name);
1151+
AUDIT("Invalid username \"%s\"", name);
11521152
return SR_ERR_VALIDATION_FAILED;
11531153
}
11541154
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
12321232

12331233
fp = fopenf("w", "/var/run/sshd/%s.keys", username);
12341234
if (!fp) {
1235-
ERROR("failed opening %s authorized_keys file: %s", username, strerror(errno));
1235+
ERROR("failed opening user \"%s\" authorized_keys file: %s", username, strerror(errno));
12361236
continue;
12371237
}
12381238

@@ -1361,7 +1361,7 @@ static sr_error_t change_auth_done(struct confd *confd, sr_session_ctx_t *sessio
13611361

13621362
err = generate_auth_keys(session, XPATH_AUTH_"/user//.");
13631363
if (err) {
1364-
SECURITY("failed saving authorized-key data.");
1364+
AUDIT("failed saving authorized-key data.");
13651365
goto cleanup;
13661366
}
13671367

@@ -1432,11 +1432,11 @@ static int change_nacm(sr_session_ctx_t *session, uint32_t sub_id, const char *m
14321432
for (size_t i = 0; i < user_count; i++) {
14331433
const char *user = users[i].data.string_val;
14341434
bool is_admin = is_admin_user(session, user);
1435-
char *shell;
1435+
const char *shell;
14361436

14371437
shell = sys_find_usable_shell(session, (char *)user, is_admin);
14381438
if (set_shell(user, shell))
1439-
SECURITY("Failed adjusting shell for user %s", user);
1439+
AUDIT("Failed adjusting shell for user \"%s\"", user);
14401440

14411441
if (is_admin)
14421442
add_group(user, "wheel");

src/libsrx/src/common.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
extern int debug;
1212

1313
/* In IETF referred to LOG_AUDIT */
14-
#ifndef LOG_SECURITY
15-
#define LOG_SECURITY (1 << 13)
14+
#ifndef LOG_AUDIT
15+
#define LOG_AUDIT (13<<3) /* Log audit, for audit trails */
1616
#endif
1717

1818
#ifndef HAVE_VASPRINTF
@@ -33,6 +33,6 @@ int asprintf(char **strp, const char *fmt, ...);
3333
#define EMERG(fmt, ...) syslog(LOG_EMERG, fmt, ##__VA_ARGS__)
3434
#define ERROR(fmt, ...) syslog(LOG_ERR, fmt, ##__VA_ARGS__)
3535
#define ERRNO(fmt, ...) syslog(LOG_ERR, fmt ": %s", ##__VA_ARGS__, strerror(errno))
36-
#define SECURITY(fmt, ...) syslog(LOG_SECURITY | LOG_NOTICE, fmt, ##__VA_ARGS__)
36+
#define AUDIT(fmt, ...) syslog(LOG_AUDIT | LOG_NOTICE, fmt, ##__VA_ARGS__)
3737

3838
#endif /* CONFD_COMMON_H_ */

0 commit comments

Comments
 (0)