diff --git a/internal/cli/node.go b/internal/cli/node.go index 681107b..c29002e 100644 --- a/internal/cli/node.go +++ b/internal/cli/node.go @@ -114,7 +114,7 @@ engines are kept.`, } f := cmd.Flags() f.StringVar(&na.opts.Node, "node", "", "node id (default: the machine's hostname)") - f.StringVar(&na.opts.User, "ssh-user", "", "user to log in as (default: the one before @ in the address, else whoever ran sudo, else you)") + f.StringVar(&na.opts.User, "ssh-user", "", "user to log in as (default: the one before @ in the address, else the ssh config's for the machine, else whoever ran sudo, else you)") f.StringArrayVar(&na.servers, "server", nil, "control plane address the agent dials, https://host:7443; repeatable for failover (default: this machine)") f.StringArrayVar(&na.fabrics, "fabric", nil, "InfiniBand or RoCE fabric and its adapters, name:mlx5_0,mlx5_1; repeatable") f.StringVar(&na.opts.MgmtAddr, "mgmt-addr", "", "address engines are reached at (default: found by the agent)") @@ -228,8 +228,9 @@ func (na *nodeAdd) target(addr string) onboard.Options { if o.User == "" { // Under sudo, the person who ran it — their account is usually the // one that exists on the nodes too. root's own keys still do the - // logging in, since that is who is running ssh. - o.User = os.Getenv("SUDO_USER") + // logging in, since that is who is running ssh. Only a fallback: a + // User in the ssh config's block for the machine comes first. + o.FallbackUser = os.Getenv("SUDO_USER") } return o } diff --git a/internal/onboard/onboard.go b/internal/onboard/onboard.go index 9faa07e..a67af9a 100644 --- a/internal/onboard/onboard.go +++ b/internal/onboard/onboard.go @@ -79,6 +79,9 @@ type Options struct { // Addr is how to reach the machine over SSH, and User who to log in as. Addr, User string + // FallbackUser logs in when User is empty and the ssh config names no + // user for the machine. + FallbackUser string // Node is its id in the cluster. Empty means its hostname, which is what // it should be: the node id, the hostname and the certificate's name // being one string saves an afternoon each time they would differ. @@ -193,7 +196,7 @@ type Runner struct { // New makes a Runner that reaches the machine with the system's ssh. func New(o Options, emit func(Event)) *Runner { - return &Runner{Opts: o, Remote: &SSH{Addr: o.Addr, User: o.User}, Emit: emit} + return &Runner{Opts: o, Remote: &SSH{Addr: o.Addr, User: o.User, FallbackUser: o.FallbackUser}, Emit: emit} } func (r *Runner) emit(step, level, detail, fix string) { @@ -302,6 +305,9 @@ func (r *Runner) connect(ctx context.Context, res *Result) error { } func (r *Runner) target() string { + if s, ok := r.Remote.(*SSH); ok { + return s.Target(context.Background()) + } if r.Opts.User == "" { return r.Opts.Addr } diff --git a/internal/onboard/ssh.go b/internal/onboard/ssh.go index 6234981..b526cb1 100644 --- a/internal/onboard/ssh.go +++ b/internal/onboard/ssh.go @@ -5,6 +5,7 @@ import ( "context" "os/exec" "runtime" + "sync" ) // Remote runs a shell command on the machine being onboarded, with input. @@ -14,8 +15,39 @@ type Remote interface { // SSH is Remote through the system's ssh, so the operator's keys, agent and // ~/.ssh/config apply exactly as they do when they log in by hand. +// +// Addr is looked up in the ssh config first: a Host block whose HostName is +// Addr is connected through by its name, so its User, IdentityFile and +// ProxyJump apply to a machine given by address, without the address +// having to be added to its Host line. type SSH struct { Addr, User string + // FallbackUser logs in when User is empty and the ssh config names no + // user for the machine: under sudo, the person who ran it. + FallbackUser string + + once sync.Once + target string +} + +// Target is what ssh is given: [user@]host, host being the ssh config's +// name for Addr when it has one. +func (s *SSH) Target(ctx context.Context) string { + s.once.Do(func() { + host := s.Addr + if a := configAlias(ctx, s.Addr); a != "" { + host = a + } + u := s.User + if u == "" && configUser(ctx, host) == "" { + u = s.FallbackUser + } + s.target = host + if u != "" { + s.target = u + "@" + host + } + }) + return s.target } // sshOptions never prompt: a question nobody is there to answer would hang @@ -37,10 +69,7 @@ var sshOptions = []string{ // Run runs script with sh on the machine. Files travel on standard input, // not scp, which a hardened sshd may not offer. func (s *SSH) Run(ctx context.Context, script string, stdin []byte) ([]byte, error) { - target := s.Addr - if s.User != "" { - target = s.User + "@" + s.Addr - } + target := s.Target(ctx) // "--" before the target: an address beginning with "-" is not read as // an option. args := append(append([]string{}, sshOptions...), "--", target, "sh", "-c", shellQuote(script)) diff --git a/internal/onboard/sshconfig.go b/internal/onboard/sshconfig.go new file mode 100644 index 0000000..bac8091 --- /dev/null +++ b/internal/onboard/sshconfig.go @@ -0,0 +1,223 @@ +package onboard + +import ( + "bufio" + "bytes" + "context" + "net" + "os" + "os/exec" + "os/user" + "path/filepath" + "strings" + "time" +) + +// Where ssh reads its configuration, and arguments every `ssh -G` here is +// given; variables so a test can point them at a file of its own. +var ( + userSSHConfig = func() string { return filepath.Join(sshHome(), ".ssh", "config") } + systemSSHConfig = "/etc/ssh/ssh_config" + sshExtraArgs []string +) + +// sshHome is the home ssh itself reads ~/.ssh from: the account's, from the +// password database, not $HOME, which sudo may leave as the caller's. +func sshHome() string { + if u, err := user.Current(); err == nil && u.HomeDir != "" { + return u.HomeDir + } + h, _ := os.UserHomeDir() + return h +} + +// configAlias is the ssh config's name for addr: the first concrete Host +// whose HostName is addr, or resolves to the same address. A machine is +// usually written in the config under its name, `Host gpu01` with +// `HostName 10.0.0.21` and the User, IdentityFile or ProxyJump it needs, +// and given to node add by its address, which that block does not match. +// Empty when addr is itself named on a Host line, or no block points at it. +func configAlias(ctx context.Context, addr string) string { + aliases := map[string]bool{} + var order []string + seen := map[string]bool{} + collect := func(path, base string) { + for _, h := range hostNames(path, base, seen, 0) { + if !aliases[h] { + aliases[h] = true + order = append(order, h) + } + } + } + collect(userSSHConfig(), filepath.Join(sshHome(), ".ssh")) + if systemSSHConfig != "" { + collect(systemSSHConfig, filepath.Dir(systemSSHConfig)) + } + for h := range aliases { + if strings.EqualFold(h, addr) { + return "" + } + } + if len(order) > 256 { + order = order[:256] + } + // Names are looked up only when no HostName is the address as written, + // and within one budget: a config of many hosts must not hold up a + // connection by seconds each. + var pending []string + hostOf := map[string]string{} + for _, a := range order { + h := sshGet(ctx, a)["hostname"] + if strings.EqualFold(h, addr) { + return a + } + if h != "" { + hostOf[a] = h + pending = append(pending, a) + } + } + if len(pending) == 0 { + return "" + } + lctx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + want := lookup(lctx, addr) + if len(want) == 0 { + return "" + } + for _, a := range pending { + for ip := range lookup(lctx, hostOf[a]) { + if want[ip] { + return a + } + } + } + return "" +} + +// configUser is the user the ssh config logs in to host as, empty when it +// names none. ssh -G reports the local account when nothing set one, so +// that is taken as none: the config saying `User root` to a root caller is +// the one case read wrongly, and it gets the fallback user instead. +func configUser(ctx context.Context, host string) string { + u := sshGet(ctx, host)["user"] + if cur, err := user.Current(); err == nil && u == cur.Username { + return "" + } + return u +} + +// sshGet is ssh's own reading of its configuration for host, keys +// lowercased; empty when ssh cannot say. +func sshGet(ctx context.Context, host string) map[string]string { + args := append(append([]string{"-G"}, sshExtraArgs...), "--", host) + out, err := exec.CommandContext(ctx, "ssh", args...).Output() + if err != nil { + return nil + } + m := map[string]string{} + sc := bufio.NewScanner(bytes.NewReader(out)) + for sc.Scan() { + k, v, ok := strings.Cut(sc.Text(), " ") + if k = strings.ToLower(k); ok { + if _, dup := m[k]; !dup { + m[k] = v + } + } + } + return m +} + +func lookup(ctx context.Context, host string) map[string]bool { + if ip := net.ParseIP(host); ip != nil { + return map[string]bool{ip.String(): true} + } + addrs, err := net.DefaultResolver.LookupHost(ctx, host) + if err != nil { + return nil + } + m := map[string]bool{} + for _, a := range addrs { + if ip := net.ParseIP(a); ip != nil { + m[ip.String()] = true + } + } + return m +} + +// hostNames are the concrete names on the Host lines of an ssh config file +// and the files it includes — no patterns, no negations. base is where a +// relative Include is looked for, as ssh does: ~/.ssh for the user's file, +// /etc/ssh for the system's. +func hostNames(path, base string, seen map[string]bool, depth int) []string { + if depth > 16 || seen[path] { + return nil + } + seen[path] = true + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var names []string + for _, line := range strings.Split(string(data), "\n") { + key, args := configLine(line) + switch key { + case "host": + for _, a := range args { + if a != "" && !strings.ContainsAny(a, "*?!") { + names = append(names, a) + } + } + case "include": + for _, a := range args { + if strings.HasPrefix(a, "~/") { + a = filepath.Join(sshHome(), a[2:]) + } else if !filepath.IsAbs(a) { + a = filepath.Join(base, a) + } + matches, _ := filepath.Glob(a) + for _, m := range matches { + names = append(names, hostNames(m, base, seen, depth+1)...) + } + } + } + } + return names +} + +// configLine splits one line of ssh config into its keyword, lowercased, +// and arguments: "Host a b", "Host=a b" and quoted arguments alike. +func configLine(line string) (string, []string) { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + return "", nil + } + i := strings.IndexAny(line, " \t=") + if i < 0 { + return strings.ToLower(line), nil + } + key := strings.ToLower(line[:i]) + rest := strings.TrimLeft(line[i:], " \t") + rest = strings.TrimPrefix(rest, "=") + var args []string + for rest = strings.TrimSpace(rest); rest != ""; rest = strings.TrimSpace(rest) { + if rest[0] == '"' { + end := strings.IndexByte(rest[1:], '"') + if end < 0 { + args = append(args, rest[1:]) + break + } + args = append(args, rest[1:1+end]) + rest = rest[2+end:] + continue + } + j := strings.IndexAny(rest, " \t") + if j < 0 { + args = append(args, rest) + break + } + args = append(args, rest[:j]) + rest = rest[j:] + } + return key, args +} diff --git a/internal/onboard/sshconfig_test.go b/internal/onboard/sshconfig_test.go new file mode 100644 index 0000000..b518e7e --- /dev/null +++ b/internal/onboard/sshconfig_test.go @@ -0,0 +1,87 @@ +package onboard + +import ( + "context" + "os" + "os/exec" + "os/user" + "path/filepath" + "testing" +) + +// withSSHConfig points configAlias and ssh -G at one file, and nothing else. +func withSSHConfig(t *testing.T, config string, extra map[string]string) { + t.Helper() + if _, err := exec.LookPath("ssh"); err != nil { + t.Skip("no ssh") + } + dir := t.TempDir() + path := filepath.Join(dir, "config") + if err := os.WriteFile(path, []byte(config), 0o600); err != nil { + t.Fatal(err) + } + for name, body := range extra { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + oldUser, oldSys, oldArgs := userSSHConfig, systemSSHConfig, sshExtraArgs + userSSHConfig = func() string { return path } + systemSSHConfig = "" + sshExtraArgs = []string{"-F", path} + t.Cleanup(func() { userSSHConfig, systemSSHConfig, sshExtraArgs = oldUser, oldSys, oldArgs }) +} + +// A machine given by address is reached through the Host block whose +// HostName it is, with that block's user — not the sudo caller's. +func TestSSHTargetUsesConfigAlias(t *testing.T) { + withSSHConfig(t, ` +Host *.example + User nobody-wild +Host gpu01 gpu01-alt + HostName 10.9.8.7 + User ops +Host gpu02 + HostName=10.9.8.8 +`, nil) + ctx := context.Background() + cases := []struct { + s *SSH + want string + }{ + // The config's User applies by ssh itself: no fallback is added. + {&SSH{Addr: "10.9.8.7", FallbackUser: "ilker"}, "gpu01"}, + {&SSH{Addr: "10.9.8.7", User: "root", FallbackUser: "ilker"}, "root@gpu01"}, + {&SSH{Addr: "10.9.8.8", FallbackUser: "ilker"}, "ilker@gpu02"}, + {&SSH{Addr: "gpu01", FallbackUser: "ilker"}, "gpu01"}, + {&SSH{Addr: "10.9.8.9", FallbackUser: "ilker"}, "ilker@10.9.8.9"}, + {&SSH{Addr: "10.9.8.9"}, "10.9.8.9"}, + } + for _, c := range cases { + if got := c.s.Target(ctx); got != c.want { + t.Errorf("%+v: target %q, want %q", c.s.Addr, got, c.want) + } + } +} + +// A block in an included file counts, as it does for ssh. +func TestSSHTargetFollowsInclude(t *testing.T) { + withSSHConfig(t, "Include nodes.conf\n", map[string]string{ + "nodes.conf": "Host \"gpu03\"\n HostName 10.9.8.10\n", + }) + // ssh resolves a relative Include against ~/.ssh, not -F's directory; + // the scan does the same, so point base at the test's directory. + names := hostNames(userSSHConfig(), filepath.Dir(userSSHConfig()), map[string]bool{}, 0) + if len(names) != 1 || names[0] != "gpu03" { + t.Fatalf("names %q", names) + } +} + +func TestConfigUserIgnoresLocalDefault(t *testing.T) { + withSSHConfig(t, "Host x\n HostName 10.9.8.11\n", nil) + if cur, err := user.Current(); err == nil { + if u := configUser(context.Background(), "x"); u != "" { + t.Errorf("config user %q for a block naming none (local %q)", u, cur.Username) + } + } +}