francoismichel / ssh3

SSH3: faster and rich secure shell using HTTP/3, checkout our article here: https://arxiv.org/abs/2312.08396 and our Internet-Draft: https://datatracker.ietf.org/doc/draft-michel-ssh3/
https://arxiv.org/abs/2312.08396
Apache License 2.0
3.19k stars 82 forks source link

bug: SSH configuration file takes precedence over CLI #35

Closed TheoTechnicguy closed 6 months ago

TheoTechnicguy commented 6 months ago

:wave: ! Let me start with saying that I absolutely :heart: the idea! I currently use OIDC to create SSH certificates, which works, but requires external helpers. I think it would be great to streamline the authentication process!

The bug

I found that the client prefers the configuration from the file over the CLI host/port.

11:00PM DBG parsing user known hosts
11:00PM DBG parsed known hosts Certificates=0 InvalidLines=0
11:00PM DBG got url ConnectionURL=https://127.0.0.1:41443/
11:00PM DBG got command cmd="echo hi ssh3!"
11:00PM DBG parsed ssh config ConfigBytes=2848 SSHConfigFilePath=/home/theo/.ssh/config
11:00PM DBG parsed URL ParsedURL={"ForceQuery":false,"Fragment":"","Host":"127.0.0.1:41443","OmitHost":false,"Opaque":"","Path":"/","RawFragment":"","RawPath":"","RawQuery":"","Scheme":"https","User":null}
11:00PM DBG parsed url and port Host=127.0.0.1 Port=41443
11:00PM DBG fetched Host config ConfigHost=0.0.0.0 ConfigPort=9922 ConfigUser=theo Host=127.0.0.1 Port=41443
11:00PM DBG set hostname ConfigHostname=0.0.0.0 DialHostname=0.0.0.0 URLHostname=127.0.0.1
11:00PM DBG checked if hostname is IP HostnameIsIP=true
11:00PM DBG set port ConfigPort=9922 Port=9922 URLPort=41443
11:00PM DBG dialing QUIC host at 0.0.0.0:9922
11:00PM ERR could not establish client QUIC connection: timeout: no recent network activity
exit status 255

My ~/.ssh/config

[ other host configurations ]

Host *
  User theo
  Port 9922

For me, this qualifies as a bug because, even though my default hostname is 0.0.0.0 and port is 9922, not all run on this port (most, actually, don't). If host 127.0.0.1 (or port 22) is specified, I actually want to connect to 127.0.0.1. I would find it cumbersome to have to change the configuration file every time I want to connect to a server with a different hostname/port. Granted, it is unusual to have a default hostname, but having a default custom port is not, Imo.

Using the CLI arguments is actually the behaviour of OpenSSH-client (see man 5 ssh_config).

My solution

I would change the variable setting to first consider the hostname/IP/port/... from the CLI and fill the missing fields from the configuration files.

I believe it is enough to swap lines 413 and 415 in cli/client/main.go for the hostname

    hostname := configHostname
    if hostname == "" {
        hostname = urlHostname
    }
    hostname := urlHostname
    if hostname == "" {
        hostname = configHostname
    }

and 420/422 for the port.

    port := configPort
    if port == -1 && urlPort != "" {
        port, err = strconv.Atoi(urlPort)
        if err != nil {
            log.Error().Msgf("invalid port number: %s: %s", urlPort, err)
            return -1
        }
    }
    port, err := strconv.Atoi(urlPort)
    if err != nil && configPort == -1 {
        log.Fatal().Msg("No valid port is configured!")
        return -1
    } else if err != nil {
        port = configPort
    }

This works on my machine(tm).

As this repository does not have any guidelines on contributing, I am rasing this issue. If you advise me on how I should contribute, I would be happy to help out ^-^

francoismichel commented 6 months ago

Hey, thanks for the support and the feedback !

It is true that at least the port should be get precedence when provided from the cli, so if you're up to do a PR for handling the port, it is very much appreciated. :smile:

For the HostName, I am not totally sure where the 0.0.0.0 comes from in your example, as I can't see it in your config. When doing a test on OpenSSH version 9.3p1 with the following config:

Host *
  HostName 127.0.0.2
  Port 3333

, with the following command I obtain:

user@localhost:~$ ssh -v user@127.0.0.1 -p 2222
OpenSSH_9.3p1, OpenSSL 3.1.1 30 May 2023
debug1: Reading configuration data /home/user/.ssh/config
debug1: /home/user/.ssh/config line 1: Applying options for *
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: Reading configuration data /etc/ssh/ssh_config.d/50-redhat.conf
debug1: Reading configuration data /etc/crypto-policies/back-ends/openssh.config
debug1: configuration requests final Match pass
debug1: re-parsing configuration
debug1: Reading configuration data /home/user/.ssh/config
debug1: /home/user/.ssh/config line 1: Applying options for *
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: Reading configuration data /etc/ssh/ssh_config.d/50-redhat.conf
debug1: Reading configuration data /etc/crypto-policies/back-ends/openssh.config
debug1: Connecting to 127.0.0.2 [127.0.0.2] port 2222.
debug1: connect to address 127.0.0.2 port 2222: Connection refused
ssh: connect to host 127.0.0.2 port 2222: Connection refused

It tries to connect to port 2222 given in the CLI but on host 127.0.0.2 provided in the config. So it looks the CLI takes precedence for the port, but not the hostname (which makes sense to me in this case).

So it seems we only need to update the precedence for the port, do you agree ?

TheoTechnicguy commented 6 months ago

Sorry for the omitted Hostname configuration value. I played around while writing the issue and have forgotten to include that.

I have tested the hostname precedence, and get the same behaviour using 9.5p1. Although the man page does not exempt this behaviour from the standard priority, I am guessing that it would be difficult to prioritize CLI hosts that do not match any canonicalized hostnames but still are FQDNs or IP addresses.

Thank you for your input, I will consider all of this in my PR!

francoismichel commented 6 months ago

Fixed by https://github.com/francoismichel/ssh3/pull/45