guix copy incorrectly assumes port is 22

  • Open
  • quality assurance status badge
Details
4 participants
  • Dariqq
  • Maxim Cournoyer
  • Simon Tournier
  • Tomas Volf
Owner
unassigned
Submitted by
Dariqq
Severity
normal
Merged with
D
D
Dariqq wrote on 12 Dec 2024 17:45
(address . bug-guix@gnu.org)
bd64a6c0-3a38-4fe2-9315-c4ff6be02e91@posteo.net
Hello,

Here is bug report number 3 with guile-ssh@0.18.

When using a host defined in ~/.ssh/config in the --to/--from argument
in guix-copy "send-to-remote-host" and "retrieve-from-remote-host"
incorrectly pass the port as 22 to open-ssh-session.


This then leads to a failure when trying to connect:
Toggle snippet (7 lines)
guix copy hello --to=name
guix copy: error: failed to authenticate server at 'domain': not-known



With guile-ssh@0.17 guile-ssh silently ignored the "wrong port" and
instead connects to the one specified by the ssh Host

Toggle snippet (9 lines)
guix copy hello --to=name

with guile-ssh@0.17 :
#<session dariqq@domain:10022 (disconnected) 7f21d88a2fe0>

with-guile-ssh@0.18:
#<session dariqq@localhost:22 (disconnected) 7f17887a2fe0>

Are the (or port 22) clauses in guix/scripts/copy.scm still neccesary?

From my limited testing removing them fixed the problem and passing a
port of #f will result in 22 being used.

This might also be a problem in other places wghere open-ssh-session is
used?
T
T
Tomas Volf wrote on 12 Dec 2024 18:35
(name . Dariqq)(address . dariqq@posteo.net)(address . 74832@debbugs.gnu.org)
87msh0n6se.fsf@wolfsden.cz
Hi,

Dariqq <dariqq@posteo.net> writes:

Toggle quote (5 lines)
> Are the (or port 22) clauses in guix/scripts/copy.scm still neccesary?
>
> From my limited testing removing them fixed the problem and passing a
> port of #f will result in 22 being used.

Yeah I think you are right.

Toggle quote (4 lines)
>
> This might also be a problem in other places wghere open-ssh-session is
> used?

Will look them over and send a patch.

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
-----BEGIN PGP SIGNATURE-----

iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmdbHuEOHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/wak2qg//bxVsamqFAvenMNaFy3uqonKhsA6kepNaA6Gu
qKJkt6HUQdlwMiNe4lf1mBju5lEVOjXB1X4y9vQ4N5gvuT+5OdH4OBipCCoI7J68
PUC1d70ketK72B14bnWT0LJJ/np++ugQCjJ9q52uAGYGwU0miQHhhz5VORSJD753
QIgsgHyxiBB87fY+oNCRG9XQaDPE9e6WitesXSGQY7cWtTxfs18O11kSakzMl97S
gmQWCX6Et09ghSHzbu2rs5XSYPS85jwpd9woj1s9cMLBrXTCYVAeWq7/LCxaxb22
ICOpxWUZihxoJYONSkRk8ZWW1r+KmFMcMZRFR9koaayROIJO/IV1ytxzpR/j676c
qm+5ZZcltEfasPJED/kW4D6dgodieHC4/3JFOsh14a5nAqe+ypKulQuHj3Q3DwRV
jrhwBvo/Umme0rK2oWjXR/bMCxQEt79q4ROuptT2xlYHVbV77eoOF3jFrWcpmzGl
uWapeuzVgRKvkERUJOX+e6m5uqcnGPb4INvGFTro42p/HJBUJwfucbjJFlL+8u1J
yvWwXiT+BIFwtxhNr7esQZlX6dQww1LYRjNPDvwyYcFflfwS710hnYA+3oM3RVyB
4o7b6QSVU1dpVDHEbUtgdAwInXjEmWBDxXhM32ArGBFXoVfSSWmRvPKmN9zwQIpY
EOeyT9w=
=jFkk
-----END PGP SIGNATURE-----

T
T
Tomas Volf wrote on 12 Dec 2024 20:31
[PATCH] guix: Do not default to 22 ssh port (let guile-ssh do it).
(address . 74832@debbugs.gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
a3e2370fc79301d5de4aa242a3a81083d28cebb0.1734031864.git.~@wolfsden.cz
After update to guile-ssh 0.18.0, options passed to the `make-session'
procedure now take precedence over the configuration file. In few places we
however had code like `(or port 22)' leading to (in absence of alternative
port being specified) always using port 22, ignoring the configuration file.

Due to that for example following command fails:

guix copy hello --to=name

Name is reachable, but ssh server listens on port 2222. That is correctly
configured in ~/.ssh/config, and the invocation used to succeed until the
upgrade. However now it tries to connect to port 22 (since port was not
specified). While setting the port on the command line *is* possible, it is
not exactly ergonomic.

Since guile-ssh (well, libssh) defaults to 22 if not told otherwise, we can
just always pass the port, and #f will use the port from ~/.ssh/config or, iff
none is set, 22.

I went through the repository and adjusted all places where it seemed
appropriate. In particular, these places were left alone:

gnu/machine/digital-ocean.scm: The droplet is created with root user and the
expected key, so forcing them to those values seems correct.

gnu/machine/ssh.scm: For deployments reproducibility is favored over
convenience, and user can pass #f to explicitly request using value the
~/.ssh/config.

* guix/scripts/copy.scm (send-to-remote-host): Always pass the port to
open-ssh-session.
(retrieve-from-remote-host): Same.
* guix/scripts/offload.scm (open-ssh-session): Pass #f as #:config. Skips
reading the configuration file and is nicer.
* guix/ssh.scm (open-ssh-session): Drop explicit parsing of the configuration
since it is parsed by default. Report actual port used in the error message.
* guix/store/ssh.scm (connect-to-daemon): Always pass the port part of the
uri, even when #f.

Change-Id: I5fdf20f36509a9a0ef138ce72c7198f688eea494
---
I did few more tweaks than strictly required, feel free to discard them.

guix/scripts/copy.scm | 5 ++---
guix/scripts/offload.scm | 2 +-
guix/ssh.scm | 8 +++-----
guix/store/ssh.scm | 2 +-
4 files changed, 7 insertions(+), 10 deletions(-)

Toggle diff (75 lines)
diff --git a/guix/scripts/copy.scm b/guix/scripts/copy.scm
index 67975ac1a9..116583590f 100644
--- a/guix/scripts/copy.scm
+++ b/guix/scripts/copy.scm
@@ -75,8 +75,7 @@ (define (send-to-remote-host local target opts)
(options->derivations+files local opts)))
(warn-if-empty items)
(and (build-derivations local drv)
- (let* ((session (open-ssh-session host #:user user
- #:port (or port 22)))
+ (let* ((session (open-ssh-session host #:user user #:port port))
(remote (connect-to-remote-daemon session))
(sent (send-files local items remote
#:recursive? #t)))
@@ -89,7 +88,7 @@ (define (retrieve-from-remote-host local source opts)
(let*-values (((user host port)
(ssh-spec->user+host+port source))
((session)
- (open-ssh-session host #:user user #:port (or port 22)))
+ (open-ssh-session host #:user user #:port port))
((remote)
(connect-to-remote-daemon session)))
;; TODO: Here we could to compute and build the derivations on REMOTE
diff --git a/guix/scripts/offload.scm b/guix/scripts/offload.scm
index 93e9d3759c..ccf989a881 100644
--- a/guix/scripts/offload.scm
+++ b/guix/scripts/offload.scm
@@ -234,7 +234,7 @@ (define* (open-ssh-session machine #:optional max-silent-time)
#:knownhosts "/dev/null"

;; Likewise for ~/.ssh/config.
- #:config "/dev/null"
+ #:config #f

;; We need lightweight compression when
;; exchanging full archives.
diff --git a/guix/ssh.scm b/guix/ssh.scm
index ae506df14c..5e89997df3 100644
--- a/guix/ssh.scm
+++ b/guix/ssh.scm
@@ -138,10 +138,6 @@ (define* (open-ssh-session host #:key user port identity
;; Speed up RPCs by creating sockets with
;; TCP_NODELAY.
#:nodelay #t)))
-
- ;; Honor ~/.ssh/config.
- (session-parse-config! session)
-
(match (connect! session)
('ok
(if host-key
@@ -181,7 +177,9 @@ (define* (open-ssh-session host #:key user port identity
(x
;; Connection failed or timeout expired.
(raise (formatted-message (G_ "SSH connection to '~a' port ~a failed: ~a~%")
- host (or port 22) (get-error session)))))))
+ host
+ (session-get session 'port)
+ (get-error session)))))))

(define* (remote-inferior session #:optional become-command)
"Return a remote inferior for the given SESSION. If BECOME-COMMAND is
diff --git a/guix/store/ssh.scm b/guix/store/ssh.scm
index 09c0832505..7e6371acbc 100644
--- a/guix/store/ssh.scm
+++ b/guix/store/ssh.scm
@@ -33,7 +33,7 @@ (define (connect-to-daemon uri)
"Connect to the SSH daemon at URI, a URI object with the 'ssh' scheme."
(remote-daemon-channel
(open-ssh-session (uri-host uri)
- #:port (or (uri-port uri) 22)
+ #:port (uri-port uri)
#:user (uri-userinfo uri))))

;;; ssh.scm ends here
--
2.46.0
S
S
Simon Tournier wrote on 13 Dec 2024 11:36
control message for bug #74832
(address . control@debbugs.gnu.org)
87h6776f9m.fsf@gmail.com
tags 74832 + patch
quit
M
M
Maxim Cournoyer wrote on 17 Dec 2024 02:25
(address . control@debbugs.gnu.org)
87ldwfrtgb.fsf@gmail.com
merge 74832 33266
quit
M
M
Maxim Cournoyer wrote on 19 Dec 2024 03:33
Re: bug#74832: guix copy incorrectly assumes port is 22
(name . Tomas Volf)(address . ~@wolfsden.cz)
87ikrgmmfm.fsf_-_@gmail.com
Hi Tomas,

Tomas Volf <~@wolfsden.cz> writes:

Toggle quote (13 lines)
> After update to guile-ssh 0.18.0, options passed to the `make-session'
> procedure now take precedence over the configuration file. In few places we
> however had code like `(or port 22)' leading to (in absence of alternative
> port being specified) always using port 22, ignoring the configuration file.
>
> Due to that for example following command fails:
>
> guix copy hello --to=name
>
> Name is reachable, but ssh server listens on port 2222. That is correctly
> configured in ~/.ssh/config, and the invocation used to succeed until the
> upgrade.

That is curious, because I had reported the exact same problem 6 years
ago (!) in bug#33266 (now merged with this one), with a similar
solution:

Toggle snippet (11 lines)
Subject: [PATCH] Revert "copy: Default to port 22."

This reverts commit cc1dfc202f2fefb6c2eb9467d1fc90a9154550c9. Specifying a
default port had the undesirable effect of disregarding a port specification
for a given host in the ~/.ssh/config that would otherwise have been honored
at the time `open-ssh-session' calls the `session-parse-config!' method.

In any case, `make-session' will default the port value of the created session
to 22 if left unspecified.

But, Ludovic had mentioned that without it,

Toggle snippet (3 lines)
[...] "%p" would be "0" when using "ProxyCommand" in ~/.ssh/config.

So it'd perhaps regress in another way; I want to retry the test I had
done then but I need to setup at least a VM with SSH to test. If you
can beat me to that, all the better :-).

--
Thanks,
Maxim
T
T
Tomas Volf wrote on 19 Dec 2024 10:30
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
8734ik6mv4.fsf@wolfsden.cz
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (40 lines)
> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
>> After update to guile-ssh 0.18.0, options passed to the `make-session'
>> procedure now take precedence over the configuration file. In few places we
>> however had code like `(or port 22)' leading to (in absence of alternative
>> port being specified) always using port 22, ignoring the configuration file.
>>
>> Due to that for example following command fails:
>>
>> guix copy hello --to=name
>>
>> Name is reachable, but ssh server listens on port 2222. That is correctly
>> configured in ~/.ssh/config, and the invocation used to succeed until the
>> upgrade.
>
> That is curious, because I had reported the exact same problem 6 years
> ago (!) in bug#33266 (now merged with this one), with a similar
> solution:
>
> Subject: [PATCH] Revert "copy: Default to port 22."
>
> This reverts commit cc1dfc202f2fefb6c2eb9467d1fc90a9154550c9. Specifying a
> default port had the undesirable effect of disregarding a port specification
> for a given host in the ~/.ssh/config that would otherwise have been honored
> at the time `open-ssh-session' calls the `session-parse-config!' method.
>
> In any case, `make-session' will default the port value of the created session
> to 22 if left unspecified.
>
>
> But, Ludovic had mentioned that without it,
>
> [...] "%p" would be "0" when using "ProxyCommand" in ~/.ssh/config.
>
> So it'd perhaps regress in another way; I want to retry the test I had
> done then but I need to setup at least a VM with SSH to test. If you
> can beat me to that, all the better :-).

I wonder whether VM is necessary. I added the following to my
~/.ssh/config file:

Toggle snippet (5 lines)
host name
port 2222
proxycommand echo %p >/tmp/port

Then I executed guix copy:

Toggle snippet (4 lines)
$ guix copy hello --to=name
guix copy: error: SSH connection to 'name' port 2222 failed: Socket error: Connection reset by peer

And after that I checked /tmp:

Toggle snippet (4 lines)
$ cat /tmp/port
2222

So it seems to work fine? Would not hurt if someone double checked
(with the patch above applied).

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
-----BEGIN PGP SIGNATURE-----

iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmdj578OHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/wamP6Q/7B4ikDK1ixCco6JTps/lxAI5Vs36yeS3hbjHH
BrxvNT8b1HbdIY2yXkM81AIPbYaEnZ8mRUPLmDbyMD4Byrz47YmF1xbhUyS+WUg0
PM5STiimR4Uh87XuKaH9EwclTsg1ic7eBHRA7BqveSbY7BdE/y3C3m8evvg+ak7f
TTdbnYg0MZFEkZ1Fd8P4i1DpXaPyFS9xD4XFH+ND4dOFK10rFHUpAzRvZYKhAs3z
9AgrtlwSP74e7czUCGzhkyaCcNOu7lWdzgDLu1kxaZ08CxXqFOkeRPWLm7h8M0/v
TdLeru7h3KSeJGHPF/aLx9eFYohVKZLlV9KHIRDc/a788DmWPNHMEcnq9wPWyYSI
kUSR62xoS1n5enwMou0FdC2UhrdBKOW3mXo09lNU73+871kXMwjNDa0eFe67cwAW
bt4eJN6d/bololtI3wJuRa549fmVCSmfMw1Dq6g8NauisA5+QbQdoCMVWVpZMggz
Go/+yKhR88uX1XHhq2rWqGj0V7LIppXYS1N/VVlu1ROXH8+VAbgeqDTryVykrnjI
BJDbcBqVRQeOQ7/NVsSGmca8SI2q5vB/VQ0lEl6P2vRUMhoot8Kumr0nTrghXEhr
zKdQ/VZb19yYgS6unIAkIbOI5mgmbN+Vd5WXvQQ+qtKOzzD5MTIducfsEtfYNuyx
hS1+49s=
=JWzT
-----END PGP SIGNATURE-----

?
Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 74832
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