GNU bug report logs - #55055
[PATCH] gnu: wireguard: Add support for PresharedKey

Previous Next

Package: guix-patches;

Reported by: Paul Alesius <paul <at> unnservice.com>

Date: Thu, 21 Apr 2022 13:28:02 UTC

Severity: normal

Tags: patch

Done: Mathieu Othacehe <othacehe <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55055 in the body.
You can then email your comments to 55055 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 13:28:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Alesius <paul <at> unnservice.com>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 21 Apr 2022 13:28:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Alesius <paul <at> unnservice.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: wireguard: Add support for PresharedKey
Date: Thu, 21 Apr 2022 15:26:30 +0200
[Message part 1 (text/plain, inline)]
The WireGuard configuration supports a PresharedKey attribute for
additional security. This patch adds support for configuring a PresharedKey
attribute.

Tested, working.

With regards,
- Paul
[Message part 2 (text/html, inline)]
[guix.wg-psk.patch (application/octet-stream, attachment)]

Information forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 14:27:02 GMT) Full text and rfc822 format available.

Message #8 received at 55055 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Paul Alesius <paul <at> unnservice.com>, 55055 <at> debbugs.gnu.org
Subject: Re: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey
Date: Thu, 21 Apr 2022 16:25:53 +0200
[Message part 1 (text/plain, inline)]
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.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 20:43:01 GMT) Full text and rfc822 format available.

Message #11 received at 55055 <at> debbugs.gnu.org (full text, mbox):

From: Paul Alesius <paul <at> unnservice.com>
To: 55055 <at> debbugs.gnu.org
Subject: Fwd: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey
Date: Thu, 21 Apr 2022 22:41:53 +0200
[Message part 1 (text/plain, inline)]
> 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 <at> 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.
>
[Message part 2 (text/html, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 21:49:01 GMT) Full text and rfc822 format available.

Message #14 received at 55055 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Paul Alesius <paul <at> unnservice.com>
Cc: 55055 <at> debbugs.gnu.org
Subject: Re: [bug#55055] [PATCH] gnu: wireguard: Add support for PresharedKey
Date: Thu, 21 Apr 2022 23:48:37 +0200
[Message part 1 (text/plain, inline)]
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.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 21:56:01 GMT) Full text and rfc822 format available.

Message #17 received at 55055 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Paul Alesius <paul <at> unnservice.com>, 55055 <at> debbugs.gnu.org
Subject: Re: [bug#55055] Fwd: [bug#55055] [PATCH] gnu: wireguard: Add
 support for PresharedKey
Date: Thu, 21 Apr 2022 23:55:17 +0200
[Message part 1 (text/plain, inline)]
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.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#55055; Package guix-patches. (Thu, 21 Apr 2022 22:00:02 GMT) Full text and rfc822 format available.

Message #20 received at 55055 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Paul Alesius <paul <at> unnservice.com>, 55055 <at> debbugs.gnu.org
Subject: Re: [bug#55055] Fwd: [bug#55055] [PATCH] gnu: wireguard: Add
 support for PresharedKey
Date: Thu, 21 Apr 2022 23:59:40 +0200
[Message part 1 (text/plain, inline)]
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.
[signature.asc (application/pgp-signature, inline)]

Reply sent to Mathieu Othacehe <othacehe <at> gnu.org>:
You have taken responsibility. (Mon, 26 Dec 2022 16:54:02 GMT) Full text and rfc822 format available.

Notification sent to Paul Alesius <paul <at> unnservice.com>:
bug acknowledged by developer. (Mon, 26 Dec 2022 16:54:02 GMT) Full text and rfc822 format available.

Message #25 received at 55055-done <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Paul Alesius <paul <at> unnservice.com>
Cc: 55055-done <at> debbugs.gnu.org
Subject: Re: bug#55055: [PATCH] gnu: wireguard: Add support for PresharedKey
Date: Mon, 26 Dec 2022 17:53:13 +0100
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




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 24 Jan 2023 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 91 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.