Restore previously sent SCTs and stapled OCSP response during session
resumption for both TLS 1.2 and 1.3. This behavior is somewhat
complicated for TLS 1.2 as SCTs are sent during the server hello,
so they override what is saved in ClientSessionState. It is likely
that if the server is sending a different set of SCTs there is probably
a reason for doing so, such as a log being retired, or SCT validation
requirements changing, so it makes sense to defer to the server in
that case.
Fixes#39075
Change-Id: I3c0fa2f69c6bf0247a447c48a1b4c733a882a233
Reviewed-on: https://go-review.googlesource.com/c/go/+/234237
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Rather than hashing the encoding of the SPKI structure, hash the
bytes of the public key itself.
Fixes#39429
Change-Id: I55a0f8f08ab1f1b5702590b47d8b9a92d1dbcc1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/236878
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This change makes the direct call darwin loadSystemRoots implementation
match the existing cgo implementation, which in turn _mostly_ matches
the Apple implementation. The main change here is that when
SecTrustSettingsCopyTrustSettings the error is ignored, and can either
cause a fallback to check admin trust settings, or cause the
certificate to be marked kSecTrustSettingsResultUnspecified.
As well as updating the implementation to match the cgo one, this
change also updates the documentation of how the fallbacks work and
how they match the Apple implementations. References are made to the
Apple source where appropriate. This change does not update the
existing comments in the cgo implementation, since the goal is to
delete that code once the direct call implementation is matured.
Updates #38888
Change-Id: Id0344ea9d2eede3b715f341e9cbd3c1c661b7a90
Reviewed-on: https://go-review.googlesource.com/c/go/+/233360
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The ConnectionState's CipherSuite was not set prior
to the VerifyConnection callback in TLS 1.2 servers,
both for full handshakes and resumptions.
Change-Id: Iab91783eff84d1b42ca09c8df08e07861e18da30
Reviewed-on: https://go-review.googlesource.com/c/go/+/236558
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Also add a test that could reproduce this error and
ensure it doesn't occur in other configurations.
Fixes#39012
Change-Id: If792b5131f312c269fd2c5f08c9ed5c00188d1af
Reviewed-on: https://go-review.googlesource.com/c/go/+/233957
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This change adds a comment to the Verify documentation that indicates
that you can use URI and email style name constraints with a leading
period for DNS names (and explains what they do). This behavior is
not standards compliant, but matches the community application of
RFC 5280, so it makes sense to document it.
Fixes#37535
Change-Id: Ibd6f039e4fa46d40ad7ae1ab48eab86f13cf8eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/233525
Reviewed-by: Ryan Sleevi <sleevi@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This fixes a bug in CL 228777 which disallowed
a MaxPathLen of -1 without IsCA, even though the
x509.Certificate documentation indicates that
MaxPathLen of -1 is considered "unset".
Updates #38216
Change-Id: Ib7240e00408d060f27567be8b820d0eee239256f
Reviewed-on: https://go-review.googlesource.com/c/go/+/235280
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
In CL 231958, TempDir was changed to create a new temp directory on
each allocation, on the theory that it is easy to save in a variable
for callers that want the same directory repeatedly. Apply that
transformation here.
Updates #38850
Change-Id: Ibb014095426c33038e0a2c95303579cf95d5c3ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/234582
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was causing issues when fuzzing with
TestMarshalUnmarshal since the test would
occassionally set the version to VersionTLS13,
which would fail when unmarshaling. The check
doesn't add much in practice, and there is no
harm in removing it to de-flake the test.
Fixes#38902
Change-Id: I0906c570e9ed69c85fdd2c15f1b52f9e372c62e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/234486
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Since the ConnectionState will now be available during
verification, some code was moved around in order to
initialize and make available as much of the fields on
Conn as possible before the ConnectionState is verified.
Fixes#36736
Change-Id: I0e3efa97565ead7de5c48bb8a87e3ea54fbde140
Reviewed-on: https://go-review.googlesource.com/c/go/+/229122
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Automatically rotate session ticket keys for servers
that don't already have sessionTicketKeys and that
haven't called SetSessionTicketKeys.
Now, session ticket keys will be rotated every 24 hours
with a lifetime of 7 days. This adds a small performance
cost to existing clients that don't provide a session
ticket encrypted with a fresh enough session ticket key,
which would require a full handshake.
Updates #25256
Change-Id: I15b46af7a82aab9a108bceb706bbf66243a1510f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230679
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Colons are port separators, so it's risky to allow them in hostnames.
Per the CL 231377 rule, if we at least consider them invalid we will not
apply wildcard processing to them, making behavior a little more
predictable.
We were considering hostnames with colons valid (against spec) because
that meant we'd not ignore them in Common Name. (There was at least
one deployment that was putting colons in Common Name and expecting it
to verify.)
Now that Common Name is ignored by default, those clients will break
again, so it's a good time to drop the exception. Hopefully they moved
to SANs, where invalid hostnames are checked 1:1 (ignoring wildcards)
but still work. (If they didn't, this change means they can't use
GODEBUG=x509ignoreCN=0 to opt back in, but again you don't get to use a
legacy deprecated field AND invalid hostnames.)
Updates #24151
Change-Id: Id44b4fecb2d620480acdfc65fea1473f7abbca7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231381
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trailing dots are not allowed in certificate fields like CN and SANs
(while they are allowed and ignored as inputs to verification APIs).
Move to considering names with trailing dots in certificates as invalid
hostnames.
Following the rule of CL 231378, these invalid names lose wildcard
processing, but can still match if there is a 1:1 match, trailing dot
included, with the VerifyHostname input.
They also become ignored Common Name values regardless of the
GODEBUG=x509ignoreCN=X value, because we have to ignore invalid
hostnames in Common Name for #24151. The error message automatically
accounts for this, and doesn't suggest the environment variable. You
don't get to use a legacy deprecated field AND invalid hostnames.
(While at it, also consider wildcards in VerifyHostname inputs as
invalid hostnames, not that it should change any observed behavior.)
Change-Id: Iecdee8927df50c1d9daf904776b051de9f5e76ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/231380
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Common Name has been deprecated for 20 years, and has horrible
interactions with Name Constraints. The browsers managed to drop it last
year, let's try flicking the switch to disabled by default.
Return helpful errors for things that would get unbroken by flipping the
switch back with the environment variable.
Had to refresh a test certificate that was too old to have SANs.
Updates #24151
Change-Id: I2ab78577fd936ba67969d3417284dbe46e4ae02f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231379
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When the input or SAN dNSNames are not valid hostnames, the specs don't
define what should happen, because this should ideally never happen, so
everything we do is undefined behavior. Browsers get to just return an
error, because browsers can assume that the resolving layer is DNS. We
can't, names can be resolved by anything implementing a Dial function,
and the crypto/x509 APIs can also be used directly without actual
networks in sight.
Trying to process invalid hostnames leads to issues like #27591 where
wildcards glob stuff they aren't expected to, because wildcards are only
defined on hostnames.
Try to rationalize the behavior like this: if both the VerifyHostname
input and the SAN dNSNames are a valid hostname, follow the specs;
otherwise, only accept perfect 1:1 case-insensitive matches (without
wildcards or trailing dot processing).
This should allow us to keep supporting weird names, with less
unexpected side-effects from undefined behavior. Also, it's a rule, even
if completely made up, so something we can reason about and code against.
The commonName field does allow any string, but no specs define how to
process it. Processing it differently from dNSNames would be confusing,
and allowing it to match invalid hostnames is incompatible with Name
Constraint processing (#24151).
This does encourage invalid dNSNames, regrettably, but we need some way
for the standard API to match weird names, and the alternative of
keeping CN alive sounds less appealing.
Fixes#27591
Change-Id: Id2d515f068a17ff796a32b30733abe44ad4f0339
Reviewed-on: https://go-review.googlesource.com/c/go/+/231378
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
+----------------------------------------------------------------------+
| Hello, if you are reading this and run macOS, please test this code: |
| |
| $ GO111MODULE=on go get golang.org/dl/gotip@latest |
| $ gotip download |
| $ GODEBUG=x509roots=1 gotip test crypto/x509 -v -run TestSystemRoots |
+----------------------------------------------------------------------+
We currently have two code paths to extract system roots on macOS: one
uses cgo to invoke a maze of Security.framework APIs; the other is a
horrible fallback that runs "/usr/bin/security verify-cert" on every
root that has custom policies to check if it's trusted for SSL.
The fallback is not only terrifying because it shells out to a binary,
but also because it lets in certificates that are not trusted roots but
are signed by trusted roots, and because it applies some filters (EKUs
and expiration) only to roots with custom policies, as the others are
not passed to verify-cert. The other code path, of course, requires cgo,
so can't be used when cross-compiling and involves a large ball of C.
It's all a mess, and it broke oh-so-many times (#14514, #16532, #19436,
#20990, #21416, #24437, #24652, #25649, #26073, #27958, #28025, #28092,
#29497, #30471, #30672, #30763, #30889, #32891, #38215, #38365, ...).
Since macOS does not have a stable syscall ABI, we already dynamically
link and invoke libSystem.dylib regardless of cgo availability (#17490).
How that works is that functions in package syscall (like syscall.Open)
take the address of assembly trampolines (like libc_open_trampoline)
that jump to symbols imported with cgo_import_dynamic (like libc_open),
and pass them along with arguments to syscall.syscall (which is
implemented as runtime.syscall_syscall). syscall_syscall informs the
scheduler and profiler, and then uses asmcgocall to switch to a system
stack and invoke runtime.syscall. The latter is an assembly trampoline
that unpacks the Go ABI arguments passed to syscall.syscall, finally
calls the remote function, and puts the return value on the Go stack.
(This last bit is the part that cgo compiles from a C wrapper.)
We can do something similar to link and invoke Security.framework!
The one difference is that runtime.syscall and friends check errors
based on the errno convention, which Security doesn't follow, so I added
runtime.syscallNoErr which just skips interpreting the return value.
We only need a variant with six arguments because the calling convention
is register-based, and extra arguments simply zero out some registers.
That's plumbed through as crypto/x509/internal/macOS.syscall. The rest
of that package is a set of wrappers for Security.framework and Core
Foundation functions, like syscall is for libSystem. In theory, as long
as macOS respects ABI backwards compatibility (a.k.a. as long as
binaries built for a previous OS version keep running) this should be
stable, as the final result is not different from what a C compiler
would make. (One exception might be dictionary key strings, which we
make our own copy of instead of using the dynamic symbol. If they change
the value of those strings things might break. But why would they.)
Finally, I rewrote the crypto/x509 cgo logic in Go using those wrappers.
It works! I tried to make it match 1:1 the old logic, so that
root_darwin_amd64.go can be reviewed by comparing it to
root_cgo_darwin_amd64.go. The only difference is that we do proper error
handling now, and assume that if there is no error the return values are
there, while before we'd just check for nil pointers and move on.
I kept the cgo logic to help with review and testing, but we should
delete it once we are confident the new code works.
The nocgo logic is gone and we shall never speak of it again.
Fixes#32604Fixes#19561Fixes#38365
Awakens Cthulhu
Change-Id: Id850962bad667f71e3af594bdfebbbb1edfbcbb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/227037
Reviewed-by: Katie Hockman <katie@golang.org>
Also encode the certificates in a way that's more
consistent with TLS 1.3 (with a 24 byte length prefix).
Note that this will have an additional performance cost
requiring clients to do a full handshake every 7 days
where previously they were able to use the same ticket
indefinitely.
Updates #25256
Change-Id: Ic4d1ba0d92773c490b33b5f6c1320d557cc7347d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231317
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Speed up repeated HMAC operations with the same key by not recomputing
the first block of the inner and outer hashes in Reset and Sum, saving
two block computations each time.
This is a significant win for applications which hash many small
messages with the same key. In x/crypto/pbkdf2 for example, this
optimization cuts the number of block computations in half, speeding it
up by 25%-40% depending on the hash function.
The hash function needs to implement binary.Marshaler and
binary.Unmarshaler for this optimization to work, so that we can save
and restore its internal state. All hash functions in the standard
library are marshalable (CL 66710) but if the hash isn't marshalable, we
fall back on the old behaviour.
Marshaling the hashes does add a couple unavoidable new allocations, but
this only has to be done once, so the cost is amortized over repeated
uses. To minimize impact to applications which don't (or can't) reuse
hmac objects, marshaling is performed in Reset (rather than in New),
since calling Reset seems like a good indication that the caller intends
to reuse the hmac object later.
I had to add a boolean field to the hmac state to remember if we've
marshaled the hashes or not. This is paid for by removing the size and
blocksize fields, which were basically unused except for some
initialization work in New, and to fulfill the Size and Blocksize
methods. Size and Blocksize can just be forwarded to the underlying
hash, so there doesn't really seem to be any reason to waste space
caching their values.
crypto/hmac benchmarks:
name old time/op new time/op delta
HMAC_Reset/SHA1/1K-2 4.06µs ± 0% 3.77µs ± 0% -7.29% (p=0.000 n=8+10)
HMAC_Reset/SHA1/32-2 1.08µs ± 0% 0.78µs ± 1% -27.67% (p=0.000 n=10+10)
HMAC_Reset/SHA256/1K-2 10.3µs ± 0% 9.4µs ± 0% -9.03% (p=0.000 n=10+10)
HMAC_Reset/SHA256/32-2 2.32µs ± 0% 1.42µs ± 0% -38.87% (p=0.000 n=10+10)
HMAC_Reset/SHA512/1K-2 8.22µs ± 0% 7.04µs ± 0% -14.32% (p=0.000 n=9+9)
HMAC_Reset/SHA512/32-2 3.08µs ± 0% 1.89µs ± 0% -38.54% (p=0.000 n=10+9)
HMAC_New/SHA1/1K-2 4.86µs ± 1% 4.93µs ± 1% +1.30% (p=0.000 n=10+9)
HMAC_New/SHA1/32-2 1.91µs ± 1% 1.95µs ± 1% +1.84% (p=0.000 n=10+9)
HMAC_New/SHA256/1K-2 11.2µs ± 1% 11.2µs ± 0% ~ (p=1.000 n=9+10)
HMAC_New/SHA256/32-2 3.22µs ± 2% 3.19µs ± 2% -1.07% (p=0.018 n=9+10)
HMAC_New/SHA512/1K-2 9.54µs ± 0% 9.66µs ± 1% +1.31% (p=0.000 n=9+10)
HMAC_New/SHA512/32-2 4.37µs ± 1% 4.46µs ± 1% +1.97% (p=0.000 n=10+9)
name old speed new speed delta
HMAC_Reset/SHA1/1K-2 252MB/s ± 0% 272MB/s ± 0% +7.86% (p=0.000 n=8+10)
HMAC_Reset/SHA1/32-2 29.7MB/s ± 0% 41.1MB/s ± 1% +38.26% (p=0.000 n=10+10)
HMAC_Reset/SHA256/1K-2 99.1MB/s ± 0% 108.9MB/s ± 0% +9.93% (p=0.000 n=10+10)
HMAC_Reset/SHA256/32-2 13.8MB/s ± 0% 22.6MB/s ± 0% +63.57% (p=0.000 n=10+10)
HMAC_Reset/SHA512/1K-2 125MB/s ± 0% 145MB/s ± 0% +16.71% (p=0.000 n=9+9)
HMAC_Reset/SHA512/32-2 10.4MB/s ± 0% 16.9MB/s ± 0% +62.69% (p=0.000 n=10+9)
HMAC_New/SHA1/1K-2 211MB/s ± 1% 208MB/s ± 1% -1.29% (p=0.000 n=10+9)
HMAC_New/SHA1/32-2 16.7MB/s ± 1% 16.4MB/s ± 1% -1.81% (p=0.000 n=10+9)
HMAC_New/SHA256/1K-2 91.3MB/s ± 1% 91.5MB/s ± 0% ~ (p=0.950 n=9+10)
HMAC_New/SHA256/32-2 9.94MB/s ± 2% 10.04MB/s ± 2% +1.09% (p=0.021 n=9+10)
HMAC_New/SHA512/1K-2 107MB/s ± 0% 106MB/s ± 1% -1.29% (p=0.000 n=9+10)
HMAC_New/SHA512/32-2 7.32MB/s ± 1% 7.18MB/s ± 1% -1.89% (p=0.000 n=10+9)
name old alloc/op new alloc/op delta
HMAC_Reset/SHA1/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA1/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/1K-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/32-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal)
HMAC_New/SHA1/1K-2 448B ± 0% 448B ± 0% ~ (all samples are equal)
HMAC_New/SHA1/32-2 448B ± 0% 448B ± 0% ~ (all samples are equal)
HMAC_New/SHA256/1K-2 480B ± 0% 480B ± 0% ~ (all samples are equal)
HMAC_New/SHA256/32-2 480B ± 0% 480B ± 0% ~ (all samples are equal)
HMAC_New/SHA512/1K-2 800B ± 0% 800B ± 0% ~ (all samples are equal)
HMAC_New/SHA512/32-2 800B ± 0% 800B ± 0% ~ (all samples are equal)
name old allocs/op new allocs/op delta
HMAC_Reset/SHA1/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA1/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA256/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/1K-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_Reset/SHA512/32-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal)
HMAC_New/SHA1/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA1/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA256/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA256/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA512/1K-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
HMAC_New/SHA512/32-2 5.00 ± 0% 5.00 ± 0% ~ (all samples are equal)
x/crypto/pbkdf2 benchmarks:
name old time/op new time/op delta
HMACSHA1-2 4.63ms ± 0% 3.40ms ± 0% -26.58% (p=0.000 n=10+9)
HMACSHA256-2 9.75ms ± 0% 5.98ms ± 0% -38.62% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
HMACSHA1-2 516B ± 0% 708B ± 0% +37.21% (p=0.000 n=10+10)
HMACSHA256-2 549B ± 0% 772B ± 0% +40.62% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
HMACSHA1-2 8.00 ± 0% 10.00 ± 0% +25.00% (p=0.000 n=10+10)
HMACSHA256-2 8.00 ± 0% 10.00 ± 0% +25.00% (p=0.000 n=10+10)
Fixes#19941
Change-Id: I7077a6f875be68d3da05f7b3664e18514861886f
Reviewed-on: https://go-review.googlesource.com/c/go/+/27458
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Before going around making changes, surface the current behavior in the
docs as a starting point. No behavior changes.
Change-Id: If8096cedbba7eda37694dbb7f438046d590c3bcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/231377
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Clients have to reject any HelloRetryRequest message that doesn't lead
to a change in the ClientHello. Instead, we were rejecting any HRR that
didn't select an alternative group, even if it sent a cookie, which
would change the CH.
The good news is that I know of no TLS servers that use or need HRRs
exclusively for cookies (which are mostly useful in DTLS as a way to
verify the source address). The bad news is that we poisoned the
ecosystem as Go 1.12 to 1.14 will reject such HRRs. Oops, hopefully no
one needed this.
No tests because neither Go nor s_server support cookies. This would
presumably get covered once we integrate BoGo.
Fixes#30149
Change-Id: I760fb1ded81148ac3096cf201cbc1e941374b83d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231039
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Replaced almost every use of Bytes with FillBytes.
Note that the approved proposal was for
func (*Int) FillBytes(buf []byte)
while this implements
func (*Int) FillBytes(buf []byte) []byte
because the latter was far nicer to use in all callsites.
Fixes#35833
Change-Id: Ia912df123e5d79b763845312ea3d9a8051343c0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230397
Reviewed-by: Robert Griesemer <gri@golang.org>
Previously, non-standard attributes in Name.Names were being
omitted when printed using Name.String(). Now, any non-standard
attributes that would not already be printed in Name.String()
are being added temporarily to Name.ExtraNames to be printed.
Fixes#33094Fixes#23069
Change-Id: Id9829c20968e16db7194549f69c0eb5985044944
Reviewed-on: https://go-review.googlesource.com/c/go/+/229864
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Per suggestion in CL 202578, this CL drops the purego build tag used
within this package.
Change-Id: I33626c73d6602e321528544ee601741f7e634c1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230677
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
This CL allows the usage of KDSA instruction when it is available. The
instruction is designed to be resistant to side channel attacks and
offers performance improvement for ed25519.
Benchmarks:
name old time/op new time/op delta
Signing-8 120µs ±20% 62µs ±12% -48.40% (p=0.000 n=10+10)
Verification-8 325µs ±17% 69µs ±10% -78.80% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
Signing-8 448B ± 0% 0B -100.00% (p=0.000 n=10+10)
Verification-8 288B ± 0% 0B -100.00% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
Signing-8 5.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10)
Verification-8 2.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10)
Change-Id: I0330ce83d807370b419ce638bc2cae4cb3c250dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/202578
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
On darwin/arm64, the copy of the system roots takes 256 KiB of disk
and 560 KiB of memory after parsing them (which is retained forever in
a package global by x509/root.go). In constrained environments like
iOS NetworkExtensions where total disk+RAM is capped at 15 MiB, these
certs take 5.3% of the total allowed memory.
It turns out you can get down from 816 KiB to 110 KiB by instead
storing compressed x509 certs in the binary and lazily inflating just
the needed certs at runtime as a function of the certs presented to
you by the server, then building a custom root CertPool in the
crypto/tls.Config.VerifyPeerCertificate hook.
This then saves 706 KiB.
Arguably that should be the default Go behavior, but involves
cooperation between x509 and tls, and adds a dependency to
compress/gzip. Also, it may not be the right trade-off for everybody,
as it involves burning more CPU on new TLS connections. Most iOS apps
don't run in a NetworkExtension context limiting them to 15 MiB.
The build tag is chosen to match the existing "nethttpomithttp2".
Change-Id: I7b1c845de08b22674f81dd546e7fadc7dda68bd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/229762
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This saves 166 KiB for a tls.Dial hello world program (5382441 to
5212356 to bytes), by permitting the linker to remove TLS server code.
Change-Id: I16610b836bb0802b7d84995ff881d79ec03b6a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/228111
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The cgo build tag is not necessary for root_darwin_arm64.go. We can't
build for darwin/arm64 without cgo, and even if we did 1) this code
would work fine 2) the no-cgo code that shells out to
/usr/bin/security would not work.
(Suggested by Filippo.)
Change-Id: I98cac2ea96ec5ac1ae60b7e32d195d5e86e2bd66
Reviewed-on: https://go-review.googlesource.com/c/go/+/227583
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This removes all conditions and conditional code (that I could find)
that depended on darwin/arm.
Fixes#35439 (since that only happened on darwin/arm)
Fixes#37611.
Change-Id: Ia4c32a5a4368ed75231075832b0b5bfb1ad11986
Reviewed-on: https://go-review.googlesource.com/c/go/+/227198
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all files that are only used on darwin/arm and cleans up
build tags in files that are still used on other platforms.
Updates #37611.
Change-Id: Ic9490cf0edfc157c6276a7ca950c1768b34a998f
Reviewed-on: https://go-review.googlesource.com/c/go/+/227197
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Cleaned up for readability and consistency.
There is one tiny behavioral change: when PSSSaltLengthEqualsHash is
used and both hash and opts.Hash were set, hash.Size() was used for the
salt length instead of opts.Hash.Size(). That's clearly wrong because
opts.Hash is documented to override hash.
Change-Id: I3e25dad933961eac827c6d2e3bbfe45fc5a6fb0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/226937
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Per RFC 8017, reject signatures which are not the same length as the RSA
modulus. This matches the behavior of SignPKCS1v15 which properly left pads
the signatures it generates to the size of the modulus.
Fixes#21896
Change-Id: I2c42a0b24cf7fff158ece604b6f0c521a856d932
GitHub-Last-Rev: 6040f79906
GitHub-Pull-Request: golang/go#38140
Reviewed-on: https://go-review.googlesource.com/c/go/+/226203
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.
interface {
Equal(crypto.PublicKey) bool
}
Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.
Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.
Re-landing of CL 223754. Dropped the test that was assuming named curves
are not implemented by CurveParams, because it's not true for all
curves, and anyway is not a property we need to test. There is still a
test to check that different curves make keys not Equal.
Fixes#21704Fixes#38035
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
Change-Id: I736759b145bfb4f7f8eecd78c324315d5a05385c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225460
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Roland Shoemaker
Katie Hockman
Bryan C. Mills
Filippo Valsorda
Evgeniy Kulikov
Andrew Ekstedt
Brad Fitzpatrick
Ruixin Bao
Brad Fitzpatrick
Austin Clements
Xiangdong Ji
alex-semenyuk