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>
This reverts CL 223754.
Reason for revert: new tests are failing on all longtest builders.
Change-Id: I2257d106c132f3a02c0af6b20061d4f9a8093c4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/225077
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Katie Hockman <katie@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.
Fixes#21704
Change-Id: Id5379c96384a11c5afde0614955360e7470bb1c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
The existing Certificate.CreateCRL method generates non-conformant CRLs and
as such cannot be used for implementations that require standards
compliance. This change implements a new top level method, CreateCRL, which
generates compliant CRLs, and offers an extensible API if any
extensions/fields need to be supported in the future.
Here is an example Issuer/CRL generated using this change:
-----BEGIN CERTIFICATE-----
MIIBNjCB3aADAgECAgEWMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMTB3Rlc3Rpbmcw
IhgPMDAwMTAxMDEwMDAwMDBaGA8wMDAxMDEwMTAwMDAwMFowEjEQMA4GA1UEAxMH
dGVzdGluZzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLHrudbSM36sn1VBrmm/
OfQTyEsI4tIUV1VmneOKHL9ENBGCiec4GhQm2SGnDT/sZy2bB3c3yozh/roS6cZJ
UZqjIDAeMA4GA1UdDwEB/wQEAwIBAjAMBgNVHQ4EBQQDAQIDMAoGCCqGSM49BAMC
A0gAMEUCIQCoAYN6CGZPgd5Sw5a1rd5VexciT5MCxTfXj+ZfJNfoiAIgQVCTB8AE
Nm2xset7+HOgtQYlKNw/rGd8cFcv5Y9aUzo=
-----END CERTIFICATE-----
-----BEGIN X509 CRL-----
MIHWMH0CAQEwCgYIKoZIzj0EAwIwEjEQMA4GA1UEAxMHdGVzdGluZxgPMDAwMTAx
MDIwMDAwMDBaGA8wMDAxMDEwMzAwMDAwMFowFjAUAgECGA8wMDAxMDEwMTAxMDAw
MFqgHjAcMA4GA1UdIwQHMAWAAwECAzAKBgNVHRQEAwIBBTAKBggqhkjOPQQDAgNJ
ADBGAiEAjqfj/IG4ys5WkjrbTNpDbr+saHGO/NujLJotlLL9KzgCIQDm8VZPzj0f
NYEQgAW4nsiUzlvEUCoHMw0141VCZXv67A==
-----END X509 CRL-----
Fixes#35428
Change-Id: Id96b6f47698d0bed39d586b46bd12374ee6ff88f
GitHub-Last-Rev: c83a601716
GitHub-Pull-Request: golang/go#36945
Reviewed-on: https://go-review.googlesource.com/c/go/+/217298
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The minimum macOS supported version is 10.11 as of Go 1.14, see #23011.
Thus, bump macosx-version-min to 10.11
While at it, drop __MAC_OS_X_VERSION_MAX_ALLOWED as suggested by
Filippo:
In general, I can see why we'd want to tell the libraries which
minimum version we target so they drop compatibility with older
versions. No idea why we'd specify a max version, unless it's to make
sure we don't use APIs added after that version, but then it would
have to be 1011 not 1015.
Let's try dropping that define and see if anything blows up? ¯\_(ツ)_/¯
Change-Id: I6b76623a9404724ccda40311ff95b3475ae8a60c
Reviewed-on: https://go-review.googlesource.com/c/go/+/214059
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Changing "man-in-the-middle" references to "machine-in-the-middle",
it's a more inclusive term and still aligns with the MITM acronym.
Change-Id: I81f954cff3d252433443f159ff9edaf59a28ab9d
GitHub-Last-Rev: 3e8f91424a
GitHub-Pull-Request: golang/go#37918
Reviewed-on: https://go-review.googlesource.com/c/go/+/223897
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
"SSL_CERT_DIR" is meant to hold more than one directory, when a colon
is used as a delimiter. However, we assumed it'd be a single directory
for all root certificates.
OpenSSL and BoringSSL properly respected the colon separated
"SSL_CERT_DIR", as per:
* OpenSSL 12a765a523/crypto/x509/by_dir.c (L153-L209)
* BoringSSL 3ba9586bc0/crypto/x509/by_dir.c (L194-L247)
This change adds that parity to loadSystemRoots.
RELNOTE=yes
Fixes#35325
Change-Id: I0d554a00ccc34300a7f0529aa741ee7e2d5762f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/205237
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When run as a separate program, the code in ExampleDial panicked due to
an expired certificate. Fixed this problem by replacing the expired
certificate with a valid one.
Also added a comment in the certificate to give a hint about why it
might fail in the future.
Fixes#35706
Change-Id: I3d300f7bccae050e4b73ded28b8029aa04b480bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/212601
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
