Audit update #1199
Audit update #1199
Conversation
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
ikerexxe
force-pushed
the
audit-update
branch
from
3f46a30 to
79bb0f5
Compare
|
@stevegrubb before I take this out of draft status, are there any other distributions besides Fedora/CentOS/RHEL that might be affected by these changes? I'd like to ping the maintainers of those distributions to review these code changes. |
| return; | ||
| } | ||
| len = strnlen(grp, sizeof(enc_group)/2); | ||
| if (audit_value_needs_encoding(grp, len)) { |
There was a problem hiding this comment.
By default, we don't allow names that would need encoding. This reminds me that we have #1158.
I will add " to the list of strictly disallowed characters, and then we don't need to encode anything here.
There was a problem hiding this comment.
That makes sense. If it's fine for you we leave it like this for now, and if I see that #1158 mergse before this PR I will update the code.
There was a problem hiding this comment.
Yes, that's fine. Thanks!
| #ifndef AUDIT_GRP_MGMT | ||
| #define AUDIT_GRP_MGMT 1132 /* Group account was modified */ | ||
| #endif | ||
| #ifndef AUDIT_GRP_CHAUTHTOK | ||
| #define AUDIT_GRP_CHAUTHTOK 1133 /* Group account password was changed */ | ||
| #endif |
There was a problem hiding this comment.
Any reason for those specific values?
There was a problem hiding this comment.
They come from audit-records.h and probably they weren't available in all distributions when this patch was originally created (that happened years ago). I'm fine with removing this part of the patch as probably they will be available in all distributions.
There was a problem hiding this comment.
Yes, please try removing them, and if builds everywhere, let's forget about them (and if not, reintroduce them).
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
Signed-off-by: Iker Pedrosa <[email protected]>
ikerexxe
force-pushed
the
audit-update
branch
from
79bb0f5 to
6d9391c
Compare
| #ifndef AUDIT_GRP_MGMT | ||
| #define AUDIT_GRP_MGMT 1132 /* Group account was modified */ | ||
| #endif | ||
| #ifndef AUDIT_GRP_CHAUTHTOK | ||
| #define AUDIT_GRP_CHAUTHTOK 1133 /* Group account password was changed */ | ||
| #endif |
There was a problem hiding this comment.
Yes, please try removing them, and if builds everywhere, let's forget about them (and if not, reintroduce them).
| audit_logger (AUDIT_USER_ACCT, log_get_progname(), | ||
| audit_logger (AUDIT_GRP_MGMT, log_get_progname(), |
There was a problem hiding this comment.
Regarding
You could move the filename in the subject to the prefix:
lib/cleanup_group.c: Update audit messages
Also, could you add some little explanation in the commit message of what this update is about?
There was a problem hiding this comment.
Regarding
You could move the filename in the subject to the prefix:
lib/cleanup_group.c: Update audit messages
Or will you squash the commits? (I think it would make sense to squash them.)
|
@ikerexxe Most major distributions enable auditing. So, they are all affected. That said, they are all broken except Fedora. This fixes everyone and Fedora can drop it's patch. |
This is a set of changes that Fedora introduced in a downstream patch. The idea is to converge upstream and downstream versions to reduce the maintenance burden. The patch was created a long time ago and we have been adapting it, but it may not be up to date with current upstream code standards, if so, please inform me so I can update the patch.