SymmetricKeyEncrypted cached the results of decryption so, if a bad
password was given, ReadMessage would prompt forever because a later,
correct password wouldn't override the cached decryption.
The SymmetricKeyEncrypted object can't know whether a given passphrase
is correct so it should never have been a mutable object in the first
place. This change makes it so that it doesn't cache anything.
Fixes#9315
Change-Id: Ic2b75f7f60a575e2182ac7e5c5d4198597c5d0a2
Reviewed-on: https://go-review.googlesource.com/14038
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
280be00 introduced custom server versions; however, at least OpenSSH
clients do not accept a server version that doesn't start with a
protocol version like "SSH-2.0-"; this is not documented in this
library, and automatically adding it in case that the user did not does
no harm.
Change-Id: Iafc23b23160a8277b10cbf29778dc64a96351bd7
Reviewed-on: https://go-review.googlesource.com/14018
Reviewed-by: Adam Langley <agl@golang.org>
This adds a NewSignerFromSigner to crypto/ssh which takes a
crypto.Signer and turns it into an ssh.Signer, helpful if, e.g., your
crypto.Signer is backed by some sort of hardware device.
The interfaces are very similar - the biggest differences are that a
crypto.Signer accepts hashed data, while an ssh.Signer does not, and
some differences in encoding for DSA and ECDSA signatures.
This also adjusts NewSignerFromKey to use NewSignerFromSigner where
possible, dropping the rsaPrivateKey and ecdsaPrivateKey types in
favor of wrappedSigner. (However, because *dsa.PrivateKey is not a
crypto.Signer, we still have to keep dsaPrivateKey)
Change-Id: Ia2e20ece9c9d3844b4e5a64c1a7d997178ec8781
Reviewed-on: https://go-review.googlesource.com/10953
Reviewed-by: Adam Langley <agl@golang.org>
Change parseECDSA() to unmarshal the key's contents into a struct
representing the wire format, consistent with the parseRSA() and
parseDSA(), to make the code more readable and its intent clearer.
Change-Id: Iea85630107ac0b3e681807d2278390c8c50ce141
Reviewed-on: https://go-review.googlesource.com/13663
Reviewed-by: Han-Wen Nienhuys <hanwenn@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>
clearsign.Encode currently creates bad signatures for inputs containing
lines that that consist of only whitespace (gpg --verify and
clearsign.Decode both agree the signature is bad).
RFC 4880 section 7.1 says trailing whitespace is removed when generating
the signature. The previous code correctly handled this for the case of
not being at the beginning of the line by buffering the whitespace.
The previous code had incorrect behavior for the case of being at the
beginning of a line. It was only special-casing dashes and newlines,
directly hashing all other characters.
This happened to work for lines that had leading whitespace followed by
non-whitespace characters, since in that case the leading whitespace is
not trailing.
However, this behavior is incorrect for whitespace-only lines: The
previous code would incorrectly add the first whitespace character to
the hash, when instead it should have been dropped.
This commit moves the whitespace check so that it always happens,
regardless of whether we are at the beginning of a line.
This adds a few tests to capture the expected behavior. The last three
tests fail without the included code change.
Change-Id: I17848b8aaad6f7a4cee414d486be236f7edddd0b
Reviewed-on: https://go-review.googlesource.com/13681
Reviewed-by: Adam Langley <agl@golang.org>
The error message reported by the ssh client when it can't find a
"cipher" in common between the client and server was overly vague. This
adds more detailed error messages to findAgreedAlgorithms so that the
user can more easily identify which of the components can't reach
agreement.
Change-Id: I4d985e92fea964793213e5600b52b3141e712000
Reviewed-on: https://go-review.googlesource.com/13817
Reviewed-by: Adam Langley <agl@golang.org>
In the initial patch enabling generation of OCSP responses, the Reason
field in the revokedInfo struct used for serializing responses was set
to type int. That type maps to the ASN.1 type INTEGER, not ENUMERATED,
as required by RFC 6960. As a result, if you serialize an OCSP
resonse with the Reason field populated, then it will be rejected as
malformed by compliant OCSP parsers.
This patch changes the type of the Reason field in revokedInfo to
asn1.Enumerated. It leaves the RevocationReason field in the public
Response struct as int, and converts between the two. The patch
also adds constant for the defined revocation reasons.
Change-Id: I97205319503f447cde12d9a0bb0bd1a8db7a66ee
Reviewed-on: https://go-review.googlesource.com/13964
Reviewed-by: Adam Langley <agl@golang.org>
The current ARM implementation assumes that the input message
is memory aligned and so it can cause alignment fault when it
is not enabled. Also it may generate incorrect outputs in ARMv5.
This change fixes this issue by temporarily copying the input
to a local aligned space. Although there may be a better way
to handle unaligned access, this would be a safe way in all
ARM versions.
This change also added a test and benchmarks with unaligned
data. The benchmark result on RasberryPI 2 is
Benchmark64 2000000 812 ns/op 78.81 MB/s
Benchmark1K 200000 7809 ns/op 131.12 MB/s
Benchmark64Unaligned 2000000 967 ns/op 66.13 MB/s
Benchmark1KUnaligned 200000 10316 ns/op 99.26 MB/s
Change-Id: I189cc1b7bb6c67a04c9877271fb27326f2896e82
Reviewed-on: https://go-review.googlesource.com/12797
Reviewed-by: Adam Langley <agl@golang.org>
Unblock writers if a read error occurs while writers are blocked on a
pending key change.
Add test to check for deadlocks in error paths in handshake.go
Fixesgolang/go#11992.
Change-Id: Id098bd9fec3d4fe83daeb2b7f935e5647c19afd3
Reviewed-on: https://go-review.googlesource.com/13594
Reviewed-by: Adam Langley <agl@golang.org>
The current implementation is not compliant with the ASN.1 structure
for an OCSP response in RFC 6960. In the RFC, the "revoked" field is
marked "implicit". The "explicit" tag in the current struct causes
the encoder to emit an additional SEQUENCE, which cases some parsers
(notably OpenSSL) to reject OCSP responses as malformed. This patch
simply removes the "explicit" tag, so that the emitted DER is
compliant with the RFC.
Change-Id: Ifa65a73a8d24f08fe3c2794309df772edc8bb114
Reviewed-on: https://go-review.googlesource.com/13572
Reviewed-by: Adam Langley <agl@golang.org>
If the channel open request failed, a nil channel would be provided to
DiscardRequests, which would never return.
We return the error early to avoid this goroutine leak.
Change-Id: I4c0e0a7698f7623c042f2a04941b8c50e8031d33
Reviewed-on: https://go-review.googlesource.com/13390
Reviewed-by: Dave Cheney <dave@cheney.net>
Update golang/go#11811
The increased default concurrency in Go 1.5 showed up a test flake in
the TestHostKeyCert test. Under load, when the client provided incorrect
data, both sides would race to tear down the connection, which would often
lead to the server side, running in its own goroutine to see an unexpected
EOF or connection reset.
Fix this flake (and the incorrect use of t.Fatalf) by passing the error back
to the main goroutine for inspection. This also lets us ignore the expected
error in the unsuccessful path
Change-Id: I5a95c6d240479e9d537f34177e5ca8023b1b08e9
Reviewed-on: https://go-review.googlesource.com/12916
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixesgolang/go#11882
If an error occurs during handshakeTransport.writePacket the lock may not be
released. Fix this by using defer rather than manually unlocking in all paths.
Change-Id: I0010284b4f7d99907c86b4c0e140ab6cf37b0441
Reviewed-on: https://go-review.googlesource.com/12888
Reviewed-by: Adam Langley <agl@golang.org>
parseRSAPrivateKey calls rsa.PrivateKey.Precompute which triggers
divide-by-zero panic if either p or q is 1. Sanity check the parsed
values by calling rsa.PrivateKey.Validate.
Fixesgolang/go#11505
Change-Id: Ia6c9eccca0cfa49aaa58716e708c557a788bb204
Reviewed-on: https://go-review.googlesource.com/12356
Reviewed-by: Adam Langley <agl@golang.org>
Some invalid input may be parsed so that the length of an opaque
subpacket turns out to be 0. In such cases, arrange for a
StructuralError to be returned indicating truncation.
Found using gofuzz.
Fixesgolang/go#11503
Change-Id: Ib9ce8c604f35a31f852adfcd56a22dfd143a9443
Reviewed-on: https://go-review.googlesource.com/12634
Reviewed-by: Adam Langley <agl@golang.org>
The ssh-agent protocol allows the usage of keys and certs added to a
given agent to be constrained in certain ways. The only constraints
currently supported are lifetime (keys expire after some number of
seconds) and confirmation (the agent requires user confirmation before
performing any operations with the private key).
Change-Id: Idba5760db929805bf3da43fdcaca53ae6c479ca4
Reviewed-on: https://go-review.googlesource.com/12260
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Peter Moody <pmoody@uber.com>
Currently using go-fuzz with code using golang.org/x/crypto/ssh fails
because it passes CertTimeInfinity to an interface{} and automatically
tries to use an int, which overflows and results in a compile error.
This change adds a no-op type conversion inside the function which
makes things compile with go-fuzz.
Change-Id: Iade0c0df7e20cbac4928480fad3e44c831e1e00a
Reviewed-on: https://go-review.googlesource.com/11285
Reviewed-by: Adam Langley <agl@golang.org>
OCSP responders sometimes rely on pre-generated responses to increase
performance. In such cases, RFC 5019 allows responders to respond with
responseStatus unauthorized if they do not have a pre-generated response for
a certificate. This patch provides a pre-serialized unauthorized response.
This change also updates the serialization of OCSP responses so that the
resulting DER encoding is compatible with other parsers.
Note: This change depends on updates to encoding/asn1 to improve handling
of flags and time values.
https://go-review.googlesource.com/#/c/5970/
Change-Id: I77e042de6535a70b0996e058cb38a00076a16dd4
Reviewed-on: https://go-review.googlesource.com/4121
Reviewed-by: Adam Langley <agl@golang.org>
The new function allows an existing encrypted key packet
to be serialized to a Writer.
Change-Id: I20d82ca473d8ae738d239068626897c1ff868a15
Reviewed-on: https://go-review.googlesource.com/3167
Reviewed-by: Adam Langley <agl@golang.org>
This deprives an attacker of feedback for guesses against the packet
length given by the connection dropping.
Change-Id: I14939a82e5243a86d192bb18be93d45589227147
Reviewed-on: https://go-review.googlesource.com/9908
Reviewed-by: Adam Langley <agl@golang.org>
If the system is using UTC, then time.Now().loc != time.UTC().loc,
so it should not use reflect.DeepEqual to compare two times.
While we're here, also fix some copy-paste errors.
Change-Id: I1fef5f22f5b5eb978746d2695a1b43f153e4a408
Reviewed-on: https://go-review.googlesource.com/10335
Reviewed-by: Adam Langley <agl@golang.org>
Fix compilation of poly1305 using go tip - it currently fails with:
./poly1305_arm.s:124: cannot reference SP without a symbol
./poly1305_arm.s:161: cannot reference SP without a symbol
./poly1305_arm.s:162: cannot reference SP without a symbol
asm: asm: assembly of ./poly1305_arm.s failed
Change-Id: I797dcf3641cc881b6cc192034b693ccf58317987
Reviewed-on: https://go-review.googlesource.com/10307
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
This change adds ARMv6 assembly implementation. The referenced assembly code was
the public domain source by Andrew Moon in https://github.com/floodyberry/poly1305-opt/blob/master/app/extensions/poly1305/poly1305_armv6-32.inc.
The author has confirmed that it's ok to put it under the Go license.
Benchmark results on Raspberry Pi (ARMv6-compatible processor rev 7),
o Without ARMv6 assembly
Benchmark1K 5000 287177 ns/op 3.57 MB/s
Benchmark64 50000 38880 ns/op 1.65 MB/s
o With ARMv6 assembly
Benchmark1K 100000 15964 ns/op 64.14 MB/s
Benchmark64 1000000 1472 ns/op 43.46 MB/s
Change-Id: Iea5b0b831ac097cc6d477a8fccbf0ddb4819724c
Reviewed-on: https://go-review.googlesource.com/9765
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Attention - BREAKING change for the certificates generated with
the previous versions of crypto/ssh! Need to regenerate
certificates with a version of crypto/ssh library including
this fix.
[PROTOCOL.cerkeys] requires two length fields for non-empty
values of critical options (or extensions - but those are
currently always empty) - see
https://bugzilla.mindrot.org/show_bug.cgi?id=2389.
Add SSH-conform handling of such composite values in marshalTuples
and parseTuples and related test (TestParseCertWithOptions) parsing
a certificate created with ssh-keygen which includes critical options.
Fixes#10569
Change-Id: Iecbfca67a66668880635141c72bc5fc370a9c112
Reviewed-on: https://go-review.googlesource.com/9375
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Paul Meyer
Adam Langley
Lewin Bormann
Evan Broder
Reinaldo de Souza Jr
Matt Bostock
SeanBurford
Peter Tseng
Thomas Desrosiers
Richard Barnes
Jungho Ahn
Han-Wen Nienhuys
Eric Garrido
Dave Cheney
David du Colombier
Dhiru Kholia
Michael Gehring
Jeff R. Allen
Brian Gitonga Marete
Peter Moody
Andrew Gerrand
Dmitry Savintsev
Alexander Neumann
Giovanni Bajo
Taru Karttunen
KB Sriram
Keith Ball
Shenghou Ma
Joel Sing
datianshi