[PATCH] Wireguard: Rename field private-key to private-key-file

  • Open
  • quality assurance status badge
Details
3 participants
  • Apoorv Singh
  • Richard Sent
  • Sergey Trofimov
Owner
unassigned
Submitted by
Apoorv Singh
Severity
normal
A
A
Apoorv Singh wrote on 25 Sep 05:58 +0200
(address . guix-patches@gnu.org)
87h6a4jsej.fsf@gmail.com
The following patches renames the field private-key to private-key-file as it makes it more clear that it needs path to a file rather than the key it self
From 92e6d353a72e9ed0ee7097f2e5e5ff76521455a7 Mon Sep 17 00:00:00 2001
From: apoorv569 <apoorvs569@gmail.com>
Date: Wed, 25 Sep 2024 09:06:05 +0530
Subject: [PATCH 1/2] Wireguard rename field private-key to private-key-file

---
gnu/services/vpn.scm | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

Toggle diff (61 lines)
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index 7fb4775757..449909e34d 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-file wireguard-configuration-private-key-file ;string
(default "/etc/wireguard/private.key"))
(peers wireguard-configuration-peers ;list of <wiregard-peer>
(default '()))
@@ -782,7 +782,7 @@ (define (peers->preshared-keys peer keys)
keys)))
(match-record config <wireguard-configuration>
- (wireguard interface addresses port private-key peers dns
+ (wireguard interface addresses port private-key-file peers dns
pre-up post-up pre-down post-down table)
(let* ((config-file (string-append interface ".conf"))
(peer-keys (fold peers->preshared-keys (list) peers))
@@ -807,7 +807,7 @@ (define lines
(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)
+#$private-key-file '#$peer-keys)
#$@(if (null? post-up)
'()
(list (format #f "~{PostUp = ~a~%~}" post-up)))
@@ -833,22 +833,22 @@ (define lines
(define (wireguard-activation config)
(match-record config <wireguard-configuration>
- (private-key wireguard)
+ (private-key-file wireguard)
#~(begin
(use-modules (guix build utils)
(ice-9 popen)
(ice-9 rdelim))
- (mkdir-p (dirname #$private-key))
- (unless (file-exists? #$private-key)
+ (mkdir-p (dirname #$private-key-file))
+ (unless (file-exists? #$private-key-file)
(let* ((pipe
(open-input-pipe (string-append
#$(file-append wireguard "/bin/wg")
" genkey")))
(key (read-line pipe)))
- (call-with-output-file #$private-key
+ (call-with-output-file #$private-key-file
(lambda (port)
(display key port)))
- (chmod #$private-key #o400)
+ (chmod #$private-key-file #o400)
(close-pipe pipe))))))
;;; XXX: Copied from (guix scripts pack), changing define to define*.
--
2.46.0
.

--
- Apoorv Singh
- Sent from Emacs.
S
S
Sergey Trofimov wrote on 26 Sep 19:39 +0200
(name . Apoorv Singh)(address . apoorvs569@gmail.com)(address . 73465@debbugs.gnu.org)
87msju1fhh.fsf@sarg.org.ru
Apoorv Singh <apoorvs569@gmail.com> writes:

Toggle quote (4 lines)
> The following patches renames the field private-key to private-key-file as it
> makes it more clear that it needs path to a file rather than the key it self
>

Hi, you have to deprecate the field instead using
`warn-about-deprecation` procedure and to adjust the documentation as
well.

Please note that there is also preshared-key parameter which also takes
a path. It'd be nice to rename it as well for consistency sake.
A
A
Apoorv Singh wrote on 28 Sep 07:29 +0200
Wireguard: Rename field private-key to private-key-file
(address . 73465@debbugs.gnu.org)
878qvcjqh5.fsf@gmail.com
Do you want me to keep both private-key and private-key-file in
the record but still use private-key for now? but just warn about
deprecation for the field? Something like,

```
(define-record-type* <wireguard-configuration>
wireguard-configuration make-wireguard-configuration
wireguard-configuration?

;; other fields here..

(private-key wireguard-configuration-private-key-file
;deprecated
(default "/etc/wireguard/private.key"))
(private-key-file wireguard-configuration-private-key-file
;string
(default "/etc/wireguard/private.key"))
```

then, in the `wireguard-configuration-file` procedure, under
`match-record`, I should do something like,
```
(match-record config <wireguard-configuration>
(wireguard interface addresses port private-key peers dns ;;
keeping private-key field here..
pre-up post-up pre-down post-down table)
(let* ((config-file (string-append interface ".conf"))
(peer-keys (fold peers->preshared-keys (list) peers))
(peers (map peer->config peers))
(config
(computed-file
"wireguard-config"
#~(begin
(use-modules (ice-9 format)
(srfi srfi-1))

(define lines
(list
;; other stuff..

(when (not (string-null? #$private-key))
(warn-about-deprecation 'private-key
#f
#:replacement
'private-key-file))

(format #f "PostUp = ~a set %i private-key ~a\
~{ peer ~a preshared-key ~a~}" #$(file-append wireguard "/bin/wg")
#$private-key '#$peer-keys) ;; using private-key field here
still..


Sorry I'm not familiar with how all this works. Just making sure
before I commit any changes.

Also by adjust the documentation you mean edit the
doc/guix.texi:34373 file and append something like,
```
@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. 'Using private-key' is deprecated use
'private-key-file' instead.
```

--
- Apoorv Singh
- Sent from Emacs.
A
A
Apoorv Singh wrote on 30 Sep 09:04 +0200
(address . 73465@debbugs.gnu.org)
87cyklzko5.fsf@gmail.com
I made some changes, here is the output of `git diff`,

```
Toggle diff (181 lines)
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index eee7e78c6d..ebac4ad943 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -67,7 +67,8 @@ (define-module (gnu services vpn)
wireguard-peer-endpoint
wireguard-peer-allowed-ips
wireguard-peer-public-key
- wireguard-peer-preshared-key
+ wireguard-peer-preshared-key ; deprecated
+ wireguard-peer-preshared-key-file
wireguard-peer-keep-alive

wireguard-configuration
@@ -79,7 +80,8 @@ (define-module (gnu services vpn)
wireguard-configuration-dns
wireguard-configuration-monitor-ips?
wireguard-configuration-monitor-ips-interval
- wireguard-configuration-private-key
+ wireguard-configuration-private-key ; deprecated
+ wireguard-configuration-private-key-file
wireguard-configuration-peers
wireguard-configuration-pre-up
wireguard-configuration-post-up
@@ -721,15 +723,17 @@ (define strongswan-service-type
(define-record-type* <wireguard-peer>
wireguard-peer make-wireguard-peer
wireguard-peer?
- (name wireguard-peer-name)
- (endpoint wireguard-peer-endpoint
- (default #f)) ;string
- (public-key wireguard-peer-public-key) ;string
- (preshared-key wireguard-peer-preshared-key
- (default #f)) ;string
- (allowed-ips wireguard-peer-allowed-ips) ;list of strings
- (keep-alive wireguard-peer-keep-alive
- (default #f))) ;integer
+ (name wireguard-peer-name)
+ (endpoint wireguard-peer-endpoint
+ (default #f)) ;string
+ (public-key wireguard-peer-public-key) ;string
+ (preshared-key wireguard-peer-preshared-key ;deprecated
+ (default #f)) ;string
+ (preshared-key-file wireguard-peer-preshared-key-file
+ (default #f)) ;string
+ (allowed-ips wireguard-peer-allowed-ips) ;list of
strings
+ (keep-alive wireguard-peer-keep-alive
+ (default #f))) ;integer

(define-record-type* <wireguard-configuration>
wireguard-configuration make-wireguard-configuration
@@ -742,6 +746,8 @@ (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
;deprecated
+ (default "/etc/wireguard/private.key"))
(private-key-file wireguard-configuration-private-key-file
;string
(default "/etc/wireguard/private.key"))
(peers wireguard-configuration-peers ;list of
<wiregard-peer>
@@ -778,18 +784,29 @@ (define (peer->config peer)
(string-join (remove string-null? lines) "\n"))))

(define (peers->preshared-keys peer keys)
- (let ((public-key (wireguard-peer-public-key peer))
- (preshared-key (wireguard-peer-preshared-key peer)))
- (if preshared-key
- (cons* public-key preshared-key keys)
+ (let* ((public-key (wireguard-peer-public-key peer))
+ (preshared-key (wireguard-peer-preshared-key peer))
+ (preshared-key-file (wireguard-peer-preshared-key-file
peer))
+ (final-preshared-key (or preshared-key
preshared-key-file)))
+ ;; XXX Warn about deprecated preshared-key field with newer
replacement
+ (when preshared-key
+ (warn-about-deprecation 'preshared-key #f #:replacement
'preshared-key-file))
+ (if final-preshared-key
+ (cons* public-key final-preshared-key keys)
keys)))

(match-record config <wireguard-configuration>
- (wireguard interface addresses port private-key-file peers
dns
+ (wireguard interface addresses port private-key-file
private-key peers dns
pre-up post-up pre-down post-down table)
+
+ ;; XXX Warn about deprecated private-key field with newer
replacement
+ (when private-key
+ (warn-about-deprecation 'private-key #f #:replacement
'private-key-file))
+
(let* ((config-file (string-append interface ".conf"))
(peer-keys (fold peers->preshared-keys (list) peers))
(peers (map peer->config peers))
+ (final-private-key (or private-key private-key-file))
(config
(computed-file
"wireguard-config"
@@ -810,7 +827,7 @@ (define lines
(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-file '#$peer-keys)
+#$final-private-key '#$peer-keys)
#$@(if (null? post-up)
'()
(list (format #f "~{PostUp = ~a~%~}"
post-up)))
@@ -836,23 +853,29 @@ (define lines

(define (wireguard-activation config)
(match-record config <wireguard-configuration>
- (private-key-file wireguard)
- #~(begin
- (use-modules (guix build utils)
- (ice-9 popen)
- (ice-9 rdelim))
- (mkdir-p (dirname #$private-key-file))
- (unless (file-exists? #$private-key-file)
- (let* ((pipe
- (open-input-pipe (string-append
- #$(file-append wireguard
"/bin/wg")
- " genkey")))
- (key (read-line pipe)))
- (call-with-output-file #$private-key-file
- (lambda (port)
- (display key port)))
- (chmod #$private-key-file #o400)
- (close-pipe pipe))))))
+ (private-key private-key-file wireguard)
+
+ ;; XXX Warn about deprecated private-key field with newer
replacement
+ (when private-key
+ (warn-about-deprecation 'private-key #f #:replacement
'private-key-file))
+
+ (let ((final-private-key (or private-key private-key-file)))
+ #~(begin
+ (use-modules (guix build utils)
+ (ice-9 popen)
+ (ice-9 rdelim))
+ (mkdir-p (dirname #$final-private-key))
+ (unless (file-exists? #$final-private-key)
+ (let* ((pipe
+ (open-input-pipe (string-append
+ #$(file-append wireguard
"/bin/wg")
+ " genkey")))
+ (key (read-line pipe)))
+ (call-with-output-file #$final-private-key
+ (lambda (port)
+ (display key port)))
+ (chmod #$final-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* ...)
```

If this is desired way of doing this, I will share the formatted
patch as an attachment.

--
- Apoorv Singh
- Sent from Emacs.
A
R
R
Richard Sent wrote 25 hours ago
(name . Apoorv Singh)(address . apoorvs569@gmail.com)(address . 73465@debbugs.gnu.org)
87ldwo4wj1.fsf@freakingpenguin.com
With #73955, private-key better supports g-exp based command redirection,
e.g.

Toggle snippet (9 lines)
;; A config of
(wireguard-configuration
...
(private-key (string-append "(<" my-custom-script ">")))

;; Results in
PostUp = ... set %i private-key <(/gnu/store/...-my-custom-script) ...

(This was also supported before but was more limited.)

Given that, I think renaming it to private-key-file is more confusing
than keeping it as private-key. Same for preshared-key.

Perhaps we can somehow check the field, see if the user enters a
WG-compatible key literally, and emit a warning? I'm not fluent on the
format to determine the best way for that.
--
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 73465@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 73465
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch