moul / assh

:computer: make your ssh client smarter
https://manfred.life/assh
MIT License
3.05k stars 156 forks source link

Port number not changing with multiple inherits #268

Open salt-lick opened 6 years ago

salt-lick commented 6 years ago

I noticed that if you include multiple inheritances, any port number configuration is not picked up or applied to the connection.

Setup:

 local_box:
    Hostname: 1.2.3.4
    IdentityFile: [redacted]
    Inherits:
    - root_user
    - port_change

templates:
  root_user:
    User: root
  port_change:
    Port: 87

Debug Output:

INFO[0000] Host local_box
INFO[0000]   IdentityFile [redacted]
INFO[0000]   Port 22
INFO[0000]   User root
INFO[0000]   # HostName: 1.2.3.4
INFO[0000]   # Inherits: [root_user, port_change]

As you see, the port is not being overwritten with the config from the inheritance and the connection is not made because of the port misconfiguration.

If I add Port: 87 to the root_user template, the connection is made correctly.

 local_box:
    Hostname: 1.2.3.4
    IdentityFile: [redacted]
    Inherits:
    - root_user

templates:
  root_user:
    User: root
    Port: 87

Thoughts? Am I just doing something wrong?

Thanks!

4wrxb commented 6 years ago

So I just ran into this and started poking around. The issue seems to start from ApplyDefaults in host.go (ll 1076)

    // Extra defaults
    if h.Port == "" {
        h.Port = "22"
    }

I don't fully understand the flow yet, but it would appear that only hosts with an inheritance end up with 22 in the config. Compare:

Host *github.com
  StrictHostKeyChecking no
  User git
  UserKnownHostsFile /dev/null
  # Gateways: [direct]

to this (neither the z_guis template nor the media host specifies a port in the config)

Host media
  Compression yes
  ForwardX11 yes
  Port 22
  User media
  # HostName: 10.0.2.4
  # Inherits: [z_guis]
  # Gateways: [direct, my-dmz]

So, there are (at least) two separate fall-outs from this 1) Precedence is broken, as reported by @salt-lick and just encountered by myself. 2) The defaults section in the user's assh config is effectively overridden if there is an inheritance (without port numbers).

From an intent standpoint I don't think assh should ever need to specify 22 unless it's trying to override the system-wide ssh_config for some reason (but even then, why? why only for ports?). Ideally, if no port is specified in the user's config there shouldn't be any such declaration in the resulting ssh config.

With my limited knowledge of what's being run where and from the perspective of my needs (and some limited examples I tested) I'm not sure what to do. Both the below change and simply removing all the lines seem to have the same result.

    // Extra defaults
    if h.Port == "" {
-       h.Port = "22"
+       h.Port = defaults.Port
    }

I don't have time at the moment to look beyond this and understand how ApplyDefaults should work, but before I make any sort of PR I would want to understand what is happening here and what may break either way.

P.S. In grepping I also noticed that the "PrototypeHost" function has similar logic. Here, it could make sense from a "make the output consistent" perspective. But still, considering the program doesn't know the system-level SSH config it may be better to simply omit. In trying that I noticed that inheritance is completely ignored when printing the host list (for both port and user). I'm guessing this is simply unimplimented, but when I get back to things I'll double check before filing an issue for that.