[bug#48803] strongswan: provide a service definition and configuration interface.

This commit adds a strongswan-service-type which allows the user to
start strongswan correctly on Guix. Without this, they would need to
manually write a strongswan.conf file and run it with

`STRONGSWAN_CONF=/path/to/strongswan.conf ipsec start`.

For now, we only support the legacy ipsec.conf/ipsec.secrets interface.
Because ipsec.conf depends on indentation and is a deprecated intreface,
we do not provide an EDSL to configure it, and we do not put the config
file in a Guile string (to avoid indentation issues). Similarly,
ipsec.secrets contains the users authentication token/passwords, and is
for security reasons transmitted separately from the configuration file.

This change allows the user to write something as follows in their
config:

```
    (service strongswan-service-type
	     (strongswan-configuration
	      (use-ipsec? #t)
	      (ipsec-conf "/config-files/ipsec.conf")
	      (ipsec-secrets "/config-files/ipsec.secrets")))
```

This will start the charon daemon and allow them to connect to their
VPNs configured in `/config-files/ipsec.conf`.
---
 gnu/services/vpn.scm | 128 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)

Domagoj,

Domagoj Stolfa 写道：
> This commit adds a strongswan-service-type which allows the user 
> to
> start strongswan correctly on Guix.

Thank you!

> Because ipsec.conf depends on indentation and is a deprecated 
> intreface,
> we do not provide an EDSL to configure it,

OK.

> and we do not put the config
> file in a Guile string (to avoid indentation issues).

Not using a string is fine by me, but I don't understand this 
particular argument for it.

> Similarly,
> ipsec.secrets contains the users authentication token/passwords, 
> and is
> for security reasons transmitted separately from the 
> configuration file.

OK, good to make it hard to inadvertently intern into the store.

>     (service strongswan-service-type
> 	     (strongswan-configuration
> 	      (use-ipsec? #t)
> 	      (ipsec-conf "/config-files/ipsec.conf")
> 	      (ipsec-secrets "/config-files/ipsec.secrets")))

(I)IRC you told me that the majority of users simply point 
StrongSwan to a .conf/.secrets file they got from on high, and 
this is all they'll ever need to do so.  Sounds good to me.

This is a bit straightforward (no ‘local-file’, ‘plain-file’, …) 
but there's precedent for that:

  (service nginx-service-type
           (nginx-configuration
            (file "/etc/guix/nginx/nginx.conf")))

What does the daemon do now when USE-IPSEC? is #f?  Anything 
useful?

Could we drop USE-IPSEC? and allow IPSEC-CONF/IPSEC-SECRETS to be 
#f to signal the same thing (enforcing only sane combinations)? 
Or would that make things more confusing?

Is all this legacy enough to mark as such in the field name 
(LEGACY-IPSEC-CONF, etc.) or is it one of those things that will 
never ever go away and VPN providers will still hand out 
ipsecs.conf in 2038?

> This will start the charon daemon and allow them to connect to 
> their
> VPNs configured in `/config-files/ipsec.conf`.
> ---
>  gnu/services/vpn.scm | 128 
>  +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
>
> diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
> index 2bcbf76727..e026f2aa58 100644
> --- a/gnu/services/vpn.scm
> +++ b/gnu/services/vpn.scm
> @@ -4,6 +4,7 @@
>  ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;; Copyright © 2021 Guillaume Le Vaillant <glv@posteo.net>
>  ;;; Copyright © 2021 Solene Rapenne <solene@perso.pw>
> +;;; Copyright © 2021 Domagoj Stolfa <ds815@gmx.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -26,6 +27,7 @@
>    #:use-module (gnu services shepherd)
>    #:use-module (gnu system shadow)
>    #:use-module (gnu packages admin)
> +  #:use-module (gnu packages networking)
>    #:use-module (gnu packages vpn)
>    #:use-module (guix packages)
>    #:use-module (guix records)
> @@ -44,6 +46,9 @@
>              generate-openvpn-client-documentation
>              generate-openvpn-server-documentation
>  
> +            strongswan-configuration
> +            strongswan-service-type
> +
>              wireguard-peer
>              wireguard-peer?
>              wireguard-peer-name
> @@ -529,6 +534,129 @@ is truncated and rewritten every minute.")
>       (openvpn-remote-configuration 
>       ,openvpn-remote-configuration-fields))
>     'openvpn-client-configuration))
>  
> +;;;
> +;;; Strongswan.
> +;;;
> +
> +(define-record-type* <strongswan-configuration>
> +  strongswan-configuration make-strongswan-configuration
> +  strongswan-configuration?
> +  (strongswan      strongswan-configuration-strongswan 
> ;<package>
> +                   (default strongswan))
> +  (use-ipsec?      strongswan-configuration-use-ipsec? ;legacy 
> interface
> +                   (default #f))
> +  (ipsec-conf      strongswan-configuration-ipsec-conf)
> +  (ipsec-secrets   strongswan-configuration-ipsec-secrets))
> +
> +;; In the future, it might be worth implementing a record type 
> to configure
> +;; all of the plugins, but for *most* basic usecases, simply 
> creating the
> +;; files will be sufficient. Same is true of charon-plugins.
> +(define strongswand-config-files
> +  (list "charon" "charon-logging" "pki" "pool" "scepclient"
> +        "swanctl" "tnc"))
> +
> +;; Plugins to load.
> +(define charon-plugins
> +  (list "aes" "aesni" "attr" "attr-sql" "chapoly" "cmac" 
> "constraints"
> +        "counters" "curl" "curve25519" "dhcp" "dnskey" "drbg" 
> "eap-aka-3gpp"
> +        "eap-aka" "eap-dynamic" "eap-identity" "eap-md5" 
> "eap-mschapv2"
> +        "eap-peap" "eap-radius" "eap-simaka-pseudonym" 
> "eap-simaka-reauth"
> +        "eap-simaka-sql" "eap-sim" "eap-sim-file" "eap-tls" 
> "eap-tnc"
> +        "eap-ttls" "ext-auth" "farp" "fips-prf" "gmp" "ha" 
> "hmac"
> +        "kernel-netlink" "led" "md4" "md5" "mgf1" "nonce" 
> "openssl" "pem"
> +        "pgp" "pkcs12" "pkcs1" "pkcs7" "pkcs8" "pubkey" 
> "random" "rc2"
> +        "resolve" "revocation" "sha1" "sha2" "socket-default" 
> "soup" "sql"
> +        "sqlite" "sshkey" "tnc-tnccs" "vici" "x509" "xauth-eap" 
> "xauth-generic"
> +        "xauth-noauth" "xauth-pam" "xcbc"))

Are these simply ‘all of the plug-ins’?

I'm fine with this ‘temporary’ solution as long as it's never 
exported.

I'll trust you on all of this configuration syntax madness:  :-)

> +(define (strongswan-configuration-file config)
> +  (match-record config <strongswan-configuration>
> +    (strongswan use-ipsec? ipsec-conf ipsec-secrets)
> +    (let* ((strongswan-dir
> +            (computed-file
> +             "strongswan.d"
> +             #~(begin
> +                 (mkdir #$output)
> +                 ;; Create all of the configuration files in 
> strongswan.d/*.conf
> +                 (map (lambda (conf-file)
> +                        (let* ((filename (string-append
> +                                          #$output "/"
> +                                          conf-file ".conf")))
> +                          (call-with-output-file filename
> +                            (lambda (port)
> +                              (display
> +                               "# Created by 
> 'strongswan-service'\n"
> +                               port)))))
> +                      (list #$@strongswand-config-files))
> +                 (mkdir (string-append #$output "/charon"))
> +                 ;; And all of the strongswan.d/charon/*.conf 
> files (plugins)

Nitpick: ;;-comments are full sentences ending in a full stop.

> +                 (map (lambda (plugin)
> +                        (let* ((filename (string-append
> +                                          #$output "/charon/"
> +                                          plugin ".conf")))
> +                          (call-with-output-file filename
> +                            (lambda (port)
> +                              (format port "~a {
> +  load = yes
> +}"
> +                                      plugin)))))
> +                      (list #$@charon-plugins))))))
> +      ;; Generate our strongswan.conf to reflect the user 
> configuration.
> +      (computed-file
> +       "strongswan.conf"
> +       #~(begin
> +           (call-with-output-file #$output
> +             (lambda (port)
> +               (display "# Generated by 
> 'strongswan-service'.\n" port)
> +               (format port "charon {
> +  load_modular = yes
> +  plugins {
> +    include ~a/charon/*.conf"
> +                       #$strongswan-dir)
> +               (if #$use-ipsec?
> +                   (format port "
> +    stroke {
> +      load = yes
> +      secrets_file = ~a
> +    }

All this indentation is doing my head in, but it looks like here…

> +  }
> +}
> +
> +starter {
> +  config_file = ~a
> +}
> +
> +include ~a/*.conf"
> +                           #$ipsec-secrets
> +                           #$ipsec-conf
> +                           #$strongswan-dir)
> +                   (format port "
> +  }
> +}
> +include ~a/*.conf"
> +                           #$strongswan-dir)))))))))

…you had to choose between two ifs and two #$strongswan-dirs, and 
chose two #$strongswan-dirs?  I prefer two ifs.

> +(define (strongswan-shepherd-service config)
> +  (let* ((ipsec (file-append strongswan "/sbin/ipsec"))
> +        (strongswan-conf-path (strongswan-configuration-file 
> config)))
> +    (list (shepherd-service
> +           (requirement '(networking))
> +           (provision '(strongswan))

I guess.  I have no idea how ‘generic’ StrongSwan is and whether 
this makes more sense than (provision '(ipsec)) or not.

> +           (start #~(make-forkexec-constructor
> +                     (list #$ipsec "start" "--nofork")
> +                     #:environment-variables
> +                     (list (string-append "STRONGSWAN_CONF="
> + 
> #$strongswan-conf-path))))
> +           (stop #~(make-kill-destructor))
> +           (documentation "Start the charon daemon for IPsec 
> VPN")))))

"StrongSwan's charon IKE keying daemon for IPsec VPN."

Most of ‘Run the …’/‘Start the …’ noise that has snuck into 
gnu/services should probably be removed.

> +(define strongswan-service-type
> +  (service-type
> +   (name 'strongswan)
> +   (extensions
> +    (list (service-extension shepherd-root-service-type
> +                             strongswan-shepherd-service)))))
> +
>  ;;;
>  ;;; Wireguard.
>  ;;;

For this to be merged, we're still missing some documentation in 
doc/guix.text.  Would you be willing to write some?

Kind regards,

T G-R

Forgot to add: please include a GNU/Guix-style commit message 
like:

    gnu: Add strongswan service.

    * gnu/services/vpn.scm (strongswan-configuration): New record 
    type.
    (charon-plugins, strongswan-configuration-file)
    (strongswan-shepherd-service, strongswan-service-type): New 
    variables.
    * doc/guix.tex (VPN Services): Document them all!

Kind regards,

T G-R

Tobias,
> > and we do not put the config
> > file in a Guile string (to avoid indentation issues).
> 
> Not using a string is fine by me, but I don't understand this particular
> argument for it.

ipsec.conf is pythonistic in the sense that it's sensitive to
indentation. This is just to avoid copy-paste errors in a config file
that results in cryptic error messages because the user missed a space
while copy-pasting something. It's easier to just transmit the file as
given by the "higher-ups" out of bounds than have it as a configuration
string, as ipsec.secrets kind of has to be transmitted that way anyway.

> What does the daemon do now when USE-IPSEC? is #f?  Anything useful?

It doesn't do anything other than use the default configuration that is
provided by strongswan as it is shipped (basically, whatever is in the
build directory by default). This is what it has done up until this
point already, the user would have start strongswan by setting an
environment variable to some local `strongswan.conf`. It is also what
strongswan does on a fresh installation in any other distribution I've
tried it on.

> Could we drop USE-IPSEC? and allow IPSEC-CONF/IPSEC-SECRETS to be #f to
> signal the same thing (enforcing only sane combinations)? Or would that make
> things more confusing?

We could, the plan I had for `strongswan` as a service is to support
both ipsec.conf/ipsec.secrets and swanctl, hence the `use-ipsec?` as a
separate thing. I can refactor it without that flag and have no real
strong opinion on it.

> Is all this legacy enough to mark as such in the field name
> (LEGACY-IPSEC-CONF, etc.) or is it one of those things that will never ever
> go away and VPN providers will still hand out ipsecs.conf in 2038?

Unclear at this point, I don't see how strongswan could drop support for
ipsec.conf and ipsec.secrets without making a lot of users angry at this
point. The VPN that I'm using is configured and documented by people who
are quite familiar with strongswan, and even there the documentation is
referring to ipsec.conf and ipsec.secrets rather than swanctl at the
moment.

> Nitpick: ;;-comments are full sentences ending in a full stop.

ACK. Will fix.

> …you had to choose between two ifs and two #$strongswan-dirs, and chose two
> #$strongswan-dirs?  I prefer two ifs.

I think the reasoning for this was that if we're not using
ipsec.conf/ipsec.secrets, we would be writing swanctl-specific
configuration. Right now, that is just including strongswan.d, but it
might do other things, so I've kept it in a more traditional if-else
format.

> I guess.  I have no idea how ‘generic’ StrongSwan is and whether this makes
> more sense than (provision '(ipsec)) or not.

That's a good question. I think it could probably provision ipsec, but I
haven't really verified it so I didn't want to risk doing that. I assume
that it can, though.

> "StrongSwan's charon IKE keying daemon for IPsec VPN."

ACK.

> For this to be merged, we're still missing some documentation in
> doc/guix.text.  Would you be willing to write some?

Will include the docs with the next patch.

Thanks for the detailed feedback!