Send/Skip CCS, set legacy record version to 3,3 and echo session ID.
CCS must be ignored while the handshake is running, but not thereafter:
https://tools.ietf.org/html/draft-ietf-tls-tls13-22#section-5
Unconditionally send CCS as server because bogo requires it, even if no
session ID is included in the Client Hello. TLS 1.3 clients MUST ignore
it anyway, so it should not hurt.
Fixes interop with boringssl and openssl and passes bogo.
Draft 21 changed end_of_early_data from an alert into a handshake
message to allow it to integrate better with the handshake. This change
does that, rather than handling EOD at the record layer, it moves
processing up to the actual readers of (early) application data.
D19: use early_data instead of custom ticket_early_data_info extension
codepoint. D21: new ticket nonce field and change in PSK calculation.
This nonce provides some minor security advantage in case one of the PSK
is compromised (which would leak the resumption master secret).
Rename "resumptionSecret" to "pskSecret" in sessionState13 to reflect
the D21 change and use constant-time comparison for the secret.
Also fix potential panic if the ticket is large enough, but the
extensions are missing.
D19 added an additional pre-extract Derive-Secret stage. D20 shortened
labels. Bump from D18 to D21 with no backwards compat option for now
since older drafts are considered undeployable.
Similar to boringssl, reuse the NSS client image for the NSS server test
against the tris client. Bump the NSS version to 3.34.1 gain support
for TLS 1.3 keylogging which is useful while debugging.
Adjust read check to fix intermittent NSS test failures:
https://github.com/cloudflare/tls-tris/issues/58
Prepare framework for testing tls-tris as client against other servers.
Currently only boringssl is implemented, but the idea is to add support
for others too (NSS, OpenSSL, picotls, tris, ...).
To test multiple certificate types, copy ecdsa.pem and rsa.pem from
tris-localserver for boringssl. The boringssl image is reused for the
server since the binaries were built anyway. Revision is bumped to
something to fix a build error and make the -loop and -www options work.
Support validation of ECDSA and RSASSA-PSS signatures. Explicitly do not
support PKCS1-v1_5 signatures since these are not allowed for handshake
messages.
Until PSS support is finalized for TLS 1.2, let's not advertise support
for that. Since TLS 1.3 however mandates PSS, we have no other option
than advertising this even if we have not added complete support.
Another reason why I apply it to just TLS 1.3 and not 1.2 is because the
latter would require updating the testdata.
Advertise TLS 1.3 in supported_versions and send a key share for the
first preferred curve. Missing are HRR, certificate validation and
Encrypted Extensions processing (see TODO notes).
For simplicity only a single key share is remembered. This key share
should be updated with a HRR (when implemented).
This change splits handshake processing for TLS 1.3, reindenting the TLS
1.2 code path and splitting initializationg of the handshake hash. No
equivalent is added for processServerHello because session resumption is
not supported yet.
Skip reading the session cache if TLS 1.3 is in use (the cache has no
use), skip storing a session if TLS 1.3 is in use (sessionCache can
still be set when TLS 1.2 is allowed).
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
ServerKeyExchange and CertificateVerify can share the same logic for
picking a signature algorithm (based on the certificate public key and
advertised algorithms), selecting a hash algorithm (depending on TLS
version) and signature verification.
Refactor the code to achieve code reuse, have common error checking
(especially for intersecting supported signature algorithms) and to
prepare for addition of new signature algorithms. Code should be easier
to read since version-dependent logic is concentrated at one place.
Change-Id: I978dec3815d28e33c3cfbc85f0c704b1894c25a3
All TLS 1.3 cipher suites so far use a nonce length of 12, but that does
not have to be the case. Correct the cipher suite definition and use it.
Spec: https://tools.ietf.org/html/draft-ietf-tls-tls13-22#section-5.3
Note: there is no functional change, the values were previously unused.
Fixes: ("[dev.tls] crypto/tls: implement TLS 1.3 cipher suites")
Disable 0.5-RTT as it has weaker security properties than 1-RTT. The
same security considerations from TLS False Start (RFC 7918) apply.
Currently the server Handshake method returns as soon as it has sent its
parameters, but it does not wait for the client to authenticate the
handshake via a Finished message. This broke a test that assumed that
the Handshake message performs a full handshake and also
(unintentionally?) enabled the server to send application data before
the handshake is complete ("0.5-RTT data").
Fix this by moving the implicit Finished message check in the handshake
message reader to the server handshake itself (previously readRecord
would process the Finished message as a side-effect of requesting
recordTypeApplicationData). And in the special case where 0-RTT data is
actually desired, process the Finished message in the ConfirmHandshake
and Read functions.
NOTE: 0.5-RTT is not disabled when the server enables 0-RTT. It is the
server responsibility to use ConfirmHandshake before writing anything.
Explicitly panic when ConfirmHandshake is used for client connections,
this is not the intended use of that API.
Do not assume that every post-handshake messages are HelloRequests that
try to trigger a renegotiation. This could result in a no_renegotiation
(rather than an unexpected_message) alert even for other message types
(like NewSessionTicket).
This change makes the caller of readRecord(recordTypeApplicationData)
responsible for checking for renegotiation in case of a handshake
message, but that is currently already the case. And the condition
"c.phase == waitingClientFinished" can only be hit by the server, so
that won't have break the handshake either.
Related: https://github.com/cloudflare/tls-tris/issues/50
Merge serverHelloMsg13 into serverHelloMsg in preparation for draft 22.
This will also simplify the client implementation since only one
structure has to be checked.
Also fixed potential out-of-bounds access with keyShare unmarshal.
The record layer splits application data into chunks of at most 2^14
octets. When record protection is engaged in TLS 1.3, the application
data is serialized into a TLSInnerPlaintext which has an additional byte
for the content type, resulting in a maximum length of 2^14+1.
Fixes LargeMessage, TLS13-AEAD-CHACHA20-POLY1305-LargeRecord,
TLS13-AEAD-AES128-GCM-SHA256-LargeRecord and
TLS13-AEAD-AES256-GCM-SHA384-LargeRecord bogo tests.
Fixes: https://github.com/cloudflare/tls-tris/issues/46
Merge upstream go post-1.9 crypto/tls changes from master:
d8ee5d11e5 crypto/tls: limit number of consecutive warning alerts
96cd66b266 crypto/tls: advertise support for SHA-512 signatures in 1.2
f265f5db5d archive/zip, crypto/tls: use rand.Read instead of casting ints to bytes
54d04c2fcb crypto/tls: remove bookkeeping code from pHash function
d1bbdbe760 crypto/tls: replace signatureAndHash by SignatureScheme.
cb3b345209 crypto/tls: fix first byte test for 255 CBC padding bytes
d153df8e4b all: revert "all: prefer strings.LastIndexByte over strings.LastIndex"
5e42658fc0 all: prefer bytes.IndexByte over bytes.Index
d2826d3e06 all: prefer strings.LastIndexByte over strings.LastIndex
5a986eca86 all: fix article typos
0f9a2cf2c4 crypto/tls: fix clientHelloMsg fuzzer not to generate the RI SCSV
e7d46cee2f crypto/tls: fix and expand TestVerifyPeerCertificate and TestGetClientCertificate
85deaf6077 crypto/tls: fix docstring of Config.ClientSessionCache
4a5f85babb crypto/tls: disallow handshake messages fragmented across CCS
b3465646ff crypto/tls: add BenchmarkHandshakeServer
d38d357c78 crypto/tls: don't check whether an ec point is on a curve twice
e085a891f0 crypto/tls: split clientHandshake into multiple methods
Conflicts:
* handshake_client.go: conflict between our ("crypto/tls: allow client to
pick TLS 1.3, do not enable it by default.") and upstream
("crypto/tls: split clientHandshake into multiple methods"), resolve
by applying the mutualVersion->pickVersion change in pickTLSVersion.
* handshake_server.go: trivial conflict due to upstreamed patch
("crypto/tls: replace signatureAndHash by SignatureScheme.") and
("crypto/tls: implement TLS 1.3 server 0-RTT") which added pskBinder.
Other merge changes:
* tls13.go: signatureAndHashes as added in ("crypto/tls: implement TLS
1.3 minimal server") was renamed as required by ("crypto/tls: replace
signatureAndHash by SignatureScheme.").
* handshake_client.go: moved check from ("crypto/tls: check that client
cipher suite matches version") to pickCipherSuite as required by
("crypto/tls: split clientHandshake into multiple methods").
Not sure why the retry logic was removed since 0-RTT works even in
presence of the special alerts handling (alertEndOfEarlyData is
processed before the warning alert). To reduce divergence from upstream
code (which adds a restriction on the number of consecutive warnings),
restore the original retry logic.
Do not do anything fancy here, later drafts will remove the special
alert handling since it becomes a special handshake message.
Fixes: ("crypto/tls: implement TLS 1.3 server 0-RTT")
In the current implementation, it is possible for a client to
continuously send warning alerts, which are just dropped on the floor
inside readRecord.
This can enable scenarios in where someone can try to continuously
send warning alerts to the server just to keep it busy.
This CL implements a simple counter that triggers an error if
we hit the warning alert limit.
Fixes#22543
Change-Id: Ief0ca10308cf5a4dea21a5a67d3e8f6501912da6
Reviewed-on: https://go-review.googlesource.com/75750
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is the equivalent change to 1c105980 but for SHA-512.
SHA-512 certificates are already supported by default since b53bb2ca,
but some servers will refuse connections if the algorithm is not
advertised in the overloaded signatureAndHash extension (see 09b238f1).
This required adding support for SHA-512 signatures on CertificateVerify
and ServerKeyExchange messages, because of said overloading.
Some testdata/Client-TLSv1{0,1} files changed because they send a 1.2
ClientHello even if the server picks a lower version.
Closes#22422
Change-Id: I16282d03a3040260d203711ec21e6b20a0e1e105
Reviewed-on: https://go-review.googlesource.com/74950
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Since copy function can figure out how many bytes of data to copy when
two slices have different length, it is not necessary to check how many
bytes need to copy each time before copying the data.
Change-Id: I5151ddfe46af5575566fe9c9a2648e111575ec3d
Reviewed-on: https://go-review.googlesource.com/71090
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Consolidate the signature and hash fields (SignatureAndHashAlgorithm in
TLS 1.2) into a single uint16 (SignatureScheme in TLS 1.3 draft 21).
This makes it easier to add RSASSA-PSS for TLS 1.2 in the future.
Fields were named like "signatureAlgorithm" rather than
"signatureScheme" since that name is also used throughout the 1.3 draft.
The only new public symbol is ECDSAWithSHA1, other than that this is an
internal change with no new functionality.
Change-Id: Iba63d262ab1af895420583ac9e302d9705a7e0f0
Reviewed-on: https://go-review.googlesource.com/62210
Reviewed-by: Adam Langley <agl@golang.org>
The BadCBCPadding255 test from bogo failed because at most 255 trailing
bytes were checked, but for a padding of 255 there are 255 padding bytes
plus 1 length byte with value 255.
Change-Id: I7dd237c013d2c7c8599067246e31b7ba93106cf7
Reviewed-on: https://go-review.googlesource.com/68070
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The default version (TLS 1.2) is no longer overridden with TLS 1.3 so
the server must explicitly set it.
Fixes: ("crypto/tls: allow client to pick TLS 1.3, do not enable it by default.")
mutualVersion takes a maximum version, but skips TLS 1.3 because this
version is not negotiated via ClientHello.legacy_version. The server
however still uses its ServerHello.version field to select a version
from the supported_versions extension and the client must do the same.
A new getSupportedVersions method is introduced to have a single place
to handle the mapping of VersionTLS13 to the draft values.
Remove the MaxVersion override to TLS 1.3, users must set MaxVersion if
they intent to use the experimental TLS 1.3 functionality.
Fixes: ("crypto/tls: make 1.3 version negotiation more robust")
To allow clients to advertise both TLS 1.2 and TLS 1.3 cipher suites,
remove the distinction between both suites. TLS 1.3 suites are now
always included in the default cipher list (and the client will send it
if MaxVersion allows for it).
Since TLS 1.3 is expected to become the default MaxVersion and
applications might have set only TLS 1.2 cipher suites in their
configuration, TLS 1.3 cipher suites are added when none are present.
Alternatively, I considered disallowing overriding the TLS 1.3 suites,
but that requires more complexity and has not much benefits. Provide a
mechanism and do not dictate policy, application developers might want
to fix a cipher suite for testing other implementations for example.
Fixes https://github.com/cloudflare/tls-tris/pull/22
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>
strings.LastIndexByte was introduced in go1.5 and it can be used
effectively wherever the second argument to strings.LastIndex is
exactly one byte long.
This avoids generating unnecessary string symbols and saves
a few calls to strings.LastIndex.
Change-Id: I7b5679d616197b055cffe6882a8675d24a98b574
Reviewed-on: https://go-review.googlesource.com/66372
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use the new keySchedule13 type instead of hash.Hash to avoid tracking
the hashContext and intermediate secrets manually.
checkPSK is modified not to return the calculated early secret, this is
internal to keySchedule13 now. The caller just learns whether it was
resumed using a PSK or not.
In order to reduce repetitive complexity (extract handshake context,
pass the right secrets around to calculate a secret and build a cipher),
create a new type that tracks the hash type, the handshake context and
intermediate secrets.
Advantages: facilitates reuse between client and server code, makes it
easier to update labels for draft-19, makes it easier to add central
KeyLogWriter functionality.
It was causing mysterious fuzzing failure because it affects the
unmarshaling of the secureNegotiationSupported field.
Change-Id: Id396b84eab90a3b22fb6e306b10bdd7e39707012
Reviewed-on: https://go-review.googlesource.com/60912
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@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>
