GNU bug report logs - #57473
[PATCH] gnu: fail2ban-service-type: Fix field name serialization.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57473 in the body.
You can then email your comments to 57473 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: fail2ban-service-type: Fix field name serialization.
Date: Mon, 29 Aug 2022 12:42:05 +0300

* gnu/services/security.scm: Wrong condition in string-filter
function in fail2ban-jail-configuration-serialize-field-name.
---
 gnu/services/security.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 1e0360c07f..9ab23ffb46 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -127,7 +127,7 @@ (define (fail2ban-jail-configuration-serialize-field-name name)
           (string-append "bantime." (substring name 9))))
         ((string-contains name "-")
          (fail2ban-jail-configuration-serialize-field-name
-          (string-filter (lambda (c) (equal? c #\-)) name)))
+          (string-filter (lambda (c) (not (equal? c #\-))) name)))
         (else name)))
 
 (define (fail2ban-jail-configuration-serialize-string field-name value)
-- 
2.37.2

Message #8 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: 57473 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 1/4] gnu: fail2ban-service-type: Fix field name
 serialization.
Date: Mon, 29 Aug 2022 16:51:18 +0300

* gnu/services/security.scm: Wrong condition in string-filter
function in fail2ban-jail-configuration-serialize-field-name.
---
 gnu/services/security.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 1e0360c07f..9ab23ffb46 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -127,7 +127,7 @@ (define (fail2ban-jail-configuration-serialize-field-name name)
           (string-append "bantime." (substring name 9))))
         ((string-contains name "-")
          (fail2ban-jail-configuration-serialize-field-name
-          (string-filter (lambda (c) (equal? c #\-)) name)))
+          (string-filter (lambda (c) (not (equal? c #\-))) name)))
         (else name)))
 
 (define (fail2ban-jail-configuration-serialize-string field-name value)
-- 
2.37.2

Message #11 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: 57473 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 2/4] gnu: fail2ban-service-type: Improve jail configuration
 serialization.
Date: Mon, 29 Aug 2022 16:51:19 +0300

* gnu/services/security.scm (fail2ban-jail-configuration)
[name]: Should use empty-serializer.
---
 gnu/services/security.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 9ab23ffb46..7f66c462f3 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -194,7 +194,8 @@ (define-maybe fail2ban-jail-filter-configuration (prefix fail2ban-jail-configura
 (define-configuration fail2ban-jail-configuration
   (name
    string
-   "Required name of this jail configuration.")
+   "Required name of this jail configuration."
+   empty-serializer)
   (enabled?
    (boolean #t)
    "Whether this jail is enabled.")
-- 
2.37.2

Message #14 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: 57473 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enabled? field.
Date: Mon, 29 Aug 2022 16:51:20 +0300

* gnu/services/security.scm (fail2ban-jail-configuration)
[enabled?]: Should be maybe-boolean, which is how normaly fail2ban
is configured. This also allows sections without enabled field
like @samp{[DEFAULT]}.
* doc/guix.texi: Type of enabled? is maybe-boolean.
---
 doc/guix.texi             | 2 +-
 gnu/services/security.scm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 7199ff6bc2..ebfc8fb3fa 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -36656,7 +36656,7 @@ Available @code{fail2ban-jail-configuration} fields are:
 @item @code{name} (type: string)
 Required name of this jail configuration.
 
-@item @code{enabled?} (default: @code{#t}) (type: boolean)
+@item @code{enabled?} (type: maybe-boolean)
 Whether this jail is enabled.
 
 @item @code{backend} (type: maybe-symbol)
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 7f66c462f3..3283a582f2 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -197,7 +197,7 @@ (define-configuration fail2ban-jail-configuration
    "Required name of this jail configuration."
    empty-serializer)
   (enabled?
-   (boolean #t)
+   maybe-boolean
    "Whether this jail is enabled.")
   (backend
    maybe-symbol
-- 
2.37.2

Message #17 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: 57473 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 4/4] gnu: fail2ban-service-type: Fix documentation typos.
Date: Mon, 29 Aug 2022 16:51:21 +0300

---
 doc/guix.texi             | 6 +++---
 gnu/services/security.scm | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ebfc8fb3fa..25eed72d30 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -36555,7 +36555,7 @@ is an example of a basic, explicit configuration:
              (list
               (fail2ban-jail-configuration
                (name "sshd")
-               (enabled #t))))))
+               (enabled? #t))))))
   ;; There is no implicit dependency on an actual SSH
   ;; service, so you need to provide one.
   (service openssh-service-type))
@@ -36580,7 +36580,7 @@ For example:
     openssh-service-type
     (fail2ban-jail-configuration
      (name "sshd")
-     (enabled #t)))
+     (enabled? #t)))
    (openssh-configuration ...))))
 @end lisp
 @end deffn
@@ -36660,7 +36660,7 @@ Required name of this jail configuration.
 Whether this jail is enabled.
 
 @item @code{backend} (type: maybe-symbol)
-Backend to use to detect changes in the @code{ogpath}.  The default is
+Backend to use to detect changes in the @code{log-path}.  The default is
 'auto.  To consult the defaults of the jail configuration, refer to the
 @file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package.
 
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 3283a582f2..949f332333 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -201,7 +201,7 @@ (define-configuration fail2ban-jail-configuration
    "Whether this jail is enabled.")
   (backend
    maybe-symbol
-   "Backend to use to detect changes in the @code{ogpath}.  The default is
+   "Backend to use to detect changes in the @code{log-path}.  The default is
 'auto.  To consult the defaults of the jail configuration, refer to the
 @file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
 fail2ban-jail-configuration-serialize-backend)
-- 
2.37.2

Message #20 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: muradm <mail <at> muradm.net>
Cc: 57473 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enabled?
 field.
Date: Mon, 29 Aug 2022 16:56:46 -0400

Hi,

muradm <mail <at> muradm.net> writes:

> * gnu/services/security.scm (fail2ban-jail-configuration)
> [enabled?]: Should be maybe-boolean, which is how normaly fail2ban
> is configured. This also allows sections without enabled field
> like @samp{[DEFAULT]}.
> * doc/guix.texi: Type of enabled? is maybe-boolean.
> ---
>  doc/guix.texi             | 2 +-
>  gnu/services/security.scm | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 7199ff6bc2..ebfc8fb3fa 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -36656,7 +36656,7 @@ Available @code{fail2ban-jail-configuration} fields are:
>  @item @code{name} (type: string)
>  Required name of this jail configuration.
>  
> -@item @code{enabled?} (default: @code{#t}) (type: boolean)
> +@item @code{enabled?} (type: maybe-boolean)
>  Whether this jail is enabled.
>  
>  @item @code{backend} (type: maybe-symbol)
> diff --git a/gnu/services/security.scm b/gnu/services/security.scm
> index 7f66c462f3..3283a582f2 100644
> --- a/gnu/services/security.scm
> +++ b/gnu/services/security.scm
> @@ -197,7 +197,7 @@ (define-configuration fail2ban-jail-configuration
>     "Required name of this jail configuration."
>     empty-serializer)
>    (enabled?
> -   (boolean #t)
> +   maybe-boolean

I made this change willingly, because it doesn't make sense to me to
define a jail configuration that is disabled by default.

Does that make sense?

Thanks,

Maxim

Message #23 received at 57473 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 57473 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enabled?
 field.
Date: Tue, 30 Aug 2022 09:42:16 +0300

[Message part 1 (text/plain, inline)]

Hi,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi,
>
> muradm <mail <at> muradm.net> writes:
>
>> * gnu/services/security.scm (fail2ban-jail-configuration)
>> [enabled?]: Should be maybe-boolean, which is how normaly 
>> fail2ban
>> is configured. This also allows sections without enabled field
>> like @samp{[DEFAULT]}.
>> * doc/guix.texi: Type of enabled? is maybe-boolean.
>> ---

[...]

>> @@ -197,7 +197,7 @@ (define-configuration 
>> fail2ban-jail-configuration
>>     "Required name of this jail configuration."
>>     empty-serializer)
>>    (enabled?
>> -   (boolean #t)
>> +   maybe-boolean
>
> I made this change willingly, because it doesn't make sense to 
> me to
> define a jail configuration that is disabled by default.
>
> Does that make sense?

When I was implementing configuration, I was thinking the same.

However, while trying to find sensible or hardcoded defaults,
looking at code and usage examples, I found that configuration
of fail2ban is quite loose, than one can anticipiate. Variables
can be overriden from many places, so absence of variable also
becomes as important. So I decided to opt-in for most fields
as maybe-.

IMHO, this is why they had to make --d/--dp flags to
dump effective configuration :)

I actually mention that in the docs, where it says that
unspecified values are not serialized in favor of whatever
are defaults and/or values specified elsewhere. Which are
basically hard to track.

And finally there is a [DEFAULT] section which applies to
all jails. But it is not a jail by itself. So if there is
"enabled = false" or "enabled = true", fail2ban fails to
start, because of unexpected variable for that section.

I hope it is clear enough.

Thanks in advance,
muradm

>
> Thanks,
>
> Maxim

[signature.asc (application/pgp-signature, inline)]

Message #28 received at 57473-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: muradm <mail <at> muradm.net>
Cc: 57473-done <at> debbugs.gnu.org
Subject: Re: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enabled?
 field.
Date: Thu, 01 Sep 2022 09:03:43 -0400

Hi muradm,

muradm <mail <at> muradm.net> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
>
>> Hi,
>>
>> muradm <mail <at> muradm.net> writes:
>>
>>> * gnu/services/security.scm (fail2ban-jail-configuration)
>>> [enabled?]: Should be maybe-boolean, which is how normaly fail2ban
>>> is configured. This also allows sections without enabled field
>>> like @samp{[DEFAULT]}.
>>> * doc/guix.texi: Type of enabled? is maybe-boolean.
>>> ---
>
> [...]
>
>>> @@ -197,7 +197,7 @@ (define-configuration
>>> fail2ban-jail-configuration
>>>     "Required name of this jail configuration."
>>>     empty-serializer)
>>>    (enabled?
>>> -   (boolean #t)
>>> +   maybe-boolean
>>
>> I made this change willingly, because it doesn't make sense to me to
>> define a jail configuration that is disabled by default.
>>
>> Does that make sense?
>
> When I was implementing configuration, I was thinking the same.
>
> However, while trying to find sensible or hardcoded defaults,
> looking at code and usage examples, I found that configuration
> of fail2ban is quite loose, than one can anticipiate. Variables
> can be overriden from many places, so absence of variable also
> becomes as important. So I decided to opt-in for most fields
> as maybe-.
>
> IMHO, this is why they had to make --d/--dp flags to
> dump effective configuration :)
>
> I actually mention that in the docs, where it says that
> unspecified values are not serialized in favor of whatever
> are defaults and/or values specified elsewhere. Which are
> basically hard to track.

I had looked at how this all works, and while it's true that omitting
values seems the norm, in the context of Guix, it seems OK diverge for
this for the 'enabled' value of extra jail configurations (jail.local).

> And finally there is a [DEFAULT] section which applies to all
> jails. But it is not a jail by itself. So if there is "enabled =
> false" or "enabled = true", fail2ban fails to start, because of
> unexpected variable for that section.

You mean the default section from $(guix build
fail2ban)/etc/fail2ban/jail.conf?  It has 'enabled = false', and we
don't generate such section ourselves, so I don't understand your point.
The test suite also passes; can you think of a scenario that would make
it fail?  We could add a test for it.

I've pushed the other corrections, thanks!

Closing (but we can continue the discussion here).

Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.