[bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Paul Alesius]

[-- Attachment #1.1: Type: text/plain, Size: 193 bytes --]

The WireGuard configuration supports a PresharedKey attribute for
additional security. This patch adds support for configuring a PresharedKey
attribute.

Tested, working.

With regards,
- Paul

[-- Attachment #1.2: Type: text/html, Size: 292 bytes --]

[-- Attachment #2: guix.wg-psk.patch --]
[-- Type: application/octet-stream, Size: 1744 bytes --]

diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index b24e9cffb3..e3f5ff0d05 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -62,6 +62,7 @@ (define-module (gnu services vpn)
             wireguard-peer-allowed-ips
             wireguard-peer-public-key
             wireguard-peer-keep-alive
+            wireguard-peer-preshared-key
 
             wireguard-configuration
             wireguard-configuration?
@@ -701,6 +702,8 @@ (define-record-type* <wireguard-peer>
   (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
@@ -727,16 +730,20 @@ (define (wireguard-configuration-file config)
   (define (peer->config peer)
     (let ((name (wireguard-peer-name peer))
           (public-key (wireguard-peer-public-key peer))
+          (preshared-key (wireguard-peer-preshared-key peer))
           (endpoint (wireguard-peer-endpoint peer))
           (allowed-ips (wireguard-peer-allowed-ips peer))
           (keep-alive (wireguard-peer-keep-alive peer)))
       (format #f "[Peer] #~a
 PublicKey = ~a
 AllowedIPs = ~a
-~a~a"
+~a~a~a"
               name
               public-key
               (string-join allowed-ips ",")
+              (if preshared-key
+                  (format #f "PresharedKey = ~a\n" preshared-key)
+                  "")
               (if endpoint
                   (format #f "Endpoint = ~a\n" endpoint)
                   "")

[bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

Paul Alesius schreef op do 21-04-2022 om 15:26 [+0200]:
> +  (preshared-key     wireguard-peer-preshared-key
> +                     (default #f))   ;string

This should be documented in the documentation, otherwise it will be
difficult to discover.  Also, #f is not a string, did you mean
‘;#f|string’?

Also, a limitation: the preshared key will end up in the store, and
hence be world-readable.  So other users on the same system (other
people or compromised system daemons) could now determine the preshared
key.

Questions:

  * Could the security limitation be documented?

  * What security impact does a leaked secret key have?

  * Does wireguard has some inclusion mechanism, such that the
    wireguard configuration can ‘include’ some file outside the store?

  * WDYT of verifying that the preshared key looks ‘reasonable’
    (I guess only a-z0-9 characters, no spaces or newlines, not a
    bytevector ...)

    As-is, if I do (preshared-keys (string->utf8 "oops I thought this
    needs to be bytevector)) then "guix system reconfigure" doesn't
    give a nice error message, it will just silently produce a broken
    configuration file.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55055] Fwd: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Paul Alesius]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

> Also, #f is not a string, did you mean ‘;#f|string’?

The idea behind #f is that the field is optional, so that if it isn't
specified in the configuration then it isn't written to the configuration
file at all, hence #f is for a conditional when writing the actual
configuration file and has no default value.

>  * Could the security limitation be documented?
> * Does wireguard has some inclusion mechanism, such that the wireguard
configuration can ‘include’ some file outside the store?

I'll fix it properly to allow for loading of a key file, WireGuard does not
have an inclusion mechanism. How does it work with regards to documentation
and i18n versions, do you use online translation for the other languages? I
can really only fill in the english version.

>   * What security impact does a leaked secret key have?

Minimal to none, one should worry about the cloud peers over the wire guard
pre-shared key. It's just an additional layer of security in case the
public key algorithms are broken (for instance with quantum decryption),
then the pre-shared key functions as a one-time pad. If none is specified,
wireguard will use a default one of an all-zero string.

Since countries log all traffic, you never know what they have, hence my
patch submission.

> * WDYT of verifying that the preshared key looks ‘reasonable’ (I guess
only a-z0-9 characters, no spaces or newlines, not a bytevector ...)

I could develop a subsystem for validating the fields of the wireguard but
isn't it better to provide validations from the guix framework more
broadly? With my level of Guile scripting right now, I doubt that it would
be accepted.

With regards,
- Paul

On Thu, 21 Apr 2022 at 16:26, Maxime Devos <maximedevos@telenet.be> wrote:

> Paul Alesius schreef op do 21-04-2022 om 15:26 [+0200]:
> > +  (preshared-key     wireguard-peer-preshared-key
> > +                     (default #f))   ;string
>
> This should be documented in the documentation, otherwise it will be
> difficult to discover.  Also, #f is not a string, did you mean
> ‘;#f|string’?
>
> Also, a limitation: the preshared key will end up in the store, and
> hence be world-readable.  So other users on the same system (other
> people or compromised system daemons) could now determine the preshared
> key.
>
> Questions:
>
>   * Could the security limitation be documented?
>
>   * What security impact does a leaked secret key have?
>
>   * Does wireguard has some inclusion mechanism, such that the
>     wireguard configuration can ‘include’ some file outside the store?
>
>   * WDYT of verifying that the preshared key looks ‘reasonable’
>     (I guess only a-z0-9 characters, no spaces or newlines, not a
>     bytevector ...)
>
>     As-is, if I do (preshared-keys (string->utf8 "oops I thought this
>     needs to be bytevector)) then "guix system reconfigure" doesn't
>     give a nice error message, it will just silently produce a broken
>     configuration file.
>
> Greetings,
> Maxime.
>

[-- Attachment #2: Type: text/html, Size: 3821 bytes --]

[bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Paul Alesius schreef op do 21-04-2022 om 22:30 [+0200]:
> > * Does wireguard has some inclusion mechanism, such that the
> > wireguard configuration can ‘include’ some file outside the store?
> 
> I'll fix it properly to allow for loading of a key file, WireGuard
> does not have an inclusion mechanism. How does it work with regards
> to documentation and i18n versions, do you use online translation for
> the other languages? I can really only fill in the english version.

The main document is the English guix.texi, contributing.texi, ...

Translation happens at <https://translate.fedoraproject.org/projects/guix/documentation-manual>.
There is an automated system for making the translated guix.texi from
the main guix.texi and the translations at translate.fedoraproject.org.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55055] Fwd: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

Paul Alesius schreef op do 21-04-2022 om 22:41 [+0200]:
> > * WDYT of verifying that the preshared key looks ‘reasonable’ (I
> > guess only a-z0-9 characters, no spaces or newlines, not a
> > bytevector ...)
> 
> I could develop a subsystem for validating the fields of the
> wireguard but isn't it better to provide validations from the guix
> framework more broadly? With my level of Guile scripting right now, I
> doubt that it would be accepted.

There's already a basic system for this: field sanitisers. Have a look
at <network-address> and its 'assert-valid-address'.  Long term, there
were some ideas for a contract system à la racket, there was some e-
mail thread about that.

Also, some very basic validation could be replacing

  (format #f "PresharedKey = ~a\n" preshared-key)

by

  (string-append "PresharedKey = " preshared-key "\n")

-- basically, let string-append do some basic type checking.  This only
checks that it's a string though.  'make-regexp' and friends may be
useful for more complete validation.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#55055] Fwd: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

Paul Alesius schreef op do 21-04-2022 om 22:41 [+0200]:
> > Also, #f is not a string, did you mean ‘;#f|string’?
> 
> The idea behind #f is that the field is optional, so that if it isn't
> specified in the configuration then it isn't written to the
> configuration file at all, hence #f is for a conditional when writing
> the actual configuration file and has no default value.

It's optional in the generated wireguard configuration file, but not in
the Guix record -- Guile records don't have a concept of optional
fields, though there are fields with default values.

Though apparently conventions are a bit inconsistent in Guix on this
matter.  wireguard-configuration just does ;string, but <agetty-
configuration> does

(define-record-type* <agetty-configuration>
  [...]
  (tty              agetty-configuration-tty)     ;string | #f
  (term             agetty-term                   ;string | #f
                    (default #f))
  (baud-rate        agetty-baud-rate              ;string | #f
                    (default #f))
  (auto-login       agetty-auto-login             ;list of strings | #f
                    (default #f))
  [...]

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#55055: [PATCH] gnu: wireguard: Add support for PresharedKey

[Mathieu Othacehe]

Hello Paul,

> The WireGuard configuration supports a PresharedKey attribute for
> additional security. This patch adds support for configuring a
> PresharedKey attribute.

I noticed this patchset after merging a more recent one, sorry about
that. I think we can close this one though.

Thanks,

Mathieu