| Message ID | a545ae09-1801-3d71-ef4c-a490dcc39cdf@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#54309] services: auditd: use exclusive log directory for auditd | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
fesoj000 schreef op za 19-03-2022 om 12:34 [+0100]: > + (let* ((previous-umask (umask #o077))) > + (mkdir-p "/var/log/audit") > + (umask previous-umask))))) I cannot recommend this, what if 'mkdir-p' throws an exception? That might cause problems. Or maybe not, but it would require some analysis that can be avoided with 'mkdir-p/perms'. Greetings, Maxime.
On 3/20/22 12:13 AM, Maxime Devos wrote: > fesoj000 schreef op za 19-03-2022 om 12:34 [+0100]: >> + (let* ((previous-umask (umask #o077))) >> + (mkdir-p "/var/log/audit") >> + (umask previous-umask))))) > > I cannot recommend this, what if 'mkdir-p' throws an exception? > That might cause problems. Or maybe not, but it would require > some analysis that can be avoided with 'mkdir-p/perms'. Hm, but i still have to set umask to prevent TOCTOU, the implementation of 'mkdir-p/perms' does not take care of that. BR
fesoj000 schreef op zo 20-03-2022 om 21:22 [+0100]: > > I cannot recommend this, what if 'mkdir-p' throws an exception? > > That might cause problems. Or maybe not, but it would require > > some analysis that can be avoided with 'mkdir-p/perms'. > Hm, but i still have to set umask to prevent TOCTOU, the > implementation of 'mkdir-p/perms' does not take care of that. mkdir-p/perms could be modified to take care of that. If that is done, then other users of mkdir-p/perms would benefit as well. To implement this, I recommend using the prodecures from <https://lists.gnu.org/archive/html/guile-devel/2021-11/msg00005.html> -- that patch was written to remove the TOCTOU! Greetings, Maxime.
Maxime Devos schreef op zo 20-03-2022 om 21:30 [+0100]: > fesoj000 schreef op zo 20-03-2022 om 21:22 [+0100]: > > > I cannot recommend this, what if 'mkdir-p' throws an exception? > > > That might cause problems. Or maybe not, but it would require > > > some analysis that can be avoided with 'mkdir-p/perms'. > > Hm, but i still have to set umask to prevent TOCTOU, the > > implementation of 'mkdir-p/perms' does not take care of that. > > mkdir-p/perms could be modified to take care of that. > If that is done, then other users of mkdir-p/perms would benefit as > well. > > To implement this, I recommend using the prodecures from > <https://lists.gnu.org/archive/html/guile-devel/2021-11/msg00005.html > > > -- that patch was written to remove the TOCTOU! It seems to be ignored so far upstream, so I'll look into defining a Guile package variant that can be used by the activation code. Greetings, Maxime.
diff --git a/gnu/services/auditd.scm b/gnu/services/auditd.scm index abde811f51..e9e19c61be 100644 --- a/gnu/services/auditd.scm +++ b/gnu/services/auditd.scm @@ -31,10 +31,15 @@ (define-module (gnu services auditd) %default-auditd-configuration-directory)) (define auditd.conf - (plain-file "auditd.conf" "log_file = /var/log/audit.log\nlog_format = \ -ENRICHED\nfreq = 1\nspace_left = 5%\nspace_left_action = \ -syslog\nadmin_space_left_action = ignore\ndisk_full_action = \ -ignore\ndisk_error_action = syslog\n")) + (plain-file "auditd.conf" "\ +log_format = ENRICHED +freq = 1 +space_left = 5% +space_left_action = syslog +admin_space_left_action = ignore +disk_full_action = ignore +disk_error_action = syslog +")) (define %default-auditd-configuration-directory (computed-file "auditd" @@ -50,6 +55,16 @@ (define-record-type* <auditd-configuration> (default audit)) (configuration-directory auditd-configuration-configuration-directory)) ; file-like +(define (auditd-activation config) + (with-imported-modules '((guix build utils)) + #~(begin + (use-modules (guix build utils)) + ;; make sure /var/log exists before changing umask + (mkdir-p "/var/log") + (let* ((previous-umask (umask #o077))) + (mkdir-p "/var/log/audit") + (umask previous-umask))))) + (define (auditd-shepherd-service config) (let* ((audit (auditd-configuration-audit config)) (configuration-directory (auditd-configuration-configuration-directory config))) @@ -67,7 +82,9 @@ (define auditd-service-type (extensions (list (service-extension shepherd-root-service-type - auditd-shepherd-service))) + auditd-shepherd-service) + (service-extension activation-service-type + auditd-activation))) (default-value (auditd-configuration (configuration-directory %default-auditd-configuration-directory)))))