bug#74832: guix copy incorrectly assumes port is 22

bug#74832: guix copy incorrectly assumes port is 22

[Dariqq]

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:
--8<---------------cut here---------------start------------->8---

guix copy hello --to=name
guix copy: error: failed to authenticate server at 'domain': not-known

--8<---------------cut here---------------end--------------->8---


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

--8<---------------cut here---------------start------------->8---
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>
--8<---------------cut here---------------end--------------->8---

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?

bug#74832: guix copy incorrectly assumes port is 22

[Tomas Volf]

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


Hi,

Dariqq <dariqq@posteo.net> writes:

> 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.

>
> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

bug#74832: [PATCH] guix: Do not default to 22 ssh port (let guile-ssh do it).

[Tomas Volf]

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(-)

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

bug#74832: guix copy incorrectly assumes port is 22

[Maxim Cournoyer]

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:

--8<---------------cut here---------------start------------->8---
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.
--8<---------------cut here---------------end--------------->8---

But, Ludovic had mentioned that without it,

--8<---------------cut here---------------start------------->8---
[...] "%p" would be "0" when using "ProxyCommand" in ~/.ssh/config.
--8<---------------cut here---------------end--------------->8---

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

bug#74832: guix copy incorrectly assumes port is 22

[Tomas Volf]

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

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> 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:

--8<---------------cut here---------------start------------->8---
host name
port 2222
proxycommand echo %p >/tmp/port
--8<---------------cut here---------------end--------------->8---

Then I executed guix copy:

--8<---------------cut here---------------start------------->8---
$ guix copy hello --to=name
guix copy: error: SSH connection to 'name' port 2222 failed: Socket error: Connection reset by peer
--8<---------------cut here---------------end--------------->8---

And after that I checked /tmp:

--8<---------------cut here---------------start------------->8---
$ cat /tmp/port 
2222
--8<---------------cut here---------------end--------------->8---

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]