Merge upstream crypto/tls #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The processClientKeyExchange and processServerKeyExchange functions unmarshal an encoded EC point and explicitly check whether the point is on the curve. The explicit check can be omitted because elliptic.Unmarshal fails if the point is not on the curve and the returned error would always be the same. Fixes #20496 Change-Id: I5231a655eace79acee2737dd036a0c255ed42dbb Reviewed-on: https://go-review.googlesource.com/44311 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: Avelino <[email protected]> Run-TryBot: Adam Langley <[email protected]>
name time/op HandshakeServer/RSA-4 1.10ms ± 0% HandshakeServer/ECDHE-P256-RSA-4 1.23ms ± 1% HandshakeServer/ECDHE-P256-ECDSA-P256-4 178µs ± 1% HandshakeServer/ECDHE-X25519-ECDSA-P256-4 180µs ± 2% HandshakeServer/ECDHE-P521-ECDSA-P521-4 19.8ms ± 1% Change-Id: I6b2c79392995d259cfdfc5199be44cc7cc40e155 Reviewed-on: https://go-review.googlesource.com/44730 Reviewed-by: Adam Langley <[email protected]> Run-TryBot: Adam Langley <[email protected]>
Detected by BoGo test FragmentAcrossChangeCipherSpec-Server-Packed. Change-Id: I9a76697b9cdeb010642766041971de5c7e533481 Reviewed-on: https://go-review.googlesource.com/48811 Reviewed-by: Adam Langley <[email protected]> Run-TryBot: Adam Langley <[email protected]>
Closes #21519 Change-Id: I1247e9435de93aae7e4db2b6e8e5be1b010c296b Reviewed-on: https://go-review.googlesource.com/56832 Reviewed-by: Avelino <[email protected]> Reviewed-by: Adam Langley <[email protected]>
…tCertificate 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 <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]>
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]>
a -> an Change-Id: I7362bdc199e83073a712be657f5d9ba16df3077e Reviewed-on: https://go-review.googlesource.com/63850 Reviewed-by: Rob Pike <[email protected]>
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
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 <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
This reverts https://golang.org/cl/66372. Updates #22148 Change-Id: I3e94af3dfc11a2883bf28e1d5e1f32f98760b3ee Reviewed-on: https://go-review.googlesource.com/68431 Reviewed-by: Ian Lance Taylor <[email protected]>
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 <[email protected]> Run-TryBot: Adam Langley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
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 <[email protected]>
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 <[email protected]> Run-TryBot: Filippo Valsorda <[email protected]> Run-TryBot: Tobias Klauser <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
Makes tests run ~1ms faster. Change-Id: Ida509952469540280996d2bd9266724829e53c91 Reviewed-on: https://go-review.googlesource.com/47359 Reviewed-by: Filippo Valsorda <[email protected]> Run-TryBot: Filippo Valsorda <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
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 <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> Run-TryBot: Adam Langley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
…ientAuth Change-Id: I3ff478912a5a178492d544d2f4ee9cc7570d9acc Reviewed-on: https://go-review.googlesource.com/84475 Reviewed-by: Filippo Valsorda <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
Follows the wording in RFC4366 more precisely which allows a server to optionally return a "certificate_status" when responding to a client hello containing "status_request" extension. fixes #8549 Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7 Reviewed-on: https://go-review.googlesource.com/86115 Reviewed-by: Adam Langley <[email protected]>
I don't expect these to hit often, but we should still alert users if we fail to write the correct data to the file, or fail to close it. Change-Id: I33774e94108f7f18ed655ade8cca229b1993d4d2 Reviewed-on: https://go-review.googlesource.com/91456 Reviewed-by: Brad Fitzpatrick <[email protected]>
iana.org, www.iana.org and data.iana.org all present a valid TLS certificate, so let's use it when fetching data or linking to resources to avoid errors in transit. Change-Id: Ib3ce7c19789c4e9d982a776b61d8380ddc63194d Reviewed-on: https://go-review.googlesource.com/89416 Reviewed-by: Brad Fitzpatrick <[email protected]>
This change implement keying material export as described in: https://tools.ietf.org/html/rfc5705 I verified the implementation against openssl s_client and openssl s_server. Change-Id: I4dcdd2fb929c63ab4e92054616beab6dae7b1c55 Signed-off-by: Mike Danese <[email protected]> Reviewed-on: https://go-review.googlesource.com/85115 Run-TryBot: Adam Langley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]>
parsePrivateKey can't return useful error messages because it does trial decoding of multiple formats. Try ParseCertificate first in case it offers a useful error message. Fixes #23591 Change-Id: I380490a5850bee593a7d2f584a27b2a14153d768 Reviewed-on: https://go-review.googlesource.com/90435 Run-TryBot: Filippo Valsorda <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]>
If in.Mutex is never locked by Handshake when c.handshakeComplete is true, and since c.handshakeComplete is unset and then set back by handleRenegotiation all under both in.Mutex and handshakeMutex, we can significantly simplify the locking strategy by removing the sync.Cond. See also https://groups.google.com/forum/#!topic/golang-dev/Xxiai-R_jH0 and a more complete analysis at https://go-review.googlesource.com/c/go/+/33776#message-223a3ccc819f7015cc773d214c65bad70de5dfd7 Change-Id: I6052695ece9aff9e3112c2fb176596fde8aa9cb2 Reviewed-on: https://go-review.googlesource.com/33776 Reviewed-by: Adam Langley <[email protected]>
The go/printer (and thus gofmt) uses a heuristic to determine whether to break alignment between elements of an expression list which is spread across multiple lines. The heuristic only kicked in if the entry sizes (character length) was above a certain threshold (20) and the ratio between the previous and current entry size was above a certain value (4). This heuristic worked reasonably most of the time, but also led to unfortunate breaks in many cases where a single entry was suddenly much smaller (or larger) then the previous one. The behavior of gofmt was sufficiently mysterious in some of these situations that many issues were filed against it. The simplest solution to address this problem is to remove the heuristic altogether and have a programmer introduce empty lines to force different alignments if it improves readability. The problem with that approach is that the places where it really matters, very long tables with many (hundreds, or more) entries, may be machine-generated and not "post-processed" by a human (e.g., unicode/utf8/tables.go). If a single one of those entries is overlong, the result would be that the alignment would force all comments or values in key:value pairs to be adjusted to that overlong value, making the table hard to read (e.g., that entry may not even be visible on screen and all other entries seem spaced out too wide). Instead, we opted for a slightly improved heuristic that behaves much better for "normal", human-written code. 1) The threshold is increased from 20 to 40. This disables the heuristic for many common cases yet even if the alignment is not "ideal", 40 is not that many characters per line with todays screens, making it very likely that the entire line remains "visible" in an editor. 2) Changed the heuristic to not simply look at the size ratio between current and previous line, but instead considering the geometric mean of the sizes of the previous (aligned) lines. This emphasizes the "overall picture" of the previous lines, rather than a single one (which might be an outlier). 3) Changed the ratio from 4 to 2.5. Now that we ignore sizes below 40, a ratio of 4 would mean that a new entry would have to be 4 times bigger (160) or smaller (10) before alignment would be broken. A ratio of 2.5 seems more sensible. Applied updated gofmt to all of src and misc. Also tested against several former issues that complained about this and verified that the output for the given examples is satisfactory (added respective test cases). Some of the files changed because they were not gofmt-ed in the first place. For #644. For #7335. For #10392. (and probably more related issues) Fixes #22852. Change-Id: I5e48b3d3b157a5cf2d649833b7297b33f43a6f6e
I was confused about how to start an HTTP server if the server cert/key are in memory, not on disk. I thought it would be good to show an example of how to use these two functions to accomplish that. example-cert.pem and example-key.pem were generated using crypto/tls/generate_cert.go. Change-Id: I850e1282fb1c38aff8bd9aeb51988d21fe307584 Reviewed-on: https://go-review.googlesource.com/72252 Reviewed-by: Brad Fitzpatrick <[email protected]>
…of cipherhw When the internal/cpu package was introduced, the AES package still used the custom crypto/internal/cipherhw package for amd64 and s390x. This change removes that package entirely in favor of directly referencing the cpu feature flags set and exposed by the internal/cpu package. In addition, 5 new flags have been added to the internal/cpu s390x struct for detecting various cipher message (KM) features. Change-Id: I77cdd8bc1b04ab0e483b21bf1879b5801a4ba5f4 GitHub-Last-Rev: a611e3ecb1f480dcbfce3cb0c8c9e4058f56c1a4 GitHub-Pull-Request: golang/go#24766 Reviewed-on: https://go-review.googlesource.com/105695 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
Each URL was manually verified to ensure it did not serve up incorrect content. Change-Id: I4dc846227af95a73ee9a3074d0c379ff0fa955df Reviewed-on: https://go-review.googlesource.com/115798 Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]>
Users are sometimes confused why session tickets are not enabled even if SessionTicketsDisabled is false. Change-Id: I3b783d2cf3eed693a3ad6acb40a8003db7e0b648 Reviewed-on: https://go-review.googlesource.com/117255 Reviewed-by: Adam Langley <[email protected]>
…implementations
Hardware AES support in Go on s390x currently requires ECB, CBC
and CTR modes be available. It also requires that either the
GHASH or GCM facilities are available. The existing checks missed
some of these constraints.
While we're here simplify the cpu package on s390x, moving masking
code out of assembly and into Go code. Also, update SHA-{1,256,512}
implementations to use the cpu package since that is now trivial.
Finally I also added a test for internal/cpu on s390x which loads
/proc/cpuinfo and checks it against the flags set by internal/cpu.
Updates #25822 for changes to vet whitelist.
Change-Id: Iac4183f571643209e027f730989c60a811c928eb
Reviewed-on: https://go-review.googlesource.com/114397
Run-TryBot: Michael Munday <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
I tested all fingerprints and confirmed that Chrome and Firefox are working as intended. Android fingerprints were grossly unpopular, which could a result of incorrect merge, but either way we'll remove them for now.
bemasc
approved these changes
Jun 19, 2018
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
This function is not needed anymore, since check for whether ciphersuite is supported is done against per-tls.Config, not against global variable. One of needed changes for fixing data races, see refraction-networking#5
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
The root cause of races is that global variables supportedSignatureAlgorithms and cipherSuites are used both to form handshake and to check whether or not peer responded with supported algorithm. In this patch I create separate variables for this purpose. Updated tests for kicks. Finally, go fmt.
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
…/merge-upstream Merge upstream crypto/tls
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
This function is not needed anymore, since check for whether ciphersuite is supported is done against per-tls.Config, not against global variable. One of needed changes for fixing data races, see refraction-networking#5
adotkhan
pushed a commit
to Psiphon-Labs/utls
that referenced
this pull request
Dec 10, 2024
The root cause of races is that global variables supportedSignatureAlgorithms and cipherSuites are used both to form handshake and to check whether or not peer responded with supported algorithm. In this patch I create separate variables for this purpose. Updated tests for kicks. Finally, go fmt.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While merging I had to do some changes to utls to make it work with new upstream.
I probably shouldn't have updated README and remove Android in that same commit, and now I am not sure how to decouple.
EDIT: did I just ping 16 people? :(