From f594799a7f04bff0c267b27cc04c496f941ad369 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 8 Jun 2025 03:07:59 -0400 Subject: [PATCH] etcd: ssh: Improve the authentication for ssh etcd world This was rather tricky, but I think I've learned a lot more about how SSH actually works. We now only offer up to the server what we can actually support, which lets us actually get back a host key we have a chance of actually authenticating against. Needed a new version of the ssh code and had to mess with go mod garbage. --- etcd/ssh/ssh.go | 245 +++++++++++++++++++++++++++++++++++++++--------- go.mod | 14 +-- go.sum | 17 ++++ 3 files changed, 226 insertions(+), 50 deletions(-) diff --git a/etcd/ssh/ssh.go b/etcd/ssh/ssh.go index 60e3a094..1b6df098 100644 --- a/etcd/ssh/ssh.go +++ b/etcd/ssh/ssh.go @@ -38,6 +38,8 @@ import ( "net" "net/url" "os" + "path/filepath" + "sort" "strconv" "strings" @@ -58,9 +60,9 @@ const ( defaultSSHPort uint16 = 22 defaultSSHHostKeyFieldName = "hostkey" // querystring field name defaultEtcdPort uint16 = 2379 // TODO: get this from etcd pkg - defaultIDRsaPath = "~/.ssh/id_rsa" - defaultIDEd25519Path = "~/.ssh/id_ed25519" + defaultSSHDir = "~/.ssh/" defaultKnownHostsPath = "~/.ssh/known_hosts" + allowRSA = true // are big keys okay? ) // World is an implementation of the world API for etcd over SSH. @@ -82,11 +84,10 @@ type World struct { // is specified, then it overrides looking for it in the URL. HostKey string - // SSHID is the path to the ~/.ssh/id_rsa or ~/.ssh/id_ed25519 to use - // for auth. If you omit this then this will look for your private key - // in both of those default paths. If you specific a specific path, then - // that will only be used. This will expand the ~/ and ~user/ style path - // expansions. + // SSHID is the path to the ~/.ssh/id_??? key to use for auth. If you + // omit this then this will look for your private key in all possible + // paths. If you specific a specific path, then only that will be used. + // This will expand the ~/ and ~user/ style path expansions. SSHID string // Seeds are the list of etcd endpoints to connect to. @@ -109,35 +110,125 @@ type World struct { cleanups []func() error } -// sshKeyAuth is a helper function to get the ssh key auth struct needed. -func (obj *World) sshKeyAuth(sshID string) (ssh.AuthMethod, error) { - if sshID == "" { - return nil, fmt.Errorf("empty path specified") - } - - // expand strings of the form: ~james/.ssh/id_rsa - p, err := util.ExpandHome(sshID) +// keySigners gets a list of possible key signers. These are used to get the +// available types of the keys, and the auth methods. +func (obj *World) keySigners() ([]ssh.Signer, error) { + sshDir, err := util.ExpandHome(defaultSSHDir) if err != nil { return nil, errwrap.Wrapf(err, "can't find home directory") } - if p == "" { - return nil, fmt.Errorf("empty path specified") + if sshDir == "" { + return nil, fmt.Errorf("empty path found") } + + files, err := os.ReadDir(sshDir) + if err != nil { + return nil, err + } + + signers := []ssh.Signer{} + // XXX: Should we aim to pull the keys out by order of preference? + for _, file := range files { + p := filepath.Join(sshDir, file.Name()) + + if file.IsDir() || obj.isPossiblePrivateKeyFile(p) != nil { + continue + } + + signer, err := obj.keySigner(p) + if err != nil { + obj.init.Logf("%s", err) + continue + } + + signers = append(signers, signer) + } + + return signers, nil +} + +// keySigner returns a single signer from an absolute path. +func (obj *World) keySigner(p string) (ssh.Signer, error) { + data, err := os.ReadFile(p) + if err != nil { + return nil, fmt.Errorf("key file error: %s", err) + } + if len(data) == 0 { + return nil, fmt.Errorf("empty key file at: %s", p) + } + // A public key may be used to authenticate against the server by using // an unencrypted PEM-encoded private key file. If you have an encrypted // private key, the crypto/x509 package can be used to decrypt it. - key, err := os.ReadFile(p) + signer, err := ssh.ParsePrivateKey(data) if err != nil { - return nil, err + if _, ok := err.(*ssh.PassphraseMissingError); ok { + return nil, fmt.Errorf("password required for key file:: %s", p) + } + + return nil, fmt.Errorf("key file parsing error: %s", err) } - // create the Signer for this private key - signer, err := ssh.ParsePrivateKey(key) - if err != nil { - return nil, err + obj.init.Logf("found auth option in: %s", p) + + // return the Signer for this private key + return signer, nil +} + +// isPossiblePrivateKeyFile determines if we've found a private key file. +func (obj *World) isPossiblePrivateKeyFile(p string) error { + + b := filepath.Base(p) + //d := filepath.Dir(p) // no trailing slash :( + + if !strings.HasPrefix(b, "id_") { + return fmt.Errorf("keys start with id_???") } - return ssh.PublicKeys(signer), nil + if strings.HasSuffix(b, ".pub") { + return fmt.Errorf("this is a public key") + } + + if _, err := os.Stat(p + ".pub"); err != nil { + return fmt.Errorf("matching public key is inaccessible") + } + + // TODO: should we rule out anything else? + + return nil +} + +// prioritizeHostKeyAlgorithms returns the host key algorithms that we tell the +// server that we support. The order matters, because this ordering will let the +// server know which we can authenticate against. Once we send a list, the +// server then only returns a single one, so it's important that we sort this +// list properly with what we have available at the very top. +func (obj *World) prioritizeHostKeyAlgorithms(allHostKeyAlgos, keyTypes []string) []string { + rank := make(map[string]int, len(keyTypes)) + for i, t := range keyTypes { + rank[t] = i + } + + sorted := make([]string, len(allHostKeyAlgos)) + copy(sorted, allHostKeyAlgos) + + sort.SliceStable(sorted, func(i, j int) bool { + rankI, okI := rank[sorted[i]] + rankJ, okJ := rank[sorted[j]] + + switch { + case okI && okJ: + return rankI < rankJ + case okI: + return true + case okJ: + return false + default: + return false + } + }) + + return sorted } // knownHostsKey takes a known_hosts key entry (just the base64 key part) and @@ -167,13 +258,17 @@ func (obj *World) knownHostsKey(hostkey string) (ssh.PublicKey, error) { // func (obj *World) hostKeyCallback() (ssh.HostKeyCallback, error) { func (obj *World) hostKeyCallback(hostkey ssh.PublicKey) ssh.HostKeyCallback { return func(hostname string, remote net.Addr, key ssh.PublicKey) error { + obj.init.Logf("server host key type: %s", key.Type()) + obj.init.Logf("host key fingerprint: %s", ssh.FingerprintSHA256(key)) // First try our one known key if it exists. if hostkey != nil { fn := ssh.FixedHostKey(hostkey) if fn(hostname, remote, key) == nil { + obj.init.Logf("matched key") return nil // found it! } + obj.init.Logf("did not match known key: %s", ssh.FingerprintSHA256(hostkey)) } // TODO: consider allowing a user-specified path in the future @@ -192,7 +287,35 @@ func (obj *World) hostKeyCallback(hostkey ssh.PublicKey) ssh.HostKeyCallback { if err != nil { return err } - return fn(hostname, remote, key) + obj.init.Logf("trying known_hosts file at: %s", p) + err = fn(hostname, remote, key) + if err == nil { + obj.init.Logf("host key matched") + return nil + } + + ke, ok := err.(*knownhosts.KeyError) // give a better error? + if !ok || len(ke.Want) == 0 { + return err + } + + // Based on what we initially have in our ~/.ssh/ dir, our ssh + // client offers keys to the server differently, and the server + // replies with up to one of our acceptable choices. If none are + // available, then this error message is weird, so we do all + // this to make it clearer. + types := []string{} + for _, kk := range ke.Want { // known keys + typ := kk.Key.Type() + types = append(types, typ) + + // We found what the server offered, error normally... + if key.Type() == typ { + return err + } + } + + return fmt.Errorf("no known_hosts entry matching type, have: %s", strings.Join(types, ", ")) } } @@ -267,40 +390,76 @@ func (obj *World) Connect(ctx context.Context, init *engine.WorldInit) error { addr := fmt.Sprintf("%s:%s", hostname, port) + // Preference order of keys I have available... + keyTypes := []string{ + //ssh.KeyAlgoED25519, // "ssh-ed25519" + //ssh.KeyAlgoRSA, // "ssh-rsa" + } auths := []ssh.AuthMethod{} //auths = append(auths, ssh.Password("password")) // testing - choices := []string{ - //obj.SSHID, - defaultIDEd25519Path, - defaultIDRsaPath, // "~/.ssh/id_rsa" - } + if obj.SSHID != "" { - choices = []string{ - obj.SSHID, - } - } - for _, p := range choices { - if p == "" { - continue - } - auth, err := obj.sshKeyAuth(p) + p, err := util.ExpandHome(obj.SSHID) if err != nil { - //obj.init.Logf("can't get auth from: %s", p) // misleading - continue + return errwrap.Wrapf(err, "can't find home directory") } - obj.init.Logf("found auth option in: %s", p) - auths = append(auths, auth) + if p == "" { + return fmt.Errorf("empty path specified") + } + + signer, err := obj.keySigner(p) + if err != nil { + return err + } + typ := signer.PublicKey().Type() + keyTypes = append(keyTypes, typ) + auths = append(auths, ssh.PublicKeys(signer)) // add one } + + if len(auths) == 0 { + signers, err := obj.keySigners() + if err != nil { + return err + } + for _, signer := range signers { + typ := signer.PublicKey().Type() + keyTypes = append(keyTypes, typ) + } + // TODO: should the order of the signers matter? + if len(signers) > 0 { + auths = append(auths, ssh.PublicKeys(signers...)) // add all + } + } + if len(auths) == 0 { return fmt.Errorf("no auth options available") } + obj.init.Logf("found %d available key types: %s", len(keyTypes), strings.Join(keyTypes, ", ")) + + algorithms := ssh.SupportedAlgorithms() + preferredAlgoOrder := algorithms.HostKeys // the defaults + if allowRSA { + preferredAlgoOrder = append(preferredAlgoOrder, ssh.KeyAlgoRSA) + } + obj.init.Logf("supported algos: %s", strings.Join(preferredAlgoOrder, ", ")) + // SSH connection configuration sshConfig := &ssh.ClientConfig{ User: user, Auth: auths, //HostKeyCallback: ssh.InsecureIgnoreHostKey(), // testing HostKeyCallback: obj.hostKeyCallback(pubKey), + + // This is the list of host key algorithms that this SSH client + // will offer to the SSH server when it says hello. This can be + // different from what a normal terminal SSH client might do, + // which means you might not get the right SSH host key algo + // offered back to you, so make sure you provide what it's + // asking for. Maybe we need to make this configurable by the + // user. + //HostKeyAlgorithms: algorithms.HostKeys, + HostKeyAlgorithms: obj.prioritizeHostKeyAlgorithms(preferredAlgoOrder, keyTypes), } obj.init.Logf("ssh: %s@%s", user, addr) diff --git a/go.mod b/go.mod index 8c0294dd..e521ad46 100644 --- a/go.mod +++ b/go.mod @@ -43,10 +43,10 @@ require ( go.etcd.io/etcd/client/v3 v3.5.18 go.etcd.io/etcd/server/v3 v3.5.18 go4.org/netipx v0.0.0-20231129151722-fdeea329fbba - golang.org/x/crypto v0.37.0 - golang.org/x/sys v0.32.0 + golang.org/x/crypto v0.39.0 + golang.org/x/sys v0.33.0 golang.org/x/time v0.9.0 - golang.org/x/tools v0.29.0 + golang.org/x/tools v0.34.0 google.golang.org/grpc v1.70.0 gopkg.in/yaml.v2 v2.4.0 honnef.co/go/augeas v0.0.0-20161110001225-ca62e35ed6b8 @@ -182,10 +182,10 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/arch v0.16.0 // indirect golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8 // indirect - golang.org/x/net v0.39.0 // indirect - golang.org/x/sync v0.13.0 // indirect - golang.org/x/term v0.31.0 // indirect - golang.org/x/text v0.24.0 // indirect + golang.org/x/net v0.41.0 // indirect + golang.org/x/sync v0.15.0 // indirect + golang.org/x/term v0.32.0 // indirect + golang.org/x/text v0.26.0 // indirect google.golang.org/genproto v0.0.0-20250124145028-65684f501c47 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250124145028-65684f501c47 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250124145028-65684f501c47 // indirect diff --git a/go.sum b/go.sum index 37373acc..ec97614d 100644 --- a/go.sum +++ b/go.sum @@ -582,6 +582,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= +golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8 h1:yqrTHse8TCMW1M1ZCP+VAR/l0kKxwaAIqN/il7x4voA= golang.org/x/exp v0.0.0-20250106191152-7588d65b2ba8/go.mod h1:tujkw807nyEEAamNbDrEGzRav+ilXA7PCRAd6xsmwiU= @@ -619,6 +621,10 @@ golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20211123203042-d83791d6bcd9/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY= +golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds= +golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw= +golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -632,6 +638,8 @@ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= +golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -676,15 +684,21 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= +golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= +golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= +golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= +golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= +golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY= golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -700,6 +714,9 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= +golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI= +golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= +golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=