testPool currently does the old-style busy loop to wait until cleanups
have executed. Clean this up by using the linkname'd
blockUntilCleanupQueueEmpty.
For #73642.
Change-Id: Ie0c2614db858a984f25b33a805dc52948069eb52
Reviewed-on: https://go-review.googlesource.com/c/go/+/671675
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change makes it so that cleanup goroutines, in race mode, create a
fake race context and switch to it, emulating cleanups running on new
goroutines. This helps in catching races between cleanups that might run
concurrently.
Change-Id: I4c4e33054313798d4ac4e5d91ff2487ea3eb4b16
Reviewed-on: https://go-review.googlesource.com/c/go/+/652635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
On every arch except amd64, it is faster to do x&(x-1) than x^(1<<n).
Most archs need 3 instructions for the latter: MOV $1, R; SLL n, R;
ANDN R, x. Maybe 4 if there's no ANDN.
Most archs need only 2 instructions to do x&(x-1). It takes 3 on
x86/amd64 because NEG only works in place.
Only amd64 can do x^(1<<n) in a single instruction.
(We could on 386 also, but that's currently not implemented.)
Change-Id: I3b74b7a466ab972b20a25dbb21b572baf95c3467
Reviewed-on: https://go-review.googlesource.com/c/go/+/672956
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Unhidden filenames with forbidden characters in subdirectories now
correctly fail the build instead of silently being skipped.
Previously this behavior would only trigger on files in the root of
the embedded directory.
Fixes#54003
Change-Id: I52967d90d6b7929e4ae474b78d3f88732bdd3ac4
Reviewed-on: https://go-review.googlesource.com/c/go/+/670615
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes#69994
Change-Id: I2a23e5998b7421fd5ae0fdb68303d3244361b341
Reviewed-on: https://go-review.googlesource.com/c/go/+/671635
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
To understand this change, we begin with a short description of the UIR
file format.
Every file is a header followed by a series of sections. Each section
has a kind, which determines the type of elements it contains. An
element is just a collection of one or more primitives, as defined by
package pkgbits.
Strings have their own section. Elements in the string section contain
only string primitives. To use a string, elements in other sections
encode a reference to the string section.
To illustrate, consider a simple file which exports nothing at all.
package p
In the meta section, there is an element representing a package stub.
In that package stub, a string ("p") represents both the path and name
of the package. Again, these are encoded as references.
To manage references, every element begins with a reference table.
Instead of writing the bytes for "p" directly, the package stub encodes
an index in this reference table. At that index, a pair of numbers is
stored, indicating:
1. which section
2. which element index within the section
Effectively, elements always use *2* layers of indirection; first to the
reference table, then to the bytes themselves.
With some minor hand-waving, an encoding for the above package is given
below, with (S)ections, (E)lements and (P)rimitives denoted.
+ Header
| + Section Ends // each section has 1 element
| | + 1 // String is elements [0, 1)
| | + 2 // Meta is elements [1, 2)
| + Element Ends
| | + 1 // "p" is bytes [0, 1)
| | + 6 // stub is bytes [1, 6)
+ Payload
| + (S) String
| | + (E) String
| | | + (P) String { byte } 0x70 // "p"
| + (S) Meta
| | + (E) Package Stub
| | | + Reference Table
| | | | + (P) Entry Count uvarint 1 // there is a single entry
| | | | + (P) 0th Section uvarint 0 // to String, 0th section
| | | | + (P) 0th Index uvarint 0 // to 0th element in String
| | | + Internals
| | | | + (P) Path uvarint 0 // 0th entry in table
| | | | + (P) Name uvarint 0 // 0th entry in table
Note that string elements do not have reference tables like other
elements. They behave more like a primitive.
As this is a bit complicated and getting into details of the UIR file
format, we omit some details in the documentation here. The structure
will become clearer as we continue documenting.
Change-Id: I12a5ce9a34251c5358a20f2f2c4d0f9bd497f4d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/671997
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Match standard Unix behavior: Symlinks are not followed when
O_CREATE|O_EXCL is passed to open.
Thanks to Junyoung Park and Dong-uk Kim of KAIST Hacking Lab
for discovering this issue.
Fixes#73702
Fixes CVE-2025-0913
Change-Id: Ieb46a6780c5e9a6090b09cd34290f04a8e3b0ca5
Reviewed-on: https://go-review.googlesource.com/c/go/+/672396
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
The change fixes `linkOrCopy` to work on systems wihtout symlinks,
when copying directories. This was originally noticed on Windows
systems when the user did not have admin privs.
Fixes#73692
Change-Id: I8ca66d65e99433ad38e70314abfabafd43794b79
Reviewed-on: https://go-review.googlesource.com/c/go/+/672275
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Because freebsd is now enabling la57 by default.
Fixes#49405
Change-Id: I30f7bac8b8a9baa85e0c097e06072c19ad474e5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/670715
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In order to make it easier to write in assembly and to be consistent
with the usage of general instructions, a new assembly format is
added for the instructions VANDV and VANDB.
It also works for instructions XVAND{V,B}, [X]V{OR,XOR,NOR,ANDN,ORN}V
and [X]V{OR,XOR,NOR}B.
Change-Id: Ia75d607ac918950e58840ec627aaf0be45d837fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/671316
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Output is a method on T, B and F. It provides an io.Writer that writes
to the same test output stream as TB.Log. The new output writer is
used to refactor the implementation of Log. It maintains the formatting
provided by Log while making call site information optional.
Additionally, it provides buffering of log messages. This fixes and
expands on
https://go-review.googlesource.com/c/go/+/646956.
For #59928.
Change-Id: I08179c35a681f601cf125c0f4aeb648bc10c7a9f
GitHub-Last-Rev: e6e202793c
GitHub-Pull-Request: golang/go#73703
Reviewed-on: https://go-review.googlesource.com/c/go/+/672395
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Jonathan Amsterdam <jba@google.com>
Replace the use of SetFinalizer with AddCleanup.
For #70907
Change-Id: I0cb2c2985eb9285e5f92be9dbcb9d77acc0f59c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/671441
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Replace a usage of runtime.SetFinalizer with runtime.AddCleanup in
the TestCallReturnsEmpty test. There is an additional use of
SetFinalizer in the reflect package which depends on object
resurrection and needs further refactoring to replace.
Updates #70907
Change-Id: I4c0e56c35745a225776bd611d026945efdaf96f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/667595
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This reverts commit 8d189f188e.
Reason for revert: failing test
Change-Id: I951087eaef7818697acf87e3206003bcc8a81ee2
Reviewed-on: https://go-review.googlesource.com/c/go/+/672335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Disabling key usage validation (by passing ExtKeyUsageAny)
unintentionally disabled policy validation. This change decouples these
two checks, preventing the user from unintentionally disabling policy
validation.
Thanks to Krzysztof Skrzętnicki (@Tener) of Teleport for reporting this
issue.
Fixes#73612
Fixes CVE-2025-22874
Change-Id: Iec8f080a8879a3dd44cb3da30352fa3e7f539d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/670375
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Stapleton Cordasco <graffatcolmingov@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Git's supported SHA 256 object hashes since 2.29[1] in 2021, and Gitlab
now has experimental support for sha256 repos.
Take rsc@'s suggestion of checking the of the length of the hashes from
git ls-remote to determine whether a git repo is using sha256 hashes and
decide whether to pass --object-format=sha256 to git init.
Unfortunately, just passing --object-format=sha256 wasn't quite enough,
though. We also need to decide whether the hash-length is 64 hex bytes
or 40 hex bytes when resolving refs to decide whether we've been passed
a full commit-hash. To that end, we use
git config extensions.objectformat to decide whether the (now guaranteed
local) repo is using sha256 hashes and hence 64-hex-byte strings.
[1]: lost experimental status in 2.42 from Aug 2023
(https://lore.kernel.org/git/xmqqr0nwp8mv.fsf@gitster.g/)
For: #68359
Change-Id: I47f480ab8334128c5d17570fe76722367d0d8ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/636475
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: David Finkel <david.finkel@gmail.com>
It supports features described in the issue:
* add -json flag for 'go version -m' to print json encoding of
runtime/debug.BuildSetting to standard output.
* report an error when specifying -json flag without -m.
* print build settings on seperated line for each binary
Fixes#69712
Change-Id: I79cba2109f80f7459252d197a74959694c4eea1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/619955
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Output is a method on T, B and F. It provides an io.Writer that writes
to the same test output stream as TB.Log. The new output writer is
used to refactor the implementation of Log. It maintains the formatting
provided by Log while making call site information optional.
Additionally, it provides buffering of log messages.
Co-authored-by: Aleks Fazlieva <britishrum@users.noreply.github.com>
Fixes#59928.
Change-Id: I29090b3d4f61f7334388b373ec18750d5637aafa
GitHub-Last-Rev: 18af0e1526
GitHub-Pull-Request: golang/go#71575
Reviewed-on: https://go-review.googlesource.com/c/go/+/646956
Reviewed-by: Arati <artichaut2023@gmail.com>
Auto-Submit: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Specified in RISC-V ELF psABI[1], mapping symbols are symbols starting
with "$d" or "$x" with STT_NOTYPE, STB_LOCAL and zero sizes, indicating
boundaries between code and data in the same section.
Let's simply ignore them as they're only markers instead of real symbols.
This fixes linking errors like
sym#63 ("$d"): ignoring symbol in section 4 (".riscv.attributes") (type 0)
when using CGO together with Clang and internal linker, which are caused
by unnecessary (but technically correct) mapping symbols created by LLVM
for various sections.
[1]: 87aecf6017/riscv-elf.adoc?plain=1#L1448
Fixes#73516
Change-Id: I02ca90c100ba8a38733fe3b8b8403836b44a3dd1
GitHub-Last-Rev: d7842ceafb
GitHub-Pull-Request: golang/go#73592
Reviewed-on: https://go-review.googlesource.com/c/go/+/669675
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This change reintroduces CL 564197. It was reverted due to a failing
benchmark. That failure has been resolved.
For #65064
Change-Id: Ic88841d2bc24c2717ad324873f0f52699f21dc66
Reviewed-on: https://go-review.googlesource.com/c/go/+/669235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This is a basic refactoring. This enumeration refers primarily to
the different sections of a UIR file, so this naming is a bit more
direct.
Change-Id: Ib70ab054e97effaabc035450d246ae4354da8075
Reviewed-on: https://go-review.googlesource.com/c/go/+/671935
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Mark Freeman <mark@golang.org>
This complements the grammar being developed in package noder. It
is unclear how to discuss references in their current state, as
they require knowledge of sections, elements, etc.
Perhaps the references here should refer to indices on the byte
array. This would allow a stronger separation of pkgbits and noder.
Change-Id: Ic0e5ac9c07f0a0b92d6ffd4d4e26dbe5dcf89e57
Reviewed-on: https://go-review.googlesource.com/c/go/+/671440
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Auto-Submit: Mark Freeman <mark@golang.org>
If the context expires before acquireThread(ctx) succeeds, then the goroutine can block like:
net.lookupProtocol.func1()
src/net/lookup_windows.go:58 +0x105
created by net.lookupProtocol in goroutine 2834
src/net/lookup_windows.go:56 +0xda
We saw this in our UTs with a leak detector, confirmed by inspection of the source code.
Change-Id: I9b927f0345a2fa7336b23d95c506a8a0976e28d0
GitHub-Last-Rev: 27af7477a9
GitHub-Pull-Request: golang/go#73364
Reviewed-on: https://go-review.googlesource.com/c/go/+/664956
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Zxilly Chou <zhouxinyu1001@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
NewFile was recently updated (in CL 668195) to detect whether the
handle is a socket or not. This special case is not really necessary,
given that socket handles can be used as if they were normal file
handles on all functions supported by os.File (see https://learn.microsoft.com/en-us/windows/win32/winsock/socket-handles-2).
Not only is not necessary, but is can also be problematic, as there is
no way to reliably detect whether a handle is a socket or not. For
example, the test failure reported in #73630 is caused by a named pipe
wrongly detected as a socket.
This aligns with the Unix NewFile behavior of returning an os.File that
identifies itself as a file handle even if it is a socket. This makes
os.File.Close to always return os.ErrClosed in case of multiple calls
rather than sometimes returning "use of closed network connection".
Updates #10350.
Fixes#73630.
Change-Id: Ia8329783d5c8ef6dac34ef69ed1ce9d2a9862e11
Reviewed-on: https://go-review.googlesource.com/c/go/+/671455
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Windows prefers 64-bit binaries to be loaded at an address above 4GB.
Having a preferred base address below this boundary triggers a
compatibility mode in Address Space Layout Randomization (ASLR) on
recent versions of Windows that reduces the number of locations to which
ASLR may relocate the binary.
The Go internal linker was using a smaller base address due to an issue
with how dynamic cgo symbols were relocated, which has been fixed in
this CL.
Fixes#73561.
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest
Change-Id: Ia8cb35d57d921d9be706a8975fa085af7996f124
Reviewed-on: https://go-review.googlesource.com/c/go/+/671515
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
A sentinel error http.ErrServerClosed is returned after Server.Shutdown
and Server.Close but it is not documented on the Server.Shutdown while
other methods such as Server.Serve are documented on it.
Change-Id: Id82886d9d6a1474a514d62e9169b35f3579a9eee
Reviewed-on: https://go-review.googlesource.com/c/go/+/671695
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
gcMarkTermination() ensures that all caches are flushed before continuing the GC cycle, thus preempting all goroutines.
However, inlining calls to lock() in bgsweep makes it non-preemptible for most of the time, leading to livelock.
This change adds explicit preemption to avoid this.
Fixes#73499.
Change-Id: I4abf0d658f3d7a03ad588469cd013a0639de0c8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/668795
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
The old parameter name sep was probably copied from ExampleCut. Change
the parameter names to prefix and suffix, respectivly to make the
examples a bit more readable.
Change-Id: Ie14b0050c2fafe3301c5368efd548a1629a7545f
Reviewed-on: https://go-review.googlesource.com/c/go/+/670955
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Robert Griesemer <gri@google.com>
The first loop leaves the lengths of the two arguments unchanged.
Take advantage of this invariant in the loop's condition. Here are some
benchmark results (no change to allocations):
goos: darwin
goarch: amd64
pkg: strings
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
│ old │ new │
│ sec/op │ sec/op vs base │
EqualFold/Tests-8 240.0n ± 4% 245.1n ± 5% ~ (p=0.516 n=20)
EqualFold/ASCII-8 11.50n ± 1% 11.04n ± 0% -3.96% (p=0.000 n=20)
EqualFold/UnicodePrefix-8 102.1n ± 0% 102.2n ± 0% ~ (p=0.455 n=20)
EqualFold/UnicodeSuffix-8 90.14n ± 0% 89.80n ± 1% ~ (p=0.113 n=20)
geomean 71.00n 70.60n -0.56%
Change-Id: I1f6d1df8a0398f9493692f59d7369c3f0fbba436
GitHub-Last-Rev: 9508ee26ad
GitHub-Pull-Request: golang/go#73672
Reviewed-on: https://go-review.googlesource.com/c/go/+/671756
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
If the client hello legacy version is >= TLS 1.3, and no
supported_versions extension is sent, negotiate TLS 1.2 or lower when
supported.
On the topic of supported version negotiation RFC 8446 4.2.1 indicates
TLS 1.3 implementations MUST send a supported_versions extension with
a list of their supported protocol versions. The crypto/tls package
enforces this when the client hello legacy version indicates TLS 1.3
(0x0304), aborting the handshake with an alertMissingExtension alert if
no supported_versions were received.
However, section 4.2.1 indicates different behaviour should be used when
the extension is not present and TLS 1.2 or prior are supported:
If this extension is not present, servers which are compliant with
this specification and which also support TLS 1.2 MUST negotiate
TLS 1.2 or prior as specified in [RFC5246], even if
ClientHello.legacy_version is 0x0304 or later.
This commit updates the client hello processing logic to allow this
behaviour. If no supported_versions extension was received we ignore the
legacy version being >= TLS 1.3 and instead negotiate a lower supported
version if the server configuration allows.
This fix in turn allows enabling the BoGo ClientHelloVersionTooHigh,
MinorVersionTolerance, and MajorVersionTolerance tests.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/671235
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously for protocol versions older than TLS 1.3 our server handshake
implementation sent an alertBadCertificate alert in the case where the
server TLS config indicates a client cert is required and none was
received.
This commit updates the relevant logic to instead send
alertHandshakeFailure in these circumstances.
For TLS 1.2, RFC 5246 §7.4.6 unambiguously describes this as the correct
alert:
If the client does not send any certificates, the
server MAY at its discretion either continue the handshake without
client authentication, or respond with a fatal handshake_failure
alert.
The TLS 1.1 and 1.0 specs also describe using this alert (RFC 4346 §7.4.6
and RFC 2246 §7.4.6) both say:
If client authentication is required by the server for the handshake
to continue, it may respond with a fatal handshake failure alert.
Making this correction also allows enabling the
RequireAnyClientCertificate-TLS1* bogo tests.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/671195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Previously a handful of large record tests were in the bogo config
ignore list. The ignored tests were failing because they used
insecure ciphersuites that aren't enabled by default.
This commit adds the non-default insecure ciphersuites to the bogo
TLS configuration and re-enables the tests. Doing this uncovered
a handful of unrelated tests that needed to be fixed, each handled
before this commit.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/669158
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
These two bogo tests mutate the version number used for the premaster
secret calculation for a client RSA key exchange, with the expectation
the server rejects the handshake.
Per the comment in the end of rsaKeyAgreement.processClientKeyExchange
we explicitly choose *not* to verify the version number.
This commit adds the two version number tests to the ignore list. They
coincidentally happen to produced the expected failure because they use
a non-default ciphersuite. When we add this ciphersuite to the client
config for the bogo test they will start to fail unless ignored.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/669175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
If a client or server explicitly offers point formats, and the point
formats don't include the uncompressed format, then error. This matches
BoringSSL and Rustls behaviour and allows enabling the
PointFormat-Client-MissingUncompressed bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/669157
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Unlike in earlier TLS versions, in TLS 1.3 when processing a server
hello the legacy_compression_method MUST have the value 0. It is no
longer a parameter that offers a choice of compression method.
With this in mind, it seems more appropriate to return a decode error
when we encounter a non-zero compression method in a server hello
message. We haven't found a parameter value we reject, we've found
a message that doesn't decode according to its specification.
Making this change also aligns with BoringSSL and allows enabling the
TLS13-HRR-InvalidCompressionMethod bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/669156
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Previously if the clientHandshakeState for the TLS 1.2 client code
encountered a server helo message that contained a compression method
other than compressionNone, we would emit an unexpected message alert.
Instead, it seems more appropriate to return an illegal parameter alert.
The server hello message _was_ expected, it just contained a bad
parameter option.
Making this change also allows enabling the InvalidCompressionMethod
bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/669155
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek
Keith Randall
Xiaolin Zhao
Julian Zhu
kmvijay
Ian Alexander
qiulaidongfeng
Mark Freeman
Damien Neil
Julien Cretel
limeidan
suntala
Carlos Amedee
Jonathan Amsterdam
Roland Shoemaker
David Finkel
Sean Liao
xieyuschen
Radu Berinde
Yao Zi
Guoqi Chen
Spike Curtis
qmuntal
t-katsumura
ArsenySamoylov
Tobias Klauser
Daniel McCarney