Commit graph

401 commits

Author SHA1 Message Date
Robert Griesemer
498b1761d8 go/printer, gofmt: tuned table alignment for better results
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
2018-04-04 13:39:34 -07:00
Filippo Valsorda
d8f27b6eac crypto/tls: simplify the Handshake locking strategy
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 <agl@golang.org>
2018-04-03 16:44:55 +00:00
Filippo Valsorda
3ec0b31ffa crypto/tls: parse certificate first in X509KeyPair to get better errors
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 <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-03-27 21:47:54 +00:00
Mike Danese
a6e50819c2 crypto/tls: support keying material export
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 <mikedanese@google.com>
Reviewed-on: https://go-review.googlesource.com/85115
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-03-22 18:48:49 +00:00
Sergey Frolov
c66e042136 Add no-parrot and setClientRandom tests 2018-02-28 17:02:17 -07:00
Kevin Burke
3ecc46ed22 all: use HTTPS for iana.org links
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 <bradfitz@golang.org>
2018-02-13 18:36:48 +00:00
Kevin Burke
a94b12d7a8 crypto/tls: handle errors in generate_cert.go
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 <bradfitz@golang.org>
2018-02-13 18:14:27 +00:00
Brad Burch
6489cd90fc crypto/tls: optional "certificate_status" with OCSP
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 <agl@golang.org>
2018-01-04 21:32:08 +00:00
Filippo Valsorda
f940be3592 crypto/tls: document VerifyPeerCertificate behavior in relation to ClientAuth
Change-Id: I3ff478912a5a178492d544d2f4ee9cc7570d9acc
Reviewed-on: https://go-review.googlesource.com/84475
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-12-31 22:37:29 +00:00
sergeyfrolov
3aa7191ad5
Fix Travis CI 2017-11-14 13:09:33 -07:00
Sergey Frolov
2a22b8f813 Merge branch 'master' of https://github.com/refraction-networking/utls 2017-11-14 11:38:58 -07:00
Sergey Frolov
c98df6b261 Add Firefox 56 and Chrome 62 2017-11-14 11:36:09 -07:00
filewalkwithme
3fe5088752 crypto/tls: limit number of consecutive warning alerts
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>
2017-11-08 23:18:52 +00:00
Filippo Valsorda
ca44103d11 crypto/tls: advertise support for SHA-512 signatures in 1.2
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>
2017-11-08 22:39:36 +00:00
Carl Mastrangelo
90f8fc90bd archive/zip, crypto/tls: use rand.Read instead of casting ints to bytes
Makes tests run ~1ms faster.

Change-Id: Ida509952469540280996d2bd9266724829e53c91
Reviewed-on: https://go-review.googlesource.com/47359
Reviewed-by: Filippo Valsorda <hi@filippo.io>
Run-TryBot: Filippo Valsorda <hi@filippo.io>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-11-01 05:51:30 +00:00
Joe Kyo
fd917e14e0 crypto/tls: remove bookkeeping code from pHash function
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>
2017-10-31 19:11:03 +00:00
Peter Wu
b3caca679b crypto/tls: replace signatureAndHash by SignatureScheme.
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>
2017-10-13 23:25:03 +00:00
Peter Wu
e3522a12ad crypto/tls: fix first byte test for 255 CBC padding bytes
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>
2017-10-06 18:07:04 +00:00
Marvin Stenger
53c5404836 all: revert "all: prefer strings.LastIndexByte over strings.LastIndex"
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 <iant@golang.org>
2017-10-05 23:19:42 +00:00
Marvin Stenger
61ea6f3dce all: prefer bytes.IndexByte over bytes.Index
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>
2017-09-27 01:09:13 +00:00
Marvin Stenger
6e1bd19dc7 all: prefer strings.LastIndexByte over strings.LastIndex
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>
2017-09-27 00:54:24 +00:00
Kunpei Sakai
fca29fbd02 all: fix article typos
a -> an

Change-Id: I7362bdc199e83073a712be657f5d9ba16df3077e
Reviewed-on: https://go-review.googlesource.com/63850
Reviewed-by: Rob Pike <r@golang.org>
2017-09-15 02:39:16 +00:00
sergeyfrolov
4a02beebd9 Update README 2017-09-12 21:05:37 -06:00
EKR
c00813cb77 crypto/tls: fix clientHelloMsg fuzzer not to generate the RI SCSV
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>
2017-09-09 19:24:12 +00:00
Filippo Valsorda
ff8dfbaa19 crypto/tls: fix and expand TestVerifyPeerCertificate and TestGetClientCertificate
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>
2017-09-09 19:23:45 +00:00
sergeyfrolov
8d58a32e44 README: add build status 2017-09-08 19:26:55 -06:00
Sergey Frolov
dea429d0f0 Add Travis CI integration 2017-09-08 15:59:13 -06:00
Sergey Frolov
3b53b610a6 go vet fixes 2017-09-08 15:59:05 -06:00
sergeyfrolov
f6ecfa941c Update README 2017-09-08 12:53:50 -06:00
sergeyfrolov
048fc50c2d Merge pull request #2 from refraction-networking/getoutkeystream
Implement GetOutKeystream()
2017-09-07 11:40:08 -06:00
Sergey Frolov
08a039c453 Implement GetOutKeystream() 2017-09-07 09:03:04 -06:00
Sergey Frolov
004572df2e Make ApplyConfig() and MarshalClientHello() public 2017-09-06 19:26:38 -06:00
Sergey Frolov
5893031118 RandomizedHello: fix cipherSuite' shuffling 2017-09-06 19:26:16 -06:00
Benjamin M. Schwartz
21fd80b8b6 Merge pull request #11 from Jigsaw-Code/readme-update
Update README.md
2017-09-05 23:35:13 +02:00
Justin Henck
769571a451 Update README.md 2017-09-05 16:36:58 -04:00
Artyom Pervukhin
fe92b81ca1 crypto/tls: fix docstring of Config.ClientSessionCache
Closes #21519

Change-Id: I1247e9435de93aae7e4db2b6e8e5be1b010c296b
Reviewed-on: https://go-review.googlesource.com/56832
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Adam Langley <agl@golang.org>
2017-08-25 22:37:26 +00:00
Benjamin M. Schwartz
a6b7d98736 Merge pull request #10 from Jigsaw-Code/utls-write
Add utls.Write(). Fixes #3
2017-08-18 20:11:18 +02:00
Sergey Frolov
853deec862 Add utls.Write(). Fixes #3 2017-08-18 14:08:51 -04:00
Sergey Frolov
b36d51a64c Merge branch 'master' of github.com:Jigsaw-Code/utls 2017-08-18 14:06:07 -04:00
Sergey Frolov
e797c80048 Update README 2017-08-18 14:05:48 -04:00
sergeyfrolov
a83ace6117 Merge pull request #8 from Jigsaw-Code/fix-races
Fixes #5
2017-08-16 17:22:35 -04:00
Sergey Frolov
fd96e317e6 Fixes #5
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.
2017-08-16 16:12:27 -04:00
Sergey Frolov
09f732cdd6 Update and add new tests 2017-08-16 14:34:06 -04:00
Sergey Frolov
a07d9bc87d Added global cipherSuites are disabled by default
Makes sure that old cipher suite ids, added to global cipherSuites,
are now disabled by default, and, thus, do not affect default Golang
handshake.
2017-08-16 11:56:58 -04:00
Sergey Frolov
eb3c8be9a1 Revert "Remove appendToGlobalCipherSuites"
This reverts commit 0af497570e.
2017-08-16 11:55:39 -04:00
Sergey Frolov
ab61409e7d Add ems 2017-08-16 11:54:20 -04:00
Filippo Valsorda
f3b1bbce00 crypto/tls: disallow handshake messages fragmented across CCS
Detected by BoGo test FragmentAcrossChangeCipherSpec-Server-Packed.

Change-Id: I9a76697b9cdeb010642766041971de5c7e533481
Reviewed-on: https://go-review.googlesource.com/48811
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
2017-08-15 18:45:06 +00:00
Filippo Valsorda
b1d5de15f4 crypto/tls: add BenchmarkHandshakeServer
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 <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
2017-08-15 18:44:38 +00:00
Andreas Auernhammer
a3bb151457 crypto/tls: don't check whether an ec point is on a curve twice
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 <agl@golang.org>
Reviewed-by: Avelino <t@avelino.xxx>
Run-TryBot: Adam Langley <agl@golang.org>
2017-08-15 18:44:20 +00:00
Sergey Frolov
6890f3e435 Finish Firefox 55 fignerprint. Implements #4 2017-08-15 12:50:40 -04:00