[bug#73955] [PATCH 0/2] Improve customizability of WireGuard service

[bug#73955] [PATCH 0/2] Improve customizability of WireGuard service

[Richard Sent]

Hi all,

The goal for this patch series is to improve wireguard-service's
customizability, primarily by supporting gexps evaluating to strings in most
fields. Prior to this patch, lists of gexp's were not serialized to strings,
preventing certain constructs from being used.

This was prompted from an issue I ran into a while back. [1]

I tested the serialization of several config records and did not notice any
issues. I would greatly appreciate if any users of wireguard-service could
confirm their existing configurations still serialize correctly. You can do so
via these guix REPL commands:

$ guix repl -L /path/to/guix/clone/with/patches
,use (guix)
,use (gnu services vpn)
,build ((@@ (gnu services vpn) wireguard-configuration-file)
        <paste-your-wireguard-configuration>)

I took the liberty of CCing a few people who previously committed to
WireGuard. Apologies if I committed a faux pas. :)

[1]: https://lists.gnu.org/archive/html/help-guix/2024-01/msg00204.html

Richard Sent (2):
  services: wireguard: Make the private-key field optional.
  services: wireguard: Support lists of gexps for most fields.

 doc/guix.texi        |  5 ++-
 gnu/services/vpn.scm | 74 +++++++++++++++++++++++---------------------
 2 files changed, 43 insertions(+), 36 deletions(-)


base-commit: bd26815cf8ce38a3b03676a6e3fc482bb74247cb
-- 
2.46.0

[bug#73955] [PATCH 1/2] services: wireguard: Make the private-key field optional.

[Richard Sent]

Users who retrieve the private-key via a PreUp field need to be able to
disable the default retrieval mechanism.

* gnu/services/vpn.scm (<wireguard-configuration>)[private-key]: Change
comment.
(wireguard-configuration-file): Conditionally serialize private-key.
* gnu/services/vpn.scm (wireguard-activation): Do not create private-key if
the field is #f.
* doc/guix.texi (VPN Services)[wireguard-configuration]: Document it.

Change-Id: Iac419809ae94eb76e97ff1f1749e2f4b3e65bb04
---
 doc/guix.texi        |  5 ++++-
 gnu/services/vpn.scm | 36 ++++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ac3a7adef0..5558bd7d44 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34453,7 +34453,10 @@ VPN Services
 
 @item @code{private-key} (default: @code{"/etc/wireguard/private.key"})
 The private key file for the interface.  It is automatically generated
-if the file does not exist.
+if the file does not exist.  If this field is @code{#f}, a private key
+is not created and the path is not serialized to the configuration file.
+This allows for retrieving the private key programmatically with a PreUp
+command.
 
 @item @code{peers} (default: @code{'()})
 The authorized peers on this interface.  This is a list of
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index 7fb4775757..b62e0ac838 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -741,7 +741,7 @@ (define-record-type* <wireguard-configuration>
                       (default '("10.0.0.1/32")))
   (port               wireguard-configuration-port ;integer
                       (default 51820))
-  (private-key        wireguard-configuration-private-key ;string
+  (private-key        wireguard-configuration-private-key ;maybe-string
                       (default "/etc/wireguard/private.key"))
   (peers              wireguard-configuration-peers ;list of <wiregard-peer>
                       (default '()))
@@ -805,9 +805,12 @@ (define (wireguard-configuration-file config)
                     #$@(if (null? pre-up)
                            '()
                            (list (format #f "~{PreUp = ~a~%~}" pre-up)))
-                    (format #f "PostUp = ~a set %i private-key ~a\
-~{ peer ~a preshared-key ~a~}" #$(file-append wireguard "/bin/wg")
-#$private-key '#$peer-keys)
+                    (if #$private-key
+                        (format #f "PostUp = ~a set %i private-key ~a\
+~{ peer ~a preshared-key ~a~}"
+                                #$(file-append wireguard "/bin/wg")
+                                #$private-key '#$peer-keys)
+                        "")
                     #$@(if (null? post-up)
                            '()
                            (list (format #f "~{PostUp = ~a~%~}" post-up)))
@@ -838,18 +841,19 @@ (define (wireguard-activation config)
         (use-modules (guix build utils)
                      (ice-9 popen)
                      (ice-9 rdelim))
-        (mkdir-p (dirname #$private-key))
-        (unless (file-exists? #$private-key)
-          (let* ((pipe
-                  (open-input-pipe (string-append
-                                    #$(file-append wireguard "/bin/wg")
-                                    " genkey")))
-                 (key (read-line pipe)))
-            (call-with-output-file #$private-key
-              (lambda (port)
-                (display key port)))
-            (chmod #$private-key #o400)
-            (close-pipe pipe))))))
+        (when #$private-key
+          (mkdir-p (dirname #$private-key))
+          (unless (file-exists? #$private-key)
+            (let* ((pipe
+                    (open-input-pipe (string-append
+                                      #$(file-append wireguard "/bin/wg")
+                                      " genkey")))
+                   (key (read-line pipe)))
+              (call-with-output-file #$private-key
+                (lambda (port)
+                  (display key port)))
+              (chmod #$private-key #o400)
+              (close-pipe pipe)))))))
 
 ;;; XXX: Copied from (guix scripts pack), changing define to define*.
 (define-syntax-rule (define-with-source (variable args ...) body body* ...)
-- 
2.46.0

[bug#73955] [PATCH 2/2] services: wireguard: Support lists of gexps for most fields.

[Richard Sent]

In order to support more flexibility in Wireguard configuration, ungexp the
configuration fields directly instead of ungexp-splicing a sexp
calculator. This allows for the fields to take arbitrary gexps instead of only
strings which is particularly helpful for the Pre/Post Up/Down commands.

For example, the wg-quick(8) manual has an example on how to use
password-store to retrieve a private key with a PreUp entry. This is now
possible.

* gnu/services/vpn.scm (wireguard-configuration-file): Ungexp configuration
lists instead of ungexp-splicing the code surrounding them.

Change-Id: If074cbb78473b6fd34e0e4e990d2ed268001d6c7
---
 gnu/services/vpn.scm | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index b62e0ac838..21a7fb827a 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -797,33 +797,33 @@ (define (wireguard-configuration-file config)
                  (define lines
                    (list
                     "[Interface]"
-                    #$@(if (null? addresses)
-                           '()
-                           (list (format #f "Address = ~{~a~^, ~}"
-                                         addresses)))
+                    (if (null? '#$addresses)
+                        ""
+                        (format #f "Address = ~{~a~^, ~}"
+                                (list #$@addresses)))
                     (format #f "~@[Table = ~a~]" #$table)
-                    #$@(if (null? pre-up)
-                           '()
-                           (list (format #f "~{PreUp = ~a~%~}" pre-up)))
+                    (if (null? '#$pre-up)
+                        ""
+                        (format #f "~{PreUp = ~a~%~}" (list #$@pre-up)))
                     (if #$private-key
                         (format #f "PostUp = ~a set %i private-key ~a\
 ~{ peer ~a preshared-key ~a~}"
                                 #$(file-append wireguard "/bin/wg")
                                 #$private-key '#$peer-keys)
                         "")
-                    #$@(if (null? post-up)
-                           '()
-                           (list (format #f "~{PostUp = ~a~%~}" post-up)))
-                    #$@(if (null? pre-down)
-                           '()
-                           (list (format #f "~{PreDown = ~a~%~}" pre-down)))
-                    #$@(if (null? post-down)
-                           '()
-                           (list (format #f "~{PostDown = ~a~%~}" post-down)))
+                    (if (null? '#$post-up)
+                        ""
+                        (format #f "~{PostUp = ~a~%~}" (list #$@post-up)))
+                    (if (null? '#$pre-down)
+                        ""
+                        (format #f "~{PreDown = ~a~%~}" (list #$@pre-down)))
+                    (if (null? '#$post-down)
+                        ""
+                        (format #f "~{PostDown = ~a~%~}" (list #$@post-down)))
                     (format #f "~@[ListenPort = ~a~]" #$port)
-                    #$@(if (null? dns)
-                           '()
-                           (list (format #f "DNS = ~{~a~^, ~}" dns)))))
+                    (if (null? '#$dns)
+                        ""
+                        (format #f "DNS = ~{~a~^, ~}" (list #$@dns)))))
 
                  (mkdir #$output)
                  (chdir #$output)
-- 
2.46.0

[bug#73955] [PATCH 2/2] services: wireguard: Support lists of gexps for most fields.

[Mathieu Othacehe]

Hello Richard,

Thanks for this series.

The first commit looks OK to me.

> For example, the wg-quick(8) manual has an example on how to use
> password-store to retrieve a private key with a PreUp entry. This is now
> possible.

It would be interesting to provide some testing for that.  Sadly, we do
not have a system test for Wireguard yet. We only have a unit test file
in (tests services vpn).

Adding a new (gnu tests vpn) module would be great in the future to test
different Wireguard configurations.

That can of course be done later on :)

Regarding this patch, the documentation is somehow vague on how to pass
post and pre commands:

--8<---------------cut here---------------start------------->8---
@item @code{post-up} (default: @code{'()})
The script commands to be run after setting up the interface.

...
--8<---------------cut here---------------end--------------->8---

Maybe you could elaborate on that a little bit and give some examples
that would be the translation of some of the post and pre commands that
are given in the wg-quick man page?

Thanks,

Mathieu