GNU bug report logs -
#56608
[PATCH] gnu: security: Add fail2ban-service-type.
Previous Next
Reported by: muradm <mail <at> muradm.net>
Date: Sun, 17 Jul 2022 02:33:01 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Hi,
Squashed patch will come later on.
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
> Hi,
>
> muradm <mail <at> muradm.net> writes:
>
> [...]
>
>> --- /dev/null
>> +++ b/gnu/tests/security.scm
>
> I'd keep the tests with the introductory commit (squashed in
> preceding
> one).
>
Done.
[...]
>> +(define (run-fail2ban-basic-test)
>> +
>> + (define os
>> + (marionette-operating-system
>> + (simple-operating-system
>> + (service fail2ban-service-type))
>> + #:imported-modules '((gnu services herd)
>> + (guix combinators))))
> ^ (guix combinators) seems unused
>
Done including other places.
>> + (define vm
>> + (virtual-machine
>> + (operating-system os)
>> + (port-forwardings '())))
>
> (define vm (virtual-machine (operating-system os))) should be
> sufficient.
>
For me it does not work without specfying port-forwardings.
I get wierd error like following:
gnu/tests/security.scm:47:5: error: os: invalid field specifier
I suppose it is something todo with virtual-machine.
So I'm leaving port-forwardings as is.
[...]
>> + (define (wait-for-unix-socket-m socket)
>> + (wait-for-unix-socket socket marionette))
>
> Overkill as used once in scope.
>
Done including other places.
>> +
>> + (test-runner-current (system-test-runner #$output))
>> + (test-begin "fail2ban-basic-test")
>> +
>> + (test-assert "fail2ban running"
>> + (marionette-eval
>> + '(begin
>> + (use-modules (gnu services herd))
>> + (start-service 'fail2ban))
>> + marionette))
>
> I like to test that services can be restarted too, as in my
> experience
> there can be races and other situations that may cause them to
> fail
> restarting.
>
Done.
[...]
>> + (test-equal "fail2ban sshd jail running"
>> + '("Status for the jail: sshd"
>> + "|- Filter"
>> + "| |- Currently failed:\t0"
>> + "| |- Total failed:\t0"
>> + "| `- File list:\t/var/log/secure"
>> + "`- Actions"
>> + " |- Currently banned:\t0"
>> + " |- Total banned:\t0"
>> + " `- Banned IP list:\t"
>> + "")
>> + (marionette-eval
>> + '(begin
>> + (use-modules (ice-9 rdelim) (ice-9 popen)
>> (rnrs io ports))
>> + (let ((call-command
>> + (lambda (cmd)
>> + (let* ((err-cons (pipe))
>> + (port (with-error-to-port (cdr
>> err-cons)
>> + (lambda ()
>> (open-input-pipe cmd))))
>> + (_ (setvbuf (car err-cons)
>> 'block
>> + (* 1024 1024 16)))
>> + (result (read-delimited ""
>> port)))
>> + (close-port (cdr err-cons))
>> + (values result (read-delimited ""
>> (car err-cons)))))))
>> + (string-split
>> + (call-command
>> + (string-join (list #$%fail2ban-server-cmd
>> "status" "sshd") " "))
>> + #\newline)))
>> + marionette))
>
> Perhaps this could be turned into an Shepherd action, and the
> Guile
> procedure could do the above to return the text output; to
> simplify the
> test and reduce boilerplate, while providing value to the user.
>
[...]
>> + (gexp->derivation "fail2ban-extending-test" test))
>> +
>> +(define %test-fail2ban-extending
>
> Perhaps %test-fail2ban-extension ?
Done, s/extending/extension/.
> Otherwise, that last test seems to
> test exactly the same things as the preceding one, so there
> should be a
> procedure to generate the test, taking the OS as an argument to
> avoid
> code duplication.
>
Done, refactored with define-syntax-rule.
> Thanks for working on this!
>
> Maxim
[signature.asc (application/pgp-signature, inline)]
This bug report was last modified 2 years and 324 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.