Revise the code added in CL 452763 that skips some of the -race tests
on older windows builders. The old-style skip was doing a log.Printf,
which wound up being interpreted in "-list" mode. Fix is to pass in a
special rtPreFunc when registering the test (thanks Austin for the
fix suggestion).
Updates #56904.
Change-Id: Ia6ea31fb7f011b539173f47357ab3bf7389f256d
Reviewed-on: https://go-review.googlesource.com/c/go/+/452769
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Than McIntosh <thanm@google.com>
This change is being made late in the release cycle.
Disable it by default. Insecure path checks may be enabled by setting
GODEBUG=tarinsecurepath=0 or GODEBUG=zipinsecurepath=0.
We can enable this by default in Go 1.21 after publicizing the change
more broadly and giving users a chance to adapt to the change.
For #55356.
Change-Id: I549298b3c85d6c8c7fd607c41de1073083f79b1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/452616
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
The Go 1.20 code freeze has recently started. This is a time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
This CL updates them with x/build/cmd/updatestd.
For #36905.
Change-Id: Ie0ec91daeb848f00f64686003012297161ad02fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/452766
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Add a skip for the external-linkage part of the race detector test for
elderly versions of windows, until the underlying cause for the problem
can be determined.
Updates #56904.
Change-Id: I3e8650ff66f34efefabcd6bc343d57124539901b
Reviewed-on: https://go-review.googlesource.com/c/go/+/452763
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
This was an oversight from CL 452457 that I noticed while
investigating #56889.
This change essentially undoes CL 335409, which is no longer needed
after CL 450739 because we no longer attempt to use cgo by default
when no C compiler is present.
Updates #47257.
Updates #40042.
Updates #47215.
Change-Id: I29c7ce777a9ec7ba5820dc1d836b12a61b86bc37
Reviewed-on: https://go-review.googlesource.com/c/go/+/452677
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
The gcc toolchain on the aix-ppc64 builder apparently does not achieve
reproducible builds for packages that use cgo, which causes the
binaries in cmd that use package "net" (cmd/go, cmd/pprof, and
cmd/trace) to appear stale whenever the Go build cache is cleared.
For now, we work around the staleness by rebuilding std and simply not
checking whether cmd is stale.
For #56896.
Updates #47257.
Change-Id: I15f86e72dee53904b881710d5d5d613872361510
Reviewed-on: https://go-review.googlesource.com/c/go/+/452680
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ayappan Perumal <ayappanec@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
If the build cache is cleaned (using 'go clean -cache' or similar), or
if a toolchain is freshly installed without warming the cache, the
build cache might not contain the dependencies needed to verify that
cmd/addr2line is not stale. In that case, the test should refill the
cache instead of failing.
Fixes#56889.
Change-Id: Ic6cf13b92bafa9c795e50eb0e4e1a9ae00ee8538
Reviewed-on: https://go-review.googlesource.com/c/go/+/452458
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
VerifyASN1 became directly reachable without encoding/decoding in
CL 353849, so it's now possible for the signature to be empty.
Change-Id: I37d6400945ab541120180bf73335e0ec93322947
Reviewed-on: https://go-review.googlesource.com/c/go/+/452635
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Address the release notes TODO regarding the loopclosure analyzer,
documenting the new warning for parallel subtests.
In doing so, choose a structure for the vet section, opting for h4
headings. In recent years, we have used either h4 headings or simple
paragraphs to document vet changes. This year, I thought it worthwhile
to put the timeformat and loopclosure changes into separate sections.
Also document the improvements to reference capture detection introduced
in CL 452615.
Change-Id: I05886f7025d66bb7f2609f787f69d1a769ca6f5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/450735
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
I accidentally reverted its edits with a bad cherry-pick in CL 452457.
This should re-fix the windows-.*-newcc builders that regressed at
that change.
Updates #47257.
Updates #35006.
Updates #53540.
Change-Id: I5818416af7c4c8c1593c36aa0198331b42b6c7d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/452675
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL adds -d=inlstaticinit to control whether static initialization
of inlined function calls (added in CL 450136) is allowed.
We've needed to fix it once already (CL 451555) and Google-internal
testing is hitting additional failure cases, so putting this
optimization behind a feature flag seems appropriate regardless.
Also, while we diagnose and fix the remaining cases, this CL also
disables the optimization to avoid miscompilations.
Updates #56894.
Change-Id: If52a358ad1e9d6aad1c74fac5a81ff9cfa5a3793
Reviewed-on: https://go-review.googlesource.com/c/go/+/452676
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
To pick up CL 452155.
Done by
go get -d golang.org/x/tools@master
go mod tidy
go mod vendor
Change-Id: I4dba4ba9f17123cfe567704b0d5e5fc16f9ffd83
Reviewed-on: https://go-review.googlesource.com/c/go/+/452615
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
As of CL 450739, we do not need install targets for cgo files when a C
compiler is not present because cgo is not enabled by default.
(Without a C compiler, builds will proceed with cgo disabled.)
Fixes#47257.
Fixes#56888.
Change-Id: I274c50a60b5b1382e291df86a5464da8ad3695a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/452457
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Also document new cgo changes and reorder go command
section to put most important notes first.
Change-Id: I45b0c785bd8bc82c3b174800fc995312ea057f14
Reviewed-on: https://go-review.googlesource.com/c/go/+/452557
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
If GOMAXPROCS=1, force serial execution, which is better for
debugging build problems and also minimizes footprint, if that
happens to matter.
This wasn't good when the bootstrap was 1.4 because there
default GOMAXPROCS=1, but that is no longer the bootstrap
version.
Change-Id: I637e25c8acb4758795fceef63921eda359a7be29
Reviewed-on: https://go-review.googlesource.com/c/go/+/452556
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This leads to better error messages where operations are not
permitted because of empty type sets.
Fixes#51525.
Change-Id: I8d15645e2aff5145e458bdf9aaa4d2bee28d37fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/452535
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This CL changes cmd/compile to reject anonymous interface cycles like:
type I interface { m() interface { I } }
We don't anticipate any users to be affected by this change in
practice. Nonetheless, this CL also adds a `-d=interfacecycles`
compiler flag to suppress the error. And assuming no issue reports
from users, we'll move the check into go/types and types2 instead.
Updates #56103.
Change-Id: I1f1dce2d7aa19fb388312cc020e99cc354afddcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/445598
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Now that we don't install the .a files, the installed
runtime/internal/sys.a no longer exists. Stop trying to access it.
Change-Id: Ic437e7ad95be6109d0b2fcfb81e840f5cb476be8
Reviewed-on: https://go-review.googlesource.com/c/go/+/451360
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
For #54258.
Change-Id: Ib0d326af2719bca1579f84c125f6573f87dce982
Reviewed-on: https://go-review.googlesource.com/c/go/+/452455
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Similar to CL 317869, apply to a newly added directory.
Maybe we should skip all directories starting with "_"?
Updates #46027.
Change-Id: Idcb011fda877c9f2cb3032524bebfcc0c1da70b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/452437
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Replace explicit Go version names where possible with generic reference
to Go bootstrap version.
Updates #44505
Change-Id: I4a6439576efd40e72acd26fcc1472a1a8b0b06e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/344330
Auto-Submit: Martin Möhrmann <martin@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Modify skip rule for TestVectoredHandlerExceptionInNonGoThread to
trigger on both the base builder (windows-amd64-2012) and the newcc
canary builder (windows-amd64-2012-newcc).
Updates #49681.
Change-Id: I58109fc2e861b943cb66be0feec348671be84ab3
Reviewed-on: https://go-review.googlesource.com/c/go/+/452436
Run-TryBot: Than McIntosh <thanm@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This CL adds windows/arm64 to the list of ports that supports PIE
build mode. It is probably an oversight that this port is not marked
as pie-capable because windows/arm64 only supports PIE build mode.
Fixes#56872
Change-Id: I2bdd3ac207280f47ddcf8c2582f13025dafb9278
Reviewed-on: https://go-review.googlesource.com/c/go/+/452415
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Modify skip rule for TestRaiseException to trigger on both the base
builder (windows-amd64-2012) and the newcc canary builder
(windows-amd64-2012-newcc).
Updates #49681.
Change-Id: I132f9ddd102666b68ad04cc661fdcc2cd841051a
Reviewed-on: https://go-review.googlesource.com/c/go/+/451294
Auto-Submit: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
This will let us reuse it in crypto/ecdsa for the NIST scalar fields.
The main change in API is around encoding and decoding. The SetBytes +
ExpandFor sequence was hacky: SetBytes could produce a bigger size than
the modulus if leading zeroes in the top byte overflowed the limb
boundary, so ExpandFor had to check for and tolerate that. Also, the
caller was responsible for checking that the overflow was actually all
zeroes (which we weren't doing, exposing a crasher in decryption and
signature verification) and then for checking that the result was less
than the modulus. Instead, make SetBytes take a modulus and return an
error if the value overflows. Same with Bytes: we were always allocating
based on Size before FillBytes anyway, so now Bytes takes a modulus.
Finally, SetBig was almost only used for moduli, so replaced
NewModulusFromNat and SetBig with NewModulusFromBig.
Moved the constant-time bitLen to math/big.Int.BitLen. It's slower, but
BitLen is primarily used in cryptographic code, so it's safer this way.
Change-Id: Ibaf7f36d80695578cb80484167d82ce1aa83832f
Reviewed-on: https://go-review.googlesource.com/c/go/+/450055
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This is missing a test for Ed25519ph with context, since the RFC doesn't
provide one.
Fixes#31804
Change-Id: I20947374c51c6b22fb2835317d00edf816c9a2d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/404274
Auto-Submit: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Several operations emulate instructions available on power9. Use
the GOPPC64_power9 macro provided by the compiler to select the
native instructions if the minimum cpu requirements are met.
Likewise rework the LXSDX_BE to simplify usage when overriding
it. It is only used in one place.
All three configurations are tested via CI.
On POWER9:
pkg:crypto/cipher goos:linux goarch:ppc64le
AESCBCEncrypt1K 949MB/s ± 0% 957MB/s ± 0% +0.83%
AESCBCDecrypt1K 1.82GB/s ± 0% 1.99GB/s ± 0% +8.93%
pkg:crypto/aes goos:linux goarch:ppc64le
Encrypt 1.01GB/s ± 0% 1.05GB/s ± 0% +4.36%
Decrypt 987MB/s ± 0% 1024MB/s ± 0% +3.77%
Change-Id: I56d0eb845647dd3c43bcad71eb281b499e1d1789
Reviewed-on: https://go-review.googlesource.com/c/go/+/449116
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Auto-Submit: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
This patch reworks the handling of DLL import symbols in the PE host
object loader to ensure that the Go linker can deal with them properly
during internal linking.
Prior to this point the strategy was to immediately treat an import
symbol reference of the form "__imp__XXX" as if it were a reference to
the corresponding DYNIMPORT symbol XXX, except for certain special
cases. This worked for the most part, but ran into problems in
situations where the target ("XXX") wasn't a previously created
DYNIMPORT symbol (and when these problems happened, the root cause was
not always easy to see).
The new strategy is to not do any renaming or forwarding immediately,
but to delay handling until host object loading is complete. At that
point we make a scan through the newly introduced text+data sections
looking at the relocations that target import symbols, forwarding
the references to the corresponding DYNIMPORT sym where appropriate
and where there are direct refs to the DYNIMPORT syms, tagging them
for stub generation later on.
Updates #35006.
Updates #53540.
Change-Id: I2d42b39141ae150a9f82ecc334001749ae8a3b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/451738
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Adapt the testcshared tests to handle the case where the path output
by invoking
gcc -print-prog-name=dlltool
is a path lacking the final ".exe" suffix (this seems to be what clang
is doing); tack it on before using if this is the case.
Updates #35006.
Updates #53540.
Change-Id: I04fb7b9fc90677880b1ced4a4ad2a8867a3f5f86
Reviewed-on: https://go-review.googlesource.com/c/go/+/451816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Add a new debugging flag "-capturehostobjs" that instructs the linker
to capture copies of all object files loaded in during the host object
loading portion of CGO internal linking. The intent is to make it
easier to analyze the objects after the fact (as opposed to having to
dig around inside archives, which can be a "find needle in haystack"
exercise).
Change-Id: I7023a5b72b1b899ea9b3bd6501f069d1f21bbaf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/451737
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
When ctxt.Debugvlog > 1, produce additional trace output to describe
which object files are being pulled out of host archive libraries and
why they were pulled (e.g. which symbol had a reference to something
in a library). Intended to make it easier to debug problems with cgo
internal linking.
Change-Id: Icd64aff244b9145162a00cb51642ef32f26adfba
Reviewed-on: https://go-review.googlesource.com/c/go/+/451736
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This is faster than the current code because computing RR involves
one more shiftIn and using it involves an extra multiplication, but each
exponentiation was doing montgomeryRepresentation twice, once for x and
once for 1, and now they share the RR precomputation.
More importantly, it allows precomputing the value and attaching it to
the private key in a future CL.
name old time/op new time/op delta
DecryptPKCS1v15/2048-8 1.46ms ± 0% 1.40ms ± 7% -3.69% (p=0.003 n=10+9)
DecryptPKCS1v15/3072-8 4.23ms ± 0% 4.13ms ± 4% -2.36% (p=0.004 n=9+9)
DecryptPKCS1v15/4096-8 9.42ms ± 0% 9.08ms ± 3% -3.69% (p=0.000 n=9+10)
EncryptPKCS1v15/2048-8 221µs ± 0% 137µs ± 1% -37.91% (p=0.000 n=9+10)
DecryptOAEP/2048-8 1.46ms ± 0% 1.39ms ± 1% -4.97% (p=0.000 n=9+10)
EncryptOAEP/2048-8 221µs ± 0% 138µs ± 0% -37.71% (p=0.000 n=8+10)
SignPKCS1v15/2048-8 1.68ms ± 0% 1.53ms ± 1% -8.85% (p=0.000 n=9+10)
VerifyPKCS1v15/2048-8 220µs ± 0% 137µs ± 1% -37.84% (p=0.000 n=9+10)
SignPSS/2048-8 1.68ms ± 0% 1.52ms ± 1% -9.16% (p=0.000 n=8+8)
VerifyPSS/2048-8 234µs ±12% 138µs ± 1% -40.87% (p=0.000 n=10+9)
Change-Id: I6c650bad9019765d793fd37a529ca186cf1eeef7
Reviewed-on: https://go-review.googlesource.com/c/go/+/445019
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
I have never encountered multi-prime RSA in the wild. A GitHub-wide
search reveals exactly two explicit uses of it (and a couple of tools
that leave the number configurable but defaulting to two).
https://github.com/decred/tumblebit/blob/31898baea/puzzle/puzzlekey.go#L38https://github.com/carl-mastrangelo/pixur/blob/95d4a4208/tools/genkeys/genkeys.go#L13
Multi-prime RSA has a slight performance advantage, but has limited
compatibility and the number of primes must be chosen carefully based on
the key size to avoid security issues. It also requires a completely
separate and rarely used private key operation code path, which if buggy
or incorrect would leak the private key.
Mark it as deprecated, and remove the dedicated CRT optimization,
falling back instead to the slower but safer non-CRT fallback.
Change-Id: Iba95edc044fcf9b37bc1f4bb59c6ea273975837f
Reviewed-on: https://go-review.googlesource.com/c/go/+/445017
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Infamously, big.Int does not provide constant-time arithmetic, making
its use in cryptographic code quite tricky. RSA uses big.Int
pervasively, in its public API, for key generation, precomputation, and
for encryption and decryption. This is a known problem. One mitigation,
blinding, is already in place during decryption. This helps mitigate the
very leaky exponentiation operation. Because big.Int is fundamentally
not constant-time, it's unfortunately difficult to guarantee that
mitigations like these are completely effective.
This patch removes the use of big.Int for encryption and decryption,
replacing it with an internal nat type instead. Signing and verification
are also affected, because they depend on encryption and decryption.
Overall, this patch degrades performance by 55% for private key
operations, and 4-5x for (much faster) public key operations.
(Signatures do both, so the slowdown is worse than decryption.)
name old time/op new time/op delta
DecryptPKCS1v15/2048-8 1.50ms ± 0% 2.34ms ± 0% +56.44% (p=0.000 n=8+10)
DecryptPKCS1v15/3072-8 4.40ms ± 0% 6.79ms ± 0% +54.33% (p=0.000 n=10+9)
DecryptPKCS1v15/4096-8 9.31ms ± 0% 15.14ms ± 0% +62.60% (p=0.000 n=10+10)
EncryptPKCS1v15/2048-8 8.16µs ± 0% 355.58µs ± 0% +4258.90% (p=0.000 n=10+9)
DecryptOAEP/2048-8 1.50ms ± 0% 2.34ms ± 0% +55.68% (p=0.000 n=10+9)
EncryptOAEP/2048-8 8.51µs ± 0% 355.95µs ± 0% +4082.75% (p=0.000 n=10+9)
SignPKCS1v15/2048-8 1.51ms ± 0% 2.69ms ± 0% +77.94% (p=0.000 n=10+10)
VerifyPKCS1v15/2048-8 7.25µs ± 0% 354.34µs ± 0% +4789.52% (p=0.000 n=9+9)
SignPSS/2048-8 1.51ms ± 0% 2.70ms ± 0% +78.80% (p=0.000 n=9+10)
VerifyPSS/2048-8 8.27µs ± 1% 355.65µs ± 0% +4199.39% (p=0.000 n=10+10)
Keep in mind that this is without any assembly at all, and that further
improvements are likely possible. I think having a review of the logic
and the cryptography would be a good idea at this stage, before we
complicate the code too much through optimization.
The bulk of the work is in nat.go. This introduces two new types: nat,
representing natural numbers, and modulus, representing moduli used in
modular arithmetic.
A nat has an "announced size", which may be larger than its "true size",
the number of bits needed to represent this number. Operations on a nat
will only ever leak its announced size, never its true size, or other
information about its value. The size of a nat is always clear based on
how its value is set. For example, x.mod(y, m) will make the announced
size of x match that of m, since x is reduced modulo m.
Operations assume that the announced size of the operands match what's
expected (with a few exceptions). For example, x.modAdd(y, m) assumes
that x and y have the same announced size as m, and that they're reduced
modulo m.
Nats are represented over unsatured bits.UintSize - 1 bit limbs. This
means that we can't reuse the assembly routines for big.Int, which use
saturated bits.UintSize limbs. The advantage of unsaturated limbs is
that it makes Montgomery multiplication faster, by needing fewer
registers in a hot loop. This makes exponentiation faster, which
consists of many Montgomery multiplications.
Moduli use nat internally. Unlike nat, the true size of a modulus always
matches its announced size. When creating a modulus, any zero padding is
removed. Moduli will also precompute constants when created, which is
another reason why having a separate type is desirable.
Updates #20654
Co-authored-by: Filippo Valsorda <filippo@golang.org>
Change-Id: I73b61f87d58ab912e80a9644e255d552cbadcced
Reviewed-on: https://go-review.googlesource.com/c/go/+/326012
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
Currently, tfork on openbsd/mips64 returns the thread ID on success and
a negative error number on error. In CL#447175, newosproc was changed
to assume that a non-zero value is an error - return zero on success to
match this expectation.
Change-Id: I955efad49b149146165eba3d05fe40ba75caa098
Reviewed-on: https://go-review.googlesource.com/c/go/+/451257
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
This test exercises the case where a net.Conn error occurs while
writing a response body. It injects an error by setting a timeout
on the Conn. If this timeout expires before response headers are
written, the test fails. The test attempts to recover from this
failure by extending the timeout and retrying.
Set the timeout after the response headers are removed, and
remove the retry loop.
Fixes#56274.
Change-Id: I293f8bedb7b20a21d14f43ea9bb48fc56b59441c
Reviewed-on: https://go-review.googlesource.com/c/go/+/452175
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Set a logger in newClientServerTest that directs the server
log output to the testing.T's log, so log output gets properly
associated with the test that caused it.
Change-Id: I13686ca35c3e21adae16b2fc37ce36daea3df9d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/452075
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Adds a method which allows users to set a fallback certificate pool for
usage during verification if the system certificate pool is empty.
Updates #43958
Change-Id: I279dd2f753743bce19790f2ae29f063c89c9359d
Reviewed-on: https://go-review.googlesource.com/c/go/+/449235
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
To pick up CL 451515.
This CL also updates x/net because x/crypto's dependency was bumped
while tagging v0.3.0.
Done by
go get -d golang.org/x/crypto@2c476679df9a
go mod tidy
go mod vendor
Change-Id: I432a04586be3784b1027aa9b62d86c0df6d4a97e
Reviewed-on: https://go-review.googlesource.com/c/go/+/452096
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
