At the protocol level, SSH lets client and server specify different
algorithms for the read and write half of the connection. This has
never worked correctly, as Client-to-Server was always interpreted as
the "write" side, even if we were the server.
This has never been a problem because, apparently, there are no
clients that insist on different algorithm choices running against Go
SSH servers.
Since the SSH package does not expose a mechanism to specify
algorithms for read/write separately, there is end-to-end for this
change, so add a unittest instead.
Change-Id: Ie3aa781630a3bb7a3b0e3754cb67b3ce12581544
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/172538
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The new name readCipherPacket/writeCipherPacket disambiguates method
calls from packetConn.{read,write}Packet, which has a different
signature, and is called in different layers of the stack.
Change-Id: I02ca83e28f74f0a347a1cde665efdfb78f3f51bb
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/171657
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".
Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.
Includes a test with a couple of cases.
Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.
Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef2538d33a9493f3df09984c277dfd8bf0abf4
GitHub-Pull-Request: golang/crypto#82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
audited using ineffassign tool from
github.com/gordonklaus/ineffassign
go generate does not generate any changes
Change-Id: Iabbec9ec1aae39081289d503d79fd7b4caadf17b
GitHub-Last-Rev: acd17cce410e9c68ce3c87b5546261be9153e3ea
GitHub-Pull-Request: golang/crypto#70
Reviewed-on: https://go-review.googlesource.com/c/155942
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Previously, this would return the default error "no auth passed yet".
Not only is the new error more specific, it makes it easier to verify
the control flow of server authentication code.
Change-Id: I6c8de4e3f91da74274acbe9d87ec4f6158b4a94f
Reviewed-on: https://go-review.googlesource.com/c/142897
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Make sure a meaningful error is returned when the SSH agent client receives
a response that is too big.
Change-Id: Ied68b2c559a61dede2f3112c4c8143babd385f48
GitHub-Last-Rev: ad65e1b423bb00f2501dfde84040469aef4ee2df
GitHub-Pull-Request: golang/crypto#61
Reviewed-on: https://go-review.googlesource.com/c/143058
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit implements two new features. To preserve backwards
compatibility the new methods are added to an `ExtendedAgent` interface
which extends `Agent`. The client code implements `ExtendedAgent`
(which extends Agent) so you can call these additional methods against
SSH agents such as the OpenSSH agent. The ServeAgent method still
accepts Agent but will attempt to upcast the agent to `ExtendedAgent`
as needed, so if you supply an ExtendedAgent implementation you can
implement these additional methods (which keyring does).
The first feature is supporting the standard flags that can be passed to
SSH Sign requests, requesting that RSA signatures use SHA-256 or
SHA-512. See section 4.5.1 of the SSH agent protocol draft:
https://tools.ietf.org/html/draft-miller-ssh-agent-02
The second feature is supporting calling custom extensions from clients
and implementing custom extensions from servers. See section 4.7 of the
SSH agent protocol draft:
https://tools.ietf.org/html/draft-miller-ssh-agent-02
Change-Id: I0f74feb893762c27e921ec37604d3a46434ee6ef
GitHub-Last-Rev: 2e23fd01c0e95b664e8507682f0bd5bd61d4c146
GitHub-Pull-Request: golang/crypto#53
Reviewed-on: https://go-review.googlesource.com/c/123955
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix typo in error message when keyboard-interactive auth not supported by server and client requests it
Change-Id: Iedb72625852f03552481d85cce2119765cfba320
GitHub-Last-Rev: ee47092daf8c40fe3e0e166e7370556819457af6
GitHub-Pull-Request: golang/crypto#59
Reviewed-on: https://go-review.googlesource.com/c/141658
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Forgotten file from CL 112635.
The integration tests didn't catch this because the integration tests
never run (sshd isn't in $PATH usually), but even when $PATH is
modified, it seems to have been failing for ages on both Debian jessie
and stretch. I'll fix the tests and make them run in TryBots in later
changes.
Change-Id: I85fd3b6109a73990bc353a61da5f527b9a698501
Reviewed-on: https://go-review.googlesource.com/113056
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The extra goroutines were distracting while debugging something else,
especially as I wasn't using that feature. This also saves a bit of
memory.
Change-Id: Ia6489e64bbd3d5a6ff699a25018676d8ff8bd2e4
Reviewed-on: https://go-review.googlesource.com/112635
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
The tag matches the platforms defined in util*.go
where the most tested logic is defined.
Change-Id: I90f67d988c795738c3effbc8554a933a7cb355d2
Reviewed-on: https://go-review.googlesource.com/112555
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Doesn't matter because contentKey and lengthKey are the same length,
but we should really be looping over lengthKey here.
Change-Id: Ic7af3c553eb4382239bec1c030e77c7f5baa0662
Reviewed-on: https://go-review.googlesource.com/109735
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Unlock compares the length of the passphrase with the given one before
calling subtle.ConstantTimeCompare. This is redundant, since
ConstantTimeCompare already perform a lengths check before doing
anything. Remove the check from Unlock.
Updates golang/go#25173
Change-Id: Ib5fec3a94392bddf2996f5c6bf5a414529e86f2f
Reviewed-on: https://go-review.googlesource.com/110068
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
This fixes the following vulnerability scenario:
* Victim logs into SAFE-HOST on SAFE-IP-ADDRESS regularly.
* Victim is cajoled into connecting to attacker controlled
ATTACK-HOST, on ATTACK-IP-ADDRESS. ATTACK-HOST uses a different host
key type (e.g. Ed25519 vs RSA). The new key is added at the end of
known_hosts.
* Attacker makes DNS system return ATTACK-IP-ADDRESS for SAFE-HOST.
* Victim logs into SAFE-HOST, but is not warned because the host key
matches ATTACK-IP-ADDRESS.
For this attack to work, the key type has to be different, because
knownhosts gives precedence to the first key found for each type. Add
a test that asserts this behavior.
The new semantics simplify the code, but callers that modify
.ssh/known_host interactviely must now take an extra step to remain
OpenSSH compatible: on successful login, the IP address must be
checked without hostname, and if it is not known, added separately to
the known_hosts file, so future logins that use an IP address only
will be protected too.
Thanks to Daniel Parks <security@demonhorse.net> for finding this
vulnerability.
Change-Id: I62b1b60ceb02e2f583a4657213feac1a8885dd42
Reviewed-on: https://go-review.googlesource.com/104939
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The extra Close call runs in a separate goroutine, and races with the
deferred Close call, leading to test flakes if the extra call fails
Change-Id: Ia1a2fb3932906da7cd0a8854b55f82785bb33716
Reviewed-on: https://go-review.googlesource.com/104335
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestMakeRawState currently fails on Solaris:
--- FAIL: TestMakeRawState (0.00s)
terminal_test.go:334: states do not match; was &{0xc420015dd0}, expected &{0xc420015da0}
Change terminal.State to include a copy to the unix.Termios (like the
implementation for Linux and the BSDs) which also makes terminal.MakeRaw behave
as expected and lets TestMakeRawState pass.
Change-Id: I29382f83b84ff301991e1db170f32f41e144aec8
Reviewed-on: https://go-review.googlesource.com/99456
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
yet.
Since ServerAuthError returns all errors collected during the
userAuthLoop, it will always include the generic error "no auth passed
yet" (assuming the connection fails).
Change-Id: I5f6c67f3f0762b023618178d4028600d2b6c9253
Reviewed-on: https://go-review.googlesource.com/92737
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixesgolang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
Reviewed-on: https://go-review.googlesource.com/88035
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
os.NewFile assigns finalizer to close file handle
passed into ReadPassword. But that is not expected.
Make a duplicate of original file handle, and pass
copy handle into ReadPassword instead.
Fixesgolang/go#23525
Change-Id: I4d6725e9a1cc20defd1b58afc383e35a7f9ee4e9
Reviewed-on: https://go-review.googlesource.com/89395
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
yanweizhi
chigotc
Han-Wen Nienhuys
Tobias Klauser
Brad Fitzpatrick
Andrey Petrov
Max Semenik
Ian Haken
Michael Cook
Michal Bohuslávek
Jay Taylor
Igor Zhilianin
Andres Lowrie
mkishere
Adam Langley
Noel Georgi
Hana (Hyang-Ah) Kim
Michael Munday
Alberto Donizetti
Agniva De Sarker
Spencer Tung
Sami Pönkänen
Sergey Lukjanov
Yasuhiro Matsumoto