[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[Maxim Cournoyer]

Hi,

This series was motivated by investigations as to why the fail2ban would not
trigger bans although my SSH port was under constant brute force attacks.  It
turns out that it was because by default fail2ban consults /var/log/secure to
for the authentication logs, at least that's how our fail2ban package in Guix
behaves.

So this patch series does two things:

1. It adds a reload action, useful to test without rebooting the graphical
session.

2. It adds the missing auth.info log to /var/log/secure so that a fail2ban
sshd jail works out of the box on Guix System.

Thanks!

Maxim Cournoyer (4):
  services: syslog: Move configuration to /etc/syslog.conf.
  services: syslog: Add a reload action.
  services/syslog: Strip leading white space indent in syslog.conf.
  services: syslog: Log auth.info to /var/log/secure in default
    configuration.

 doc/guix.texi         |  12 ++++
 gnu/services/base.scm | 128 ++++++++++++++++++++++++++----------------
 2 files changed, 92 insertions(+), 48 deletions(-)


base-commit: 0fe2c78cac19acfb46c3bc365075293e51e0e5aa
-- 
2.39.2

[bug#62802] [PATCH 1/4] services: syslog: Move configuration to /etc/syslog.conf.

[Maxim Cournoyer]

Having the configuration live at a static location makes it possible to
hot-reload it.

* gnu/services/base.scm (syslog.conf): New variable.
(syslog-etc, syslog-shepherd-service): New procedures.
(syslog-service-type): Rewrite using the above new variable and procedures,
extending etc-service-type with its configuration file.
---

 gnu/services/base.scm | 61 ++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index e5c6bf5335..1ed874aa84 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -15,7 +15,7 @@
 ;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021 Hui Lu <luhuins@163.com>
-;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 muradm <mail@muradm.net>
 ;;; Copyright © 2022 Guillaume Le Vaillant <glv@posteo.net>
 ;;; Copyright © 2022 Justin Veilleux <terramorpha@cock.li>
@@ -1526,30 +1526,43 @@ (define-record-type* <syslog-configuration>
   (config-file          syslog-configuration-config-file
                         (default %default-syslog.conf)))
 
-(define syslog-service-type
-  (shepherd-service-type
-   'syslog
-   (lambda (config)
-     (define config-file
-       (syslog-configuration-config-file config))
+;;; Note: a static file name is used for syslog.conf so that the reload action
+;;; work as intended.
+(define syslog.conf "/etc/syslog.conf")
 
-     (shepherd-service
-      (documentation "Run the syslog daemon (syslogd).")
-      (provision '(syslogd))
-      (requirement '(user-processes))
-      (actions (list (shepherd-configuration-action config-file)))
-      (start #~(let ((spawn (make-forkexec-constructor
-                             (list #$(syslog-configuration-syslogd config)
-                                   "--rcfile" #$config-file)
-                             #:pid-file "/var/run/syslog.pid")))
-                 (lambda ()
-                   ;; Set the umask such that file permissions are #o640.
-                   (let ((mask (umask #o137))
-                         (pid  (spawn)))
-                     (umask mask)
-                     pid))))
-      (stop #~(make-kill-destructor))))
-   (syslog-configuration)
+(define (syslog-etc configuration)
+  (match-record configuration <syslog-configuration>
+    (config-file)
+    (list `(,(basename syslog.conf) ,config-file))))
+
+(define (syslog-shepherd-service config)
+  (define config-file
+    (syslog-configuration-config-file config))
+
+  (shepherd-service
+   (documentation "Run the syslog daemon (syslogd).")
+   (provision '(syslogd))
+   (requirement '(user-processes))
+   (actions (list (shepherd-configuration-action syslog.conf)))
+   (start #~(let ((spawn (make-forkexec-constructor
+                          (list #$(syslog-configuration-syslogd config)
+                                #$(string-append "--rcfile=" syslog.conf))
+                          #:pid-file "/var/run/syslog.pid")))
+              (lambda ()
+                ;; Set the umask such that file permissions are #o640.
+                (let ((mask (umask #o137))
+                      (pid  (spawn)))
+                  (umask mask)
+                  pid))))
+   (stop #~(make-kill-destructor))))
+
+(define syslog-service-type
+  (service-type
+   (name 'syslog)
+   (default-value (syslog-configuration))
+   (extensions (list (service-extension shepherd-root-service-type
+                                        (compose list syslog-shepherd-service))
+                     (service-extension etc-service-type syslog-etc)))
    (description "Run the syslog daemon, @command{syslogd}, which is
 responsible for logging system messages.")))
 

base-commit: 0fe2c78cac19acfb46c3bc365075293e51e0e5aa
-- 
2.39.2

[bug#62802] [PATCH 2/4] services: syslog: Add a reload action.

[Maxim Cournoyer]

* gnu/services/base.scm (syslog-service-type) [actions]: Add a reload action.
* doc/guix.texi (Base Services): Document it.
---

 doc/guix.texi         | 12 ++++++++++++
 gnu/services/base.scm | 16 +++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index acb6f0c2e1..70909917a5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18573,6 +18573,18 @@ Type of the service that runs the syslog daemon, whose value is a
 @code{<syslog-configuration>} object.
 @end defvar
 
+To have a modified @code{syslog-configuration} come into effect after
+reconfiguring your system, the @samp{reload} action should be preferred
+to restarting the service, as many services such as the login manager
+depend on it and would be restarted as well:
+
+@example
+# herd reload syslog
+@end example
+
+which will cause the running @command{syslogd} process to reload its
+configuration.
+
 @deftp {Data Type} syslog-configuration
 Data type representing the configuration of the syslog daemon.
 
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 1ed874aa84..db7a0bbc56 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1543,7 +1543,21 @@ (define config-file
    (documentation "Run the syslog daemon (syslogd).")
    (provision '(syslogd))
    (requirement '(user-processes))
-   (actions (list (shepherd-configuration-action syslog.conf)))
+   (actions
+    (list (shepherd-configuration-action syslog.conf)
+          (shepherd-action
+           (name 'reload)
+           (documentation "Reload the configuration file from disk.")
+           (procedure
+            #~(lambda (pid)
+                (if pid
+                    (begin
+                      (kill pid SIGHUP)
+                      (display #$(G_ "Service syslog has been asked to \
+reload its settings file.")))
+                    (display #$(G_ "Service syslog is not running."))))))))
+   ;; Note: a static file name is used for syslog.conf so that the reload
+   ;; action work as intended.
    (start #~(let ((spawn (make-forkexec-constructor
                           (list #$(syslog-configuration-syslogd config)
                                 #$(string-append "--rcfile=" syslog.conf))
-- 
2.39.2

[bug#62802] [PATCH 3/4] services/syslog: Strip leading white space indent in syslog.conf.

[Maxim Cournoyer]

This is a cosmetic change.

* gnu/services/base.scm (%default-syslog.conf): Add a comment referencing the
documentation.  Strip the extraneous leading trailing white space indent.
---

 gnu/services/base.scm | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index db7a0bbc56..0cde151e1a 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1491,31 +1491,34 @@ (define-deprecated (nscd-service #:optional (config (nscd-configuration)))
 Service Switch}, for an example."
   (service nscd-service-type config))
 
-;; Snippet adapted from the GNU inetutils manual.
+;;; Snippet adapted from the GNU inetutils manual.
 (define %default-syslog.conf
-  (plain-file "syslog.conf" "
-     # Log all error messages, authentication messages of
-     # level notice or higher and anything of level err or
-     # higher to the console.
-     # Don't log private authentication messages!
-     *.alert;auth.notice;authpriv.none      -/dev/console
+  (plain-file "syslog.conf" "\
+# See info '(inetutils) syslogd invocation' for the documentation
+# of the syslogd configuration syntax.
 
-     # Log anything (except mail) of level info or higher.
-     # Don't log private authentication messages!
-     *.info;mail.none;authpriv.none         -/var/log/messages
+# Log all error messages, authentication messages of
+# level notice or higher and anything of level err or
+# higher to the console.
+# Don't log private authentication messages!
+*.alert;auth.notice;authpriv.none      -/dev/console
 
-     # Log \"debug\"-level entries and nothing else.
-     *.=debug                               -/var/log/debug
+# Log anything (except mail) of level info or higher.
+# Don't log private authentication messages!
+*.info;mail.none;authpriv.none         -/var/log/messages
 
-     # Same, in a different place.
-     *.info;mail.none;authpriv.none         -/dev/tty12
+# Log \"debug\"-level entries and nothing else.
+*.=debug                               -/var/log/debug
 
-     # The authpriv file has restricted access.
-     # 'fsync' the file after each line (hence the lack of a leading dash).
-     authpriv.*                              /var/log/secure
+# Same, in a different place.
+*.info;mail.none;authpriv.none         -/dev/tty12
 
-     # Log all the mail messages in one place.
-     mail.*                                 -/var/log/maillog
+# The authpriv file has restricted access.
+# 'fsync' the file after each line (hence the lack of a leading dash).
+authpriv.*                              /var/log/secure
+
+# Log all the mail messages in one place.
+mail.*                                 -/var/log/maillog
 "))
 
 (define-record-type* <syslog-configuration>
-- 
2.39.2

[bug#62802] [PATCH 4/4] services: syslog: Log auth.info to /var/log/secure in default configuration.

[Maxim Cournoyer]

This causes authentication failures such as those generated by SSH brute force
attacks to appear in /var/log/secure, which is picked up by tools such as
fail2ban.

* gnu/services/base.scm (%default-syslog.conf): Add a auth.info selector for
the /var/log/secure log.

---

 gnu/services/base.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 0cde151e1a..282d36c8b1 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1515,7 +1515,9 @@ (define %default-syslog.conf
 
 # The authpriv file has restricted access.
 # 'fsync' the file after each line (hence the lack of a leading dash).
-authpriv.*                              /var/log/secure
+# Also include unprivileged auth logs of info or higher level
+# to conveniently gather the authentication data at the same place.
+authpriv.*;auth.info                    /var/log/secure
 
 # Log all the mail messages in one place.
 mail.*                                 -/var/log/maillog
-- 
2.39.2

[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[Ludovic Courtès]

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> This causes authentication failures such as those generated by SSH brute force
> attacks to appear in /var/log/secure, which is picked up by tools such as
> fail2ban.

Nice, go for it!

Ludo’.

bug#62802: [PATCH 0/4] Add reload action to syslog service.

[Maxim Cournoyer]

Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> This causes authentication failures such as those generated by SSH brute force
>> attacks to appear in /var/log/secure, which is picked up by tools such as
>> fail2ban.
>
> Nice, go for it!

Great, the change is now installed.  Thanks for the review!

-- 
Thanks,
Maxim

[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[Ludovic Courtès]

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Having the configuration live at a static location makes it possible to
> hot-reload it.
>
> * gnu/services/base.scm (syslog.conf): New variable.
> (syslog-etc, syslog-shepherd-service): New procedures.
> (syslog-service-type): Rewrite using the above new variable and procedures,
> extending etc-service-type with its configuration file.

I’m really not a fan of static configuration file names: you can never
be sure what config the service is using—compare this with the
unambiguous ‘--config=/gnu/store/…example.conf’.

Unfortunately there’s often no other option if we want to support live
reconfiguration—there’s only so much a signal can convey.

So I guess it’s a “weak accept” from me, because live reload is useful.

With the Shepherd in ‘master’, there’s a hook to change a service’s
“running value” so it should be possible to stop the previous process,
start a new one, and update the service’s running value (which is not
equivalent to SIGHUP, but maybe good enough for some cases).

A simpler approach might be run the service in a container with
/gnu/store/…conf mapped to a fixed location, and somehow update that
mapping as we go.  Food for thought!

Ludo’.

[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[Maxim Cournoyer]

Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Having the configuration live at a static location makes it possible to
>> hot-reload it.
>>
>> * gnu/services/base.scm (syslog.conf): New variable.
>> (syslog-etc, syslog-shepherd-service): New procedures.
>> (syslog-service-type): Rewrite using the above new variable and procedures,
>> extending etc-service-type with its configuration file.
>
> I’m really not a fan of static configuration file names: you can never
> be sure what config the service is using—compare this with the
> unambiguous ‘--config=/gnu/store/…example.conf’.

Right; although now if you aren't sure what is used you can 'reload' it,
eh :-).

> Unfortunately there’s often no other option if we want to support live
> reconfiguration—there’s only so much a signal can convey.
>
> So I guess it’s a “weak accept” from me, because live reload is useful.

OK!

> With the Shepherd in ‘master’, there’s a hook to change a service’s
> “running value” so it should be possible to stop the previous process,
> start a new one, and update the service’s running value (which is not
> equivalent to SIGHUP, but maybe good enough for some cases).

Wouldn't that be equivalent to restarting the service?  I wasn't aware
of the new hook facility, I'll have to read on it, thanks!

> A simpler approach might be run the service in a container with
> /gnu/store/…conf mapped to a fixed location, and somehow update that
> mapping as we go.  Food for thought!

Interesting idea... although it'd only be compatible with Linux and I
dislike writing special cases in services (or anywhere if I can help
it).

-- 
Thanks,
Maxim

[bug#62802] [PATCH 0/4] Add reload action to syslog service.

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I’m really not a fan of static configuration file names: you can never
>> be sure what config the service is using—compare this with the
>> unambiguous ‘--config=/gnu/store/…example.conf’.
>
> Right; although now if you aren't sure what is used you can 'reload' it,
> eh :-).

True.  :-)  The other issue is that that makes it impossible to run
several instances of the service (not a problem for syslogd of course,
but could be an issue elsewhere).

>> With the Shepherd in ‘master’, there’s a hook to change a service’s
>> “running value” so it should be possible to stop the previous process,
>> start a new one, and update the service’s running value (which is not
>> equivalent to SIGHUP, but maybe good enough for some cases).
>
> Wouldn't that be equivalent to restarting the service?  I wasn't aware
> of the new hook facility, I'll have to read on it, thanks!

There’s nothing to read :-) and it wasn’t designed with that use case in
mind, but we’ll see.

>> A simpler approach might be run the service in a container with
>> /gnu/store/…conf mapped to a fixed location, and somehow update that
>> mapping as we go.  Food for thought!
>
> Interesting idea... although it'd only be compatible with Linux and I
> dislike writing special cases in services (or anywhere if I can help
> it).

Yeah.

Ludo’.