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

From: Tobias Geerinckx-Rice <[email-masked]>
Date: Sun, 13 Jun 2021 14:41:00 +0200

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 <[email-masked]>
>  ;;; Copyright © 2021 Guillaume Le Vaillant <[email-masked]>
>  ;;; Copyright © 2021 Solene Rapenne <[email-masked]>
> +;;; Copyright © 2021 Domagoj Stolfa <[email-masked]>
>  ;;;
>  ;;; 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