Most of the issues that led to the decision on #30055 were related to
incompatibility with or faulty support for RSA-PSS (#29831, #29779,
v1.5 signatures). RSA-PSS is required by TLS 1.3, but is also available
to be negotiated in TLS 1.2.
Altering TLS 1.2 behavior based on GODEBUG=tls13=1 feels surprising, so
just disable RSA-PSS entirely in TLS 1.2 until TLS 1.3 is on by default,
so breakage happens all at once.
Updates #30055
Change-Id: Iee90454a20ded8895e5302e8bcbcd32e4e3031c2
Reviewed-on: https://go-review.googlesource.com/c/160998
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
signatureSchemesForCertificate was written to be used with TLS 1.3, but
ended up used for TLS 1.2 client certificates in a refactor. Since it
only supported TLS 1.3 signature algorithms, it would lead to no RSA
client certificates being sent to servers that didn't support RSA-PSS.
TestHandshakeClientCertRSAPKCS1v15 was testing *specifically* for this,
but alas the OpenSSL flag -verify accepts an empty certificates list as
valid, as opposed to -Verify...
Fixes#28925
Change-Id: I61afc02ca501d3d64ab4ad77bbb4cf10931e6f93
Reviewed-on: https://go-review.googlesource.com/c/151660
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Since they are sent after the handshake in TLS 1.3, the client was not
actually consuming them, as it doesn't make any Read calls. They were
then sitting in the kernel receive buffer when the client would call
Close. The kernel would see that and send a RST, which would race the
closeNotify, causing errors.
Also, we get to trim 600 lines of useless test data.
Fixes#28852
Change-Id: I7517feab77dabab7504bfc111098ba09ea07ae5e
Reviewed-on: https://go-review.googlesource.com/c/151659
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TLS_FALLBACK_SCSV is extremely fragile in the presence of sparse
supported_version, but gave it the best try I could.
Set the server random canaries but don't check them yet, waiting for the
browsers to clear the way of misbehaving middleboxes.
Updates #9671
Change-Id: Ie55efdec671d639cf1e716acef0c5f103e91a7ce
Reviewed-on: https://go-review.googlesource.com/c/147617
Reviewed-by: Adam Langley <agl@golang.org>
Note that the SignatureSchemes passed to GetClientCertificate in TLS 1.2
are now filtered by the requested certificate type. This feels like an
improvement anyway, and the full list can be surfaced as well when
support for signature_algorithms_cert is added, which actually matches
the semantics of the CertificateRequest signature_algorithms in TLS 1.2.
Also, note a subtle behavior change in server side resumption: if a
certificate is requested but not required, and the resumed session did
not include one, it used not to invoke VerifyPeerCertificate. However,
if the resumed session did include a certificate, it would. (If a
certificate was required but not in the session, the session is rejected
in checkForResumption.) This inconsistency could be unexpected, even
dangerous, so now VerifyPeerCertificate is always invoked. Still not
consistent with the client behavior, which does not ever invoke
VerifyPeerCertificate on resumption, but it felt too surprising to
entirely change either.
Updates #9671
Change-Id: Ib2b0dbc30e659208dca3ac07d6c687a407d7aaaf
Reviewed-on: https://go-review.googlesource.com/c/147599
Reviewed-by: Adam Langley <agl@golang.org>
Added some assertions to testHandshake, but avoided checking the error
of one of the Close() because the one that would lose the race would
write the closeNotify to a connection closed on the other side which is
broken on js/wasm (#28650). Moved that Close() after the chan sync to
ensure it happens second.
Accepting a ticket with client certificates when NoClientCert is
configured is probably not a problem, and we could hide them to avoid
confusing the application, but the current behavior is to skip the
ticket, and I'd rather keep behavior changes to a minimum.
Updates #9671
Change-Id: I93b56e44ddfe3d48c2bef52c83285ba2f46f297a
Reviewed-on: https://go-review.googlesource.com/c/147445
Reviewed-by: Adam Langley <agl@golang.org>
Also check original certificate validity when resuming TLS 1.0–1.2. Will
refuse to resume a session if the certificate is expired or if the
original connection had InsecureSkipVerify and the resumed one doesn't.
Support only PSK+DHE to protect forward secrecy even with lack of a
strong session ticket rotation story.
Tested with NSS because s_server does not provide any way of getting the
same session ticket key across invocations. Will self-test like TLS
1.0–1.2 once server side is implemented.
Incorporates CL 128477 by @santoshankr.
Fixes#24919
Updates #9671
Change-Id: Id3eaa5b6c77544a1357668bf9ff255f3420ecc34
Reviewed-on: https://go-review.googlesource.com/c/147420
Reviewed-by: Adam Langley <agl@golang.org>
Looks like the introduction of CCS records in the client second flight
gave time to s_server to send NewSessionTicket messages in between the
client application data and close_notify. There seems to be no way of
turning NewSessionTicket messages off, neither by not sending a
psk_key_exchange_modes extension, nor by command line flag.
Interleaving the client write like that tickled an issue akin to #18701:
on Windows, the client reaches Close() before the last record is drained
from the send buffer, the kernel notices and resets the connection,
cutting short the last flow. There is no good way of synchronizing this,
so we sleep for a RTT before calling close, like in CL 75210. Sigh.
Updates #9671
Change-Id: I44dc1cca17b373695b5a18c2741f218af2990bd1
Reviewed-on: https://go-review.googlesource.com/c/147419
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Since TLS 1.3 delivers handshake messages (including KeyUpdate) after
the handshake, the want argument to readRecord had became almost
pointless: it only meant something when set to recordTypeChangeCipherSpec.
Replaced it with a bool to reflect that, and added two shorthands to
avoid anonymous bools in calls.
Took the occasion to simplify and formalize the invariants of readRecord.
The maxConsecutiveEmptyRecords loop became useless when readRecord
started retrying on any non-advancing record in CL 145297.
Replaced panics with errors, because failure is better than undefined
behavior, but contained failure is better than a DoS vulnerability. For
example, I suspect the panic at the top of readRecord was reachable from
handleRenegotiation, which calls readHandshake with handshakeComplete
false. Thankfully it was not a panic in 1.11, and it's allowed now.
Removed Client-TLSv13-RenegotiationRejected because OpenSSL isn't
actually willing to ask for renegotiation over TLS 1.3, the expected
error was due to NewSessionTicket messages, which didn't break the rest
of the tests because they stop too soon.
Updates #9671
Change-Id: I297a81bde5c8020a962a92891b70d6d70b90f5e3
Reviewed-on: https://go-review.googlesource.com/c/147418
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Also, add support for the SSLKEYLOGFILE environment variable to the
tests, to simplify debugging of unexpected failures.
Updates #9671
Change-Id: I20a34a5824f083da93097b793d51e796d6eb302b
Reviewed-on: https://go-review.googlesource.com/c/147417
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
crypto/x509 already supports PSS signatures (with rsaEncryption OID),
and crypto/tls support was added in CL 79736. Advertise support for the
algorithms and accept them as a peer.
Note that this is about PSS signatures from regular RSA public keys.
RSA-PSS only public keys (with RSASSA-PSS OID) are supported in neither
crypto/tls nor crypto/x509. See RFC 8446, Section 4.2.3.
testdata/Server-TLSv12-ClientAuthRequested* got modified because the
CertificateRequest carries the supported signature algorithms.
The net/smtp tests changed because 512 bits keys are too small for PSS.
Based on Peter Wu's CL 79738, who did all the actual work in CL 79736.
Updates #9671
Change-Id: I4a31e9c6e152ff4c50a5c8a274edd610d5fff231
Reviewed-on: https://go-review.googlesource.com/c/146258
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
RFC 8446 recommends using the supported_versions extension to negotiate
lower versions as well, so begin by implementing it to negotiate the
currently supported versions.
Note that pickTLSVersion was incorrectly negotiating the ServerHello
version down on the client. If the server had illegally sent a version
higher than the ClientHello version, the client would have just
downgraded it, hopefully failing later in the handshake.
In TestGetConfigForClient, we were hitting the record version check
because the server would select TLS 1.1, the handshake would fail on the
client which required TLS 1.2, which would then send a TLS 1.0 record
header on its fatal alert (not having negotiated a version), while the
server would expect a TLS 1.1 header at that point. Now, the client gets
to communicate the minimum version through the extension and the
handshake fails on the server.
Updates #9671
Change-Id: Ie33c7124c0c769f62e10baad51cbed745c424e5b
Reviewed-on: https://go-review.googlesource.com/c/146217
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
The arm5 and mips builders are can't-send-a-packet-to-localhost-in-1s
slow apparently. 1m is less useful, but still better than an obscure
test timeout panic.
Fixes#28405
Change-Id: I2feeae6ea1b095114caccaab4f6709f405faebad
Reviewed-on: https://go-review.googlesource.com/c/145037
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If something causes the recorded tests to deviate from the expected
flows, they might wait forever for data that is not coming. Add a short
timeout, after which a useful error message is shown.
Change-Id: Ib11ccc0e17dcb8b2180493556017275678abbb08
Reviewed-on: https://go-review.googlesource.com/c/144116
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
crypto/tls is meant to work over network connections with buffering, not
synchronous connections, as explained in #24198. Tests based on net.Pipe
are unrealistic as reads and writes are matched one to one. Such tests
worked just thanks to the implementation details of the tls.Conn
internal buffering, and would break if for example the flush of the
first flight of the server was not entirely assimilated by the client
rawInput buffer before the client attempted to reply to the ServerHello.
Note that this might run into the Darwin network issues at #25696.
Fixed a few test races that were either hidden or synchronized by the
use of the in-memory net.Pipe.
Also, this gets us slightly more realistic benchmarks, reflecting some
syscall cost of Read and Write operations.
Change-Id: I5a597b3d7a81b8ccc776030cc837133412bf50f8
Reviewed-on: https://go-review.googlesource.com/c/142817
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The unexported field is hidden from reflect based marshalers, which
would break otherwise. Also, make it return an error, as there are
multiple reasons it might fail.
Fixes#27125
Change-Id: I92adade2fe456103d2d5c0315629ca0256953764
Reviewed-on: https://go-review.googlesource.com/130535
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The existing implementation of TLS connection has a deadlock. It occurs
when client connects to TLS server and doesn't send data for
handshake, so server calls Close on this connection. This is because
server reads data under locked mutex, while Close method tries to
lock the same mutex.
Fixes#23518
Change-Id: I4fb0a2a770f3d911036bfd9a7da7cc41c1b27e19
Reviewed-on: https://go-review.googlesource.com/90155
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This adds support for RSASSA-PSS signatures in handshake messages as
required by TLS 1.3. Even if TLS 1.2 is negotiated, it must support PSS
when advertised in the Client Hello (this will be done later as the
testdata will change).
Updates #9671
Change-Id: I8006b92e017453ae408c153233ce5ccef99b5c3f
Reviewed-on: https://go-review.googlesource.com/79736
Reviewed-by: Filippo Valsorda <filippo@golang.org>
bytes.IndexByte can be used wherever the second argument to
strings.Index is exactly one byte long, so we do that with this change.
This avoids generating unnecessary string symbols/converison and saves
a few calls to bytes.Index.
Change-Id: If31c775790e01edfece1169e398ad6a754fb4428
Reviewed-on: https://go-review.googlesource.com/66373
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestGetClientCertificate had disabled verification, and was only passing
because it was mistakenly checking for empty verifiedChains.
Change-Id: Iea0ddbdbbdf8ac34b499569820a2e4ce543a69c7
Reviewed-on: https://go-review.googlesource.com/47430
Run-TryBot: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Fix spelling of "original" and "occurred" in new gofmt docs. The same
misspelling of "occurred" was also present in crypto/tls, I fixed it there as
well.
Change-Id: I67b4f1c09bd1a2eb1844207d5514f08a9f525ff9
Reviewed-on: https://go-review.googlesource.com/33138
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I used the slowtests.go tool as described in
https://golang.org/cl/32684 on packages that stood out.
go test -short std drops from ~56 to ~52 seconds.
This isn't a huge win, but it was mostly an exercise.
Updates #17751
Change-Id: I9f3402e36a038d71e662d06ce2c1d52f6c4b674d
Reviewed-on: https://go-review.googlesource.com/32751
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, the selection of a client certificate done internally based
on the limitations given by the server's request and the certifcates in
the Config. This means that it's not possible for an application to
control that selection based on details of the request.
This change adds a callback, GetClientCertificate, that is called by a
Client during the handshake and which allows applications to select the
best certificate at that time.
(Based on https://golang.org/cl/25570/ by Bernd Fix.)
Fixes#16626.
Change-Id: Ia4cea03235d2aa3c9fd49c99c227593c8e86ddd9
Reviewed-on: https://go-review.googlesource.com/32115
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
VerifyPeerCertificate returns an error if the peer should not be
trusted. It will be called after the initial handshake and before
any other verification checks on the cert or chain are performed.
This provides the callee an opportunity to augment the certificate
verification.
If VerifyPeerCertificate is not nil and returns an error,
then the handshake will fail.
Fixes#16363
Change-Id: I6a22f199f0e81b6f5d5f37c54d85ab878216bb22
Reviewed-on: https://go-review.googlesource.com/26654
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change adds support for the ChaCha20-Poly1305 AEAD to crypto/tls,
as specified in https://tools.ietf.org/html/rfc7905.
Fixes#15499.
Change-Id: Iaa689be90e03f208c40b574eca399e56f3c7ecf1
Reviewed-on: https://go-review.googlesource.com/30957
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
X25519 (RFC 7748) is now commonly used for key agreement in TLS
connections, as specified in
https://tools.ietf.org/html/draft-ietf-tls-curve25519-01.
This change adds support for that in crypto/tls, but does not enabled it
by default so that there's less test noise. A future change will enable
it by default and will update all the test data at the same time.
Change-Id: I91802ecd776d73aae5c65bcb653d12e23c413ed4
Reviewed-on: https://go-review.googlesource.com/30824
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When updating the test data against OpenSSL, the handshake can fail and
the stdout/stderr output of OpenSSL is very useful in finding out why.
However, printing that output has been broken for some time because its
no longer sent to a byte.Buffer. This change fixes that.
Change-Id: I6f846c7dc80f1ccee9fa1be36f0b579b3754e05f
Reviewed-on: https://go-review.googlesource.com/30823
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We will need OpenSSL 1.1.0 in order to test some of the features
expected for Go 1.8. However, 1.1.0 also disables (by default) some
things that we still want to test, such as RC4, 3DES and SSLv3. Thus
developers wanting to update the crypto/tls test data will need to build
OpenSSL from source.
This change updates the test data with transcripts generated by 1.1.0
(in order to reduce future diffs) and also causes a banner to be printed
if 1.1.0 is not used when updating.
(The test for an ALPN mismatch is removed because OpenSSL now terminates
the connection with a fatal alert if no known ALPN protocols are
offered. There's no point testing against this because it's an OpenSSL
behaviour.)
Change-Id: I957516975e0b8c7def84184f65c81d0b68f1c551
Reviewed-on: https://go-review.googlesource.com/30821
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since there's no aspect of key logging that OpenSSL can check for us,
the tests for it might as well just connect to another goroutine as this
is lower-maintainance.
Change-Id: I746d1dbad1b4bbfc8ef6ccf136ee4824dbda021e
Reviewed-on: https://go-review.googlesource.com/30089
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joonas Kuorilehto <joneskoo@derbian.fi>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
After renegotiation support was added (af125a5193c) it's possible for a
Write to block on a Read when racing to complete the handshake:
1. The Write determines that a handshake is needed and tries to
take the neccesary locks in the correct order.
2. The Read also determines that a handshake is needed and wins
the race to take the locks.
3. The Read goroutine completes the handshake and wins a race
to unlock and relock c.in, which it'll hold when waiting for
more network data.
If the application-level protocol requires the Write to complete before
data can be read then the system as a whole will deadlock.
Unfortunately it doesn't appear possible to reverse the locking order of
c.in and handshakeMutex because we might read a renegotiation request at
any point and need to be able to do a handshake without unlocking.
So this change adds a sync.Cond that indicates that a goroutine has
committed to doing a handshake. Other interested goroutines can wait on
that Cond when needed.
The test for this isn't great. I was able to reproduce the deadlock with
it only when building with -race. (Because -race happened to alter the
timing just enough.)
Fixes#17101.
Change-Id: I4e8757f7b82a84e46c9963a977d089f0fb675495
Reviewed-on: https://go-review.googlesource.com/29164
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Since 2a8c81ff handshake messages are not written directly to wire but
buffered. If an error happens at the wrong time the alert will be
written to the buffer but never flushed, causing an EOF on the client
instead of a more descriptive alert.
Thanks to Brendan McMillion for reporting this.
Fixes#17037
Change-Id: Ie093648aa3f754f4bc61c2e98c79962005dd6aa2
Reviewed-on: https://go-review.googlesource.com/28818
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In Go 1.0, the Config struct consisted only of exported fields.
In Go 1.1, it started to grow private, uncopyable fields (sync.Once,
sync.Mutex, etc).
Ever since, people have been writing their own private Config.Clone
methods, or risking it and doing a language-level shallow copy and
copying the unexported sync variables.
Clean this up and export the Config.clone method as Config.Clone.
This matches the convention of Template.Clone from text/template and
html/template at least.
Fixes#15771
Updates #16228 (needs update in x/net/http2 before fixed)
Updates #16492 (not sure whether @agl wants to do more)
Change-Id: I48c2825d4fef55a75d2f99640a7079c56fce39ca
Reviewed-on: https://go-review.googlesource.com/28075
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Add support for writing TLS client random and master secret
in NSS key log format.
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format
Normally this is enabled by a developer debugging TLS based
applications, especially HTTP/2, by setting the KeyLogWriter
to an open file. The keys negotiated in handshake are then
logged and can be used to decrypt TLS sessions e.g. in Wireshark.
Applications may choose to add support similar to NSS where this
is enabled by environment variable, but no such mechanism is
built in to Go. Instead each application must explicitly enable.
Fixes#13057.
Change-Id: If6edd2d58999903e8390b1674ba4257ecc747ae1
Reviewed-on: https://go-review.googlesource.com/27434
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If SetSessionTicketKeys was called on a fresh tls.Config, the configured
keys would be overridden with a random key by serverInit.
Fixes#15421.
Change-Id: I5d6cc81fc3e5de4dfa15eb614d102fb886150d1b
Reviewed-on: https://go-review.googlesource.com/27317
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These were new with TLS 1.2 and, reportedly, some servers require it.
Since it's easy, this change adds suport for three flavours of
AES-128-CBC with SHA-256 MACs.
Other testdata/ files have to be updated because this changes the list
of cipher suites offered by default by the client.
Fixes#15487.
Change-Id: I1b14330c31eeda20185409a37072343552c3464f
Reviewed-on: https://go-review.googlesource.com/27315
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Jonathan Rudenberg <jonathan@titanous.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This fixes some 40 warnings from go vet.
Fixes#16134.
Change-Id: Ib9fcba275fe692f027a2a07b581c8cf503b11087
Reviewed-on: https://go-review.googlesource.com/24287
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This change causes TLS handshake messages to be buffered and written in
a single Write to the underlying net.Conn.
There are two reasons to want to do this:
Firstly, it's slightly preferable to do this in order to save sending
several, small packets over the network where a single one will do.
Secondly, since 37c28759ca46cf381a466e32168a793165d9c9e9 errors from
Write have been returned from a handshake. This means that, if a peer
closes the connection during a handshake, a “broken pipe” error may
result from tls.Conn.Handshake(). This can mask any, more detailed,
fatal alerts that the peer may have sent because a read will never
happen.
Buffering handshake messages means that the peer will not receive, and
possibly reject, any of a flow while it's still being written.
Fixes#15709
Change-Id: I38dcff1abecc06e52b2de647ea98713ce0fb9a21
Reviewed-on: https://go-review.googlesource.com/23609
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds Config.Renegotiation which controls whether a TLS
client will accept renegotiation requests from a server. This is used,
for example, by some web servers that wish to “add” a client certificate
to an HTTPS connection.
This is disabled by default because it significantly complicates the
state machine.
Originally, handshakeMutex was taken before locking either Conn.in or
Conn.out. However, if renegotiation is permitted then a handshake may
be triggered during a Read() call. If Conn.in were unlocked before
taking handshakeMutex then a concurrent Read() call could see an
intermediate state and trigger an error. Thus handshakeMutex is now
locked after Conn.in and the handshake functions assume that Conn.in is
locked for the duration of the handshake.
Additionally, handshakeMutex used to protect Conn.out also. With the
possibility of renegotiation that's no longer viable and so
writeRecordLocked has been split off.
Fixes#5742.
Change-Id: I935914db1f185d507ff39bba8274c148d756a1c8
Reviewed-on: https://go-review.googlesource.com/22475
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This promotes a connection hang during TLS handshake to a proper error.
This doesn't fully address #14539 because the error reported in that
case is a write-on-socket-not-connected error, which implies that an
earlier error during connection setup is not being checked, but it is
an improvement over the current behaviour.
Updates #14539.
Change-Id: I0571a752d32d5303db48149ab448226868b19495
Reviewed-on: https://go-review.googlesource.com/19990
Reviewed-by: Adam Langley <agl@golang.org>
This is a followup change to #13111 for filtering out IPv6 literals and
absolute FQDNs from being as the SNI values.
Updates #13111.
Fixes#14404.
Change-Id: I09ab8d2a9153d9a92147e57ca141f2e97ddcef6e
Reviewed-on: https://go-review.googlesource.com/19704
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
During the TLS handshake, check the cipher suite the server selects is
one of those offered in the ClientHello. The code was checking it was
in the larger list that was sometimes whittled down for the ClientHello.
Fixes#13174
Change-Id: Iad8eebbcfa5027f30403b9700c43cfa949e135bb
Reviewed-on: https://go-review.googlesource.com/16698
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(This relands commit a4dcc692011bf1ceca9b1a363fd83f3e59e399ee.)
https://tools.ietf.org/html/rfc6066#section-3 states:
“Literal IPv4 and IPv6 addresses are not permitted in "HostName".”
However, if an IP literal was set as Config.ServerName (which could
happen as easily as calling Dial with an IP address) then the code would
send the IP literal as the SNI value.
This change filters out IP literals, as recognised by net.ParseIP, from
being sent as the SNI value.
Fixes#13111.
Change-Id: I6e544a78a01388f8fe98150589d073b917087f75
Reviewed-on: https://go-review.googlesource.com/16776
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
https://tools.ietf.org/html/rfc6066#section-3 states:
“Literal IPv4 and IPv6 addresses are not permitted in "HostName".”
However, if an IP literal was set as Config.ServerName (which could
happen as easily as calling Dial with an IP address) then the code would
send the IP literal as the SNI value.
This change filters out IP literals, as recognised by net.ParseIP, from
being sent as the SNI value.
Fixes#13111.
Change-Id: Ie9ec7acc767ae172b48c9c6dd8d84fa27b1cf0de
Reviewed-on: https://go-review.googlesource.com/16742
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
