This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.
In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.
Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.
On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.
On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.
There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.
The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.
Updates #32426
Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
Also, fix the alert value sent when a signature by a client certificate
is invalid in TLS 1.0-1.2.
Fixes#35190
Change-Id: I2ae1d5593dfd5ee2b4d979664aec74aab4a8a704
Reviewed-on: https://go-review.googlesource.com/c/go/+/204157
Reviewed-by: Katie Hockman <katie@golang.org>
SSLv3 has been irreparably broken since the POODLE attack 5 years ago
and RFC 7568 (f.k.a. draft-ietf-tls-sslv3-diediedie) prohibits its use
in no uncertain terms.
As announced in the Go 1.13 release notes, remove support for it
entirely in Go 1.14.
Updates #32716
Change-Id: Id653557961d8f75f484a01e6afd2e104a4ccceaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/191976
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Signing with RSA-PSS can uncover faulty crypto.Signer implementations,
and it can fail for (broken) small keys. We'll have to take that
breakage eventually, but it would be nice for it to be opt-out at first.
TLS 1.3 requires RSA-PSS and is opt-out in Go 1.13. Instead of making a
TLS 1.3 opt-out influence a TLS 1.2 behavior, let's wait to add RSA-PSS
to TLS 1.2 until TLS 1.3 is on without opt-out.
Note that since the Client Hello is sent before a protocol version is
selected, we have to advertise RSA-PSS there to support TLS 1.3.
That means that we still support RSA-PSS on the client in TLS 1.2 for
verifying server certificates, which is fine, as all issues arise on the
signing side. We have to be careful not to pick (or consider available)
RSA-PSS on the client for client certificates, though.
We'd expect tests to change only in TLS 1.2:
* the server won't pick PSS to sign the key exchange
(Server-TLSv12-* w/ RSA, TestHandshakeServerRSAPSS);
* the server won't advertise PSS in CertificateRequest
(Server-TLSv12-ClientAuthRequested*, TestClientAuth);
* and the client won't pick PSS for its CertificateVerify
(Client-TLSv12-ClientCert-RSA-*, TestHandshakeClientCertRSAPSS,
Client-TLSv12-Renegotiate* because "R" requests a client cert).
Client-TLSv13-ClientCert-RSA-RSAPSS was updated because of a fix in the test.
This effectively reverts 88343530720a52c96b21f2bd5488c8fb607605d7.
Testing was made more complex by the undocumented semantics of OpenSSL's
-[client_]sigalgs (see openssl/openssl#9172).
Updates #32425
Change-Id: Iaddeb2df1f5c75cd090cc8321df2ac8e8e7db349
Reviewed-on: https://go-review.googlesource.com/c/go/+/182339
Reviewed-by: Adam Langley <agl@golang.org>
As suggested by dmitshur@, move them to their own block so they don't
conflict with changes in the upstream imports.
Change-Id: Id46fb7c766066c406023b0355f4c3c860166f0fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/181277
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Support for Ed25519 certificates was added in CL 175478, this wires them
up into the TLS stack according to RFC 8422 (TLS 1.2) and RFC 8446 (TLS 1.3).
RFC 8422 also specifies support for TLS 1.0 and 1.1, and I initially
implemented that, but even OpenSSL doesn't take the complexity, so I
just dropped it. It would have required keeping a buffer of the
handshake transcript in order to do the direct Ed25519 signatures. We
effectively need to support TLS 1.2 because it shares ClientHello
signature algorithms with TLS 1.3.
While at it, reordered the advertised signature algorithms in the rough
order we would want to use them, also based on what curves have fast
constant-time implementations.
Client and client auth tests changed because of the change in advertised
signature algorithms in ClientHello and CertificateRequest.
Fixes#25355
Change-Id: I9fdd839afde4fd6b13fcbc5cc7017fd8c35085ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/177698
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
In Go 1.13 we will enable RSA-PSS in TLS 1.2 at the same time as we make
TLS 1.3 enabled by default.
This reverts commit 7ccd3583eddcd79679fb29cfc83a6e6fb6973f1e.
Updates #30055
Change-Id: I6f2ddf7652d1172a6b29f4e335ff3a71a89974bc
Reviewed-on: https://go-review.googlesource.com/c/163080
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
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>
The Config does not own the memory pointed to by the Certificate slice.
Instead, opportunistically use Certificate.Leaf and let the application
set it if it desires the performance gain.
This is a partial rollback of CL 107627. See the linked issue for the
full explanation.
Fixes#28744
Change-Id: I33ce9e6712e3f87939d9d0932a06d24e48ba4567
Reviewed-on: https://go-review.googlesource.com/c/149098
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix a couple overlooked ConnectionState fields noticed by net/http
tests, and add a test in crypto/tls. Spun off CL 147638 to keep that one
cleanly about enabling TLS 1.3.
Change-Id: I9a6c2e68d64518a44be2a5d7b0b7b8d78c98c95d
Reviewed-on: https://go-review.googlesource.com/c/148900
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
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>
Implement a basic TLS 1.3 server handshake, only enabled if explicitly
requested with MaxVersion.
This CL intentionally leaves for future CLs:
- PSK modes and resumption
- client authentication
- compatibility mode ChangeCipherSpecs
- early data skipping
- post-handshake messages
- downgrade protection
- KeyLogWriter support
- TLS_FALLBACK_SCSV processing
It also leaves a few areas up for a wider refactor (maybe in Go 1.13):
- the certificate selection logic can be significantly improved,
including supporting and surfacing signature_algorithms_cert, but
this isn't new in TLS 1.3 (see comment in processClientHello)
- handshake_server_tls13.go can be dried up and broken into more
meaningful, smaller functions, but it felt premature to do before
PSK and client auth support
- the monstrous ClientHello equality check in doHelloRetryRequest can
get both cleaner and more complete with collaboration from the
parsing layer, which can come at the same time as extension
duplicates detection
Updates #9671
Change-Id: Id9db2b6ecc2eea21bf9b59b6d1d9c84a7435151c
Reviewed-on: https://go-review.googlesource.com/c/147017
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>
Note that there is significant code duplication due to extensions with
the same format appearing in different messages in TLS 1.3. This will be
cleaned up in a future refactor once CL 145317 is merged.
Enforcing the presence/absence of each extension in each message is left
to the upper layer, based on both protocol version and extensions
advertised in CH and CR. Duplicated extensions and unknown extensions in
SH, EE, HRR, and CT will be tightened up in a future CL.
The TLS 1.2 CertificateStatus message was restricted to accepting only
type OCSP as any other type (none of which are specified so far) would
have to be negotiated.
Updates #9671
Change-Id: I7c42394c5cc0af01faa84b9b9f25fdc6e7cfbb9e
Reviewed-on: https://go-review.googlesource.com/c/145477
Reviewed-by: Adam Langley <agl@golang.org>
I am working on a TLS server program, which issues new TLS certificates
on demand. The new certificates will be added into tls.Config.Certificates.
BuildNameToCertificate will be called to refresh the name table afterwards.
This change will reduce some workload on existing certificates.
Note that you can’t modify the Certificates field (or call BuildNameToCertificate)
on a Config in use by a Server. You can however modify an unused Config that gets
cloned in GetConfigForClient with appropriate locking.
Change-Id: I7bdb7d23fc5d68df83c73f3bfa3ba9181d38fbde
GitHub-Last-Rev: c3788f4116be47f2fdb777935c421e7dd694f5c8
GitHub-Pull-Request: golang/go#24920
Reviewed-on: https://go-review.googlesource.com/c/107627
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This change will aid users to make less mistakes where you, for example, define both HTTP/1.1 and H2, but in the wrong order.
package main
import (
"crypto/tls"
"net"
)
func main() {
srv := &http.Server{
TLSConfig: &tls.Config{
NextProtos: []string{"http/1.1", "h2"},
},
}
srv.ListenAndServeTLS("server.crt", "server.key")
}
When using major browsers or curl, they will never be served H2 since they also support HTTP/1.0 and the list is processed in order.
Change-Id: Id14098b5e48f624ca308137917874d475c2f22a0
GitHub-Last-Rev: f3594a6411bf7dde71c850f3e85a2b5a21974129
GitHub-Pull-Request: golang/go#28367
Reviewed-on: https://go-review.googlesource.com/c/144387
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Use the format "RFC XXXX, Section X.X" (or "Appendix Y.X") as it fits
more properly in prose than a link, is more future-proof, and as there
are multiple ways to render an RFC. Capital "S" to follow the quoting
standard of RFCs themselves.
Applied the new goimports grouping to all files in those packages, too.
Change-Id: I01267bb3a3b02664f8f822e97b129075bb14d404
Reviewed-on: https://go-review.googlesource.com/c/141918
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
