GNU bug report logs - #63786
[PATCH] home: services: ssh: Allow unset boolean options in ssh-config.

Previous Next

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

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

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: guix-patches <at> gnu.org
Cc: Efraim Flashner <efraim <at> flashner.co.il>
Subject: [PATCH] home: services: ssh: Allow unset boolean options in
 ssh-config.
Date: Mon, 29 May 2023 17:52:59 +0300

From man 5 ssh_config:
Unless noted otherwise, for each parameter, the first obtained value
will be used.

We want to allow falling through to the first actual user defined value.

* gnu/home/services.ssh.scm (define-maybe boolean): New configuration.
(openssh-host)[forward-x11?, forward-x11-trusted?, forward-agent?,
compression?]: Replace default value with maybe-boolean.
* doc/guix.texi (Secure Shell): Update documentation to match the
changes in the code.
---
 doc/guix.texi             | 10 +++++-----
 gnu/home/services/ssh.scm | 11 +++++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 31dc33fb97..d22924e522 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33,7 +33,7 @@
 Copyright @copyright{} 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Ricardo Wurmus@*
 Copyright @copyright{} 2016 Ben Woodcroft@*
 Copyright @copyright{} 2016, 2017, 2018, 2021 Chris Marusich@*
-Copyright @copyright{} 2016, 2017, 2018, 2019, 2020, 2021, 2022 Efraim Flashner@*
+Copyright @copyright{} 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Efraim Flashner@*
 Copyright @copyright{} 2016 John Darrington@*
 Copyright @copyright{} 2016, 2017 Nikita Gillmann@*
 Copyright @copyright{} 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 Jan Nieuwenhuizen@*
@@ -43017,19 +43017,19 @@ Secure Shell
 @item @code{user} (type: maybe-string)
 User name on the remote host.
 
-@item @code{forward-x11?} (default: @code{#f}) (type: boolean)
+@item @code{forward-x11?} (type: maybe-boolean)
 Whether to forward remote client connections to the local X11 graphical
 display.
 
-@item @code{forward-x11-trusted?} (default: @code{#f}) (type: boolean)
+@item @code{forward-x11-trusted?} (type: maybe-boolean)
 Whether remote X11 clients have full access to the original X11
 graphical display.
 
-@item @code{forward-agent?} (default: @code{#f}) (type: boolean)
+@item @code{forward-agent?} (type: maybe-boolean)
 Whether the authentication agent (if any) is forwarded to the remote
 machine.
 
-@item @code{compression?} (default: @code{#f}) (type: boolean)
+@item @code{compression?} (type: maybe-boolean)
 Whether to compress data in transit.
 
 @item @code{proxy} (type: maybe-proxy-command-or-jump-list)
diff --git a/gnu/home/services/ssh.scm b/gnu/home/services/ssh.scm
index 628dc743ae..0a4b37d84e 100644
--- a/gnu/home/services/ssh.scm
+++ b/gnu/home/services/ssh.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2022 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2023 Janneke Nieuwenhuizen <janneke <at> gnu.org>
+;;; Copyright © 2023 Efraim Flashner <efraim <at> flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -104,6 +105,8 @@ (define (serialize-natural-number field value)
   (string-append "  " (serialize-field-name field) " "
                  (number->string value) "\n"))
 
+(define-maybe boolean)
+
 (define (serialize-boolean field value)
   (string-append "  " (serialize-field-name field) " "
                  (if value "yes" "no") "\n"))
@@ -194,19 +197,19 @@ (define-configuration openssh-host
    maybe-string
    "User name on the remote host.")
   (forward-x11?
-   (boolean #f)
+   maybe-boolean
    "Whether to forward remote client connections to the local X11 graphical
 display.")
   (forward-x11-trusted?
-   (boolean #f)
+   maybe-boolean
    "Whether remote X11 clients have full access to the original X11 graphical
 display.")
   (forward-agent?
-   (boolean #f)
+   maybe-boolean
    "Whether the authentication agent (if any) is forwarded to the remote
 machine.")
   (compression?
-   (boolean #f)
+   maybe-boolean
    "Whether to compress data in transit.")
   (proxy-command
    maybe-string

base-commit: 7b400e7f8751e6b0cc6e66d3f7ecfb7f5bd51309
-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: , 63786 <at> debbugs.gnu.org
Subject: Re: bug#63786: [PATCH] home: services: ssh: Allow unset boolean
 options in ssh-config.
Date: Thu, 08 Jun 2023 22:57:37 +0200

Hello!

Efraim Flashner <efraim <at> flashner.co.il> skribis:

>>From man 5 ssh_config:
> Unless noted otherwise, for each parameter, the first obtained value
> will be used.
>
> We want to allow falling through to the first actual user defined value.

What do you mean by “first actual user-defined value”?  This service is
what generates all the “user-defined values”, no?

Overall my take is that default values should be specified in our code
(as default values of configuration record fields) rather than left
unspecified.  I think this is clearer and more predictable than relying
on upstream’s default values.

Thanks,
Ludo’.

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: 63786 <at> debbugs.gnu.org
Subject: Re: bug#63786: [PATCH] home: services: ssh: Allow unset boolean
Date: Sun, 11 Jun 2023 10:49:58 +0300

[Message part 1 (text/plain, inline)]

options in ssh-config.
Reply-To: 
X-PGP-Key-ID: 0x41AAE7DCCA3D8351
X-PGP-Key: https://flashner.co.il/~efraim/efraim_flashner.asc
X-PGP-Fingerprint: A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351

For some reason this didn't get sent to the bug.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[Message part 2 (message/rfc822, inline)]

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: bug#63786: [PATCH] home: services: ssh: Allow unset boolean
 options in ssh-config.
Date: Fri, 9 Jun 2023 16:24:26 +0300

[Message part 3 (text/plain, inline)]

On Thu, Jun 08, 2023 at 10:57:37PM +0200, Ludovic Courtès wrote:
> Hello!
> 
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> >>From man 5 ssh_config:
> > Unless noted otherwise, for each parameter, the first obtained value
> > will be used.
> >
> > We want to allow falling through to the first actual user defined value.
> 
> What do you mean by “first actual user-defined value”?  This service is
> what generates all the “user-defined values”, no?

Right now my ~/.ssh/config has

Host do1-tor
    Hostname <insert tor address>
    IdentityFile ~/.ssh/id_ed25519
Host *.onion *-tor
    #ProxyCommand /gnu/store/dgvybjrj154f4cyfbkrbqyirv5gd8ic2-netcat-openbsd-1.218-2/bin/nc -X 5 -x localhost:9050 %h %p
    ProxyCommand /home/efraim/bin/openbsd-netcat -X 5 -x localhost:9050 %h %p
    ControlPath ${XDG_RUNTIME_DIR}/%r@%k-%p
    Compression yes

The way the ssh config is read is that `ssh do1-tor` first matches
do1-tor and then also matches *-tor, so I can factor our ProxyCommand,
ControlPath and Compression for use with the other *-tor Hosts I have
listed.

This configuration could be
(openssh-host (name "do1-tor")
              (host-name <insert tor address>)
              (identity-file "~/.ssh/id_ed25519"))
(openssh-host (name "*-onion *-tor)
              (compression? #t)
              (proxy
               (proxy-command ...))
              (extra-content "  ControlPath ...\n"))

If this is all I enter, then my .ssh/config is generated like this:

Host do1-tor
  Hostname <insert tor address>
  IdentityFile ~/.ssh/id_ed25519
  ForwardX11 no
  ForwardX11Trusted no
  ForwardAgent no
  Compression no
Host *.onion *-tor
  ForwardX11 no
  ForwardX11Trusted no
  ForwardAgent no
  Compression yes
  ProxyCommand /home/efraim/bin/openbsd-netcat -X 5 -x localhost:9050 %h %p
  ControlPath ${XDG_RUNTIME_DIR}/%r@%k-%p

Compression might default to no, but in my hand crafted .ssh/config I've
set it to yes for *-tor Hosts. Forward* might all default to no, and
it's not set anywhere, but being explicit about the default here could
cause problems if I want X11 forwarding across an entire range of hosts,
not just individual ones.

> Overall my take is that default values should be specified in our code
> (as default values of configuration record fields) rather than left
> unspecified.  I think this is clearer and more predictable than relying
> on upstream’s default values.

In general this is a good plan, but here it actually interferes with the
expected configuration output. 'Fall through' is the default, not the
actual default for each of the individual configuration options. They
only get set if that field isn't set by any of the possibly multiple
configuration matches set it first.


-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

[signature.asc (application/pgp-signature, inline)]

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

From: Andrew Tropin <andrew <at> trop.in>
To: Efraim Flashner <efraim <at> flashner.co.il>, 63786 <at> debbugs.gnu.org
Subject: Re: [bug#63786] [PATCH] home: services: ssh: Allow unset boolean
Date: Mon, 12 Jun 2023 08:58:18 +0400

[Message part 1 (text/plain, inline)]

On 2023-06-11 10:49, Efraim Flashner wrote:

> options in ssh-config.
> Reply-To: 
> X-PGP-Key-ID: 0x41AAE7DCCA3D8351
> X-PGP-Key: https://flashner.co.il/~efraim/efraim_flashner.asc
> X-PGP-Fingerprint: A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
>
> For some reason this didn't get sent to the bug.
>
> -- 
> Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
> GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
> Confidentiality cannot be guaranteed on emails sent or received unencrypted
> From: Efraim Flashner <efraim <at> flashner.co.il>
> Subject: Re: bug#63786: [PATCH] home: services: ssh: Allow unset boolean options in ssh-config.
> To: Ludovic Courtès <ludo <at> gnu.org>
> Date: Fri, 09 Jun 2023 16:24:26 +0300
>
> On Thu, Jun 08, 2023 at 10:57:37PM +0200, Ludovic Courtès wrote:
>> Hello!
>> 
>> Efraim Flashner <efraim <at> flashner.co.il> skribis:
>> 
>> >>From man 5 ssh_config:
>> > Unless noted otherwise, for each parameter, the first obtained value
>> > will be used.
>> >
>> > We want to allow falling through to the first actual user defined value.
>> 
>> What do you mean by “first actual user-defined value”?  This service is
>> what generates all the “user-defined values”, no?
>
> Right now my ~/.ssh/config has
>
> Host do1-tor
>     Hostname <insert tor address>
>     IdentityFile ~/.ssh/id_ed25519
> Host *.onion *-tor
>     #ProxyCommand /gnu/store/dgvybjrj154f4cyfbkrbqyirv5gd8ic2-netcat-openbsd-1.218-2/bin/nc -X 5 -x localhost:9050 %h %p
>     ProxyCommand /home/efraim/bin/openbsd-netcat -X 5 -x localhost:9050 %h %p
>     ControlPath ${XDG_RUNTIME_DIR}/%r@%k-%p
>     Compression yes
>
> The way the ssh config is read is that `ssh do1-tor` first matches
> do1-tor and then also matches *-tor, so I can factor our ProxyCommand,
> ControlPath and Compression for use with the other *-tor Hosts I have
> listed.
>
> This configuration could be
> (openssh-host (name "do1-tor")
>               (host-name <insert tor address>)
>               (identity-file "~/.ssh/id_ed25519"))
> (openssh-host (name "*-onion *-tor)
>               (compression? #t)
>               (proxy
>                (proxy-command ...))
>               (extra-content "  ControlPath ...\n"))
>
> If this is all I enter, then my .ssh/config is generated like this:
>
> Host do1-tor
>   Hostname <insert tor address>
>   IdentityFile ~/.ssh/id_ed25519
>   ForwardX11 no
>   ForwardX11Trusted no
>   ForwardAgent no
>   Compression no
> Host *.onion *-tor
>   ForwardX11 no
>   ForwardX11Trusted no
>   ForwardAgent no
>   Compression yes
>   ProxyCommand /home/efraim/bin/openbsd-netcat -X 5 -x localhost:9050 %h %p
>   ControlPath ${XDG_RUNTIME_DIR}/%r@%k-%p
>
> Compression might default to no, but in my hand crafted .ssh/config I've
> set it to yes for *-tor Hosts. Forward* might all default to no, and
> it's not set anywhere, but being explicit about the default here could
> cause problems if I want X11 forwarding across an entire range of hosts,
> not just individual ones.
>
>> Overall my take is that default values should be specified in our code
>> (as default values of configuration record fields) rather than left
>> unspecified.  I think this is clearer and more predictable than relying
>> on upstream’s default values.
>
> In general this is a good plan, but here it actually interferes with the
> expected configuration output. 'Fall through' is the default, not the
> actual default for each of the individual configuration options. They
> only get set if that field isn't set by any of the possibly multiple
> configuration matches set it first.

A few years ago, when we were implementing the first version of ssh home
service in rde we went a slightly different way and didn't hardcode any
record fields and let user set an alist of key/value pairs:
https://git.sr.ht/~abcdw/rde/tree/19c2d2f0996624eea8b7a87b14bbc31e4a9b943b/src/gnu/home-services/ssh.scm#L204

It's not a perfect solution either, but quite flexible.  Also, it's
relatively easy to implement default values: we can provide
%default-host-options and ask people to do something like this on user
side configuration:

(merge %default-host-options '((compression . #f)))

Of course "asking people" won't work, so it's possible to set a default
value of options field to %default-host-options
https://git.sr.ht/~abcdw/rde/tree/19c2d2f0996624eea8b7a87b14bbc31e4a9b943b/src/gnu/home-services/ssh.scm#L100
and let people override it with '((compression . #f)) or enrich with
(merge %default-host-options '((compression . #f))).

It's not a proposal or something, just sharing how it's implemented in
rde.


P.S. Note that (gnu home-services *) modules are subject to deprecation
and when (rde home services ssh) appear, it will have a slightly
different interface.

-- 
Best regards,
Andrew Tropin

[signature.asc (application/pgp-signature, inline)]

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

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 63786-done <at> debbugs.gnu.org
Subject: Re: [bug#63786] [PATCH] home: services: ssh: Allow unset boolean
Date: Wed, 14 Jun 2023 22:16:47 +0300

[Message part 1 (text/plain, inline)]

On Mon, Jun 12, 2023 at 08:58:18AM +0400, Andrew Tropin wrote:
> 
> A few years ago, when we were implementing the first version of ssh home
> service in rde we went a slightly different way and didn't hardcode any
> record fields and let user set an alist of key/value pairs:
> https://git.sr.ht/~abcdw/rde/tree/19c2d2f0996624eea8b7a87b14bbc31e4a9b943b/src/gnu/home-services/ssh.scm#L204
> 
> It's not a perfect solution either, but quite flexible.  Also, it's
> relatively easy to implement default values: we can provide
> %default-host-options and ask people to do something like this on user
> side configuration:
> 
> (merge %default-host-options '((compression . #f)))
> 
> Of course "asking people" won't work, so it's possible to set a default
> value of options field to %default-host-options
> https://git.sr.ht/~abcdw/rde/tree/19c2d2f0996624eea8b7a87b14bbc31e4a9b943b/src/gnu/home-services/ssh.scm#L100
> and let people override it with '((compression . #f)) or enrich with
> (merge %default-host-options '((compression . #f))).
> 
> It's not a proposal or something, just sharing how it's implemented in
> rde.

I'm still undecided about the alist as a comparison. It would make it
easier to add arbitrary fields, but then I feel like maybe we should be
adding something to validate the configurations.

> P.S. Note that (gnu home-services *) modules are subject to deprecation
> and when (rde home services ssh) appear, it will have a slightly
> different interface.


I went ahead and pushed the patch. I believe that, after having added to
a .ssh/config file over a period of time, line by line or entry by
entry, people will be surprised to see a bunch of fields filled in
automatically, and with different results from what they had before.


-- 
Efraim Flashner   <efraim <at> flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Previous Next

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