Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/crypto/ssh/knownhosts: cannot have multiple keys for same host #36126

Open
pjtatlow opened this issue Dec 13, 2019 · 4 comments · May be fixed by golang/crypto#254
Open

x/crypto/ssh/knownhosts: cannot have multiple keys for same host #36126

pjtatlow opened this issue Dec 13, 2019 · 4 comments · May be fixed by golang/crypto#254
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@pjtatlow
Copy link

pjtatlow commented Dec 13, 2019 •
edited
Loading

What version of Go are you using (go version)?

$ go version
go version go1.13.3 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/pjt/projects/go/bin"
GOCACHE="/Users/pjt/Library/Caches/go-build"
GOENV="/Users/pjt/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/pjt/projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.3/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/p9/y23xtnms6r90wsl5lsz2tkfh0000gq/T/go-build606914329=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.13.3 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.13.3
uname -v: Darwin Kernel Version 19.0.0: Thu Oct 17 16:17:15 PDT 2019; root:xnu-6153.41.3~29/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.15.1
BuildVersion:	19B88
lldb --version: lldb-1100.0.30.11
Apple Swift version 5.1.3 (swiftlang-1100.0.282.1 clang-1100.0.33.15)

What did you do?

I'm trying to open an ssh connection to my-site.com:22 using ssh.Dial where the config uses the callback provided by knownhosts.New("~/.ssh/known_hosts"). I have a known_hosts file that looks like this:

my-site.com,host1.my-site.com,1.1.1.1 ecdsa-sha2-nistp256 <public-key-1>
my-site.com,host2.my-site.com,2.2.2.2 ecdsa-sha2-nistp256 <public-key-2>

What did you expect to see?

Connection succeeds when either public key is provided.

What did you see instead?

Connection only succeeds when I happen to connect to host1.my-site.com. If it tries to connect to host2.my-site.com I get a KeyError. I can connect to either host using the ssh program.

Why did this happen?

Using knownhosts.New to build a host key callback rejects some hosts from the known_hosts file when there are multiple Public Keys of the same type. There is the assertion in the knownhosts code which says "For each key algorithm, there can be one hostkey", which I don't believe is correct. I think we need to check keys from any line that matches the current host, rather than only ones that have key types we haven't seen yet.
@gopherbot gopherbot added this to the Unreleased milestone Dec 13, 2019
@bradfitz bradfitz changed the title x/crypto/ssh/knownhosts Cannot have multiple keys for same host x/crypto/ssh/knownhosts: cannot have multiple keys for same host Dec 13, 2019
@bradfitz
Copy link
Contributor

knownhosts.New("~/.ssh/known_hosts")

Off topic, but I assume that's just pseudo code, as os.Open doesn't expand the tilde.

I can connect to either host using the ssh program.

Thanks for that info. I didn't know such a thing was supported.

/cc @hanwen

@pjtatlow
Copy link
Author

Off topic, but I assume that's just pseudo code, as os.Open doesn't expand the tilde.

@bradfitz Yup, just to give you an idea of what I'm doing. I actually do have a fix, but I was unsure what should be in KeyError.Want since that was originally assuming one key of each type. Do you have any ideas how people use that?

@bradfitz
Copy link
Contributor

Sorry, I don't know much about this package. I'll let @hanwen handle this.

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 13, 2019
@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 15, 2020
@sweharris sweharris mentioned this issue Oct 3, 2020
Closed
@joegrasse
Copy link

I believe I ran into this problem. I had the following format in my known_hosts file.

my-site.com ecdsa-sha2-nistp256 <public-key-1>
my-site.com ssh-rsa <public-key-2>

@caarlos0 caarlos0 mentioned this issue Feb 10, 2023
Open
caarlos0 added a commit to caarlos0/crypto that referenced this issue Mar 22, 2023
@caarlos0
crypto/ssh/knownhosts: check more than one key
adcf924 
closes golang/go#36126

Signed-off-by: Carlos A Becker <[email protected]>
@caarlos0 caarlos0 linked a pull request Mar 22, 2023 that will close this issue
Open
@Kemystra Kemystra mentioned this issue Dec 10, 2023
Open
@oneirocosm oneirocosm mentioned this issue Mar 21, 2024
Closed
@oneirocosm oneirocosm mentioned this issue Jul 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

crypto/ssh/knownhosts: check more than one key caarlos0/crypto
6 participants
@bradfitz @toothrot @FiloSottile @joegrasse @pjtatlow @gopherbot