18 Commits

Author SHA1 Message Date
Brad Fitzpatrick
e8379ab596 crypto/x509: add support for CertPool to load certs lazily
This will allow building CertPools that consume less memory. (Most
certs are never accessed. Different users/programs access different
ones, but not many.)

This CL only adds the new internal mechanism (and uses it for the
old AddCert) but does not modify any existing root pool behavior.
(That is, the default Unix roots are still all slurped into memory as
of this CL)

Change-Id: Ib3a42e4050627b5e34413c595d8ced839c7bfa14
Reviewed-on: https://go-review.googlesource.com/c/go/+/229917
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2020-11-07 16:59:40 +00:00
Filippo Valsorda
6f52790a20 crypto/x509: use Security.framework without cgo for roots on macOS
+----------------------------------------------------------------------+
| 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 #32604
Fixes #19561
Fixes #38365
Awakens Cthulhu

Change-Id: Id850962bad667f71e3af594bdfebbbb1edfbcbb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/227037
Reviewed-by: Katie Hockman <katie@golang.org>
2020-05-07 19:22:19 +00:00
Austin Clements
f7e6ab44b4 all: remove scattered remnants of darwin/arm
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>
2020-04-08 18:35:49 +00:00
Filippo Valsorda
2326a66878 crypto/x509: fix and cleanup loadSystemRoots on macOS
Note how untrustedData is never NULL, so loadSystemRoots was checking
the wrong thing.

Also, renamed the C function to CopyPEMRoots to follow the
CoreFoundation naming convention on ownership.

Finally, redirect all debug output to standard error.

Change-Id: Ie80abefadf8974a75c0646aa02fcfcebcbe3bde8
Reviewed-on: https://go-review.googlesource.com/c/go/+/178538
Reviewed-by: Adam Langley <agl@golang.org>
2019-05-22 16:20:11 +00:00
Filippo Valsorda
35f4ec152b crypto/x509: ignore harmless edge case in TestSystemRoots
The no-cgo validation hack lets in certificates from the root store that
are not marked as roots themselves, but are signed by a root; the cgo
path correctly excludes them. When TestSystemRoots compares cgo and
no-cgo results it tries to ignore them by ignoring certificates which
pass validation, but expired certificates were failing validation.

Letting through expired certs is harmless anyway because we will refuse
to build chains to them.

Fixes #29497

Change-Id: I341e50c0f3426de2763468672f9ba1d13ad6cfba
Reviewed-on: https://go-review.googlesource.com/c/156330
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-01-05 00:45:14 +00:00
Filippo Valsorda
303a596d8c crypto/x509: ignore 5 phantom 1024-bit roots in TestSystemRoots
On macOS 10.11, but not 10.10 and 10.12, the C API returns 5 old root
CAs which are not in SystemRootCertificates.keychain (but seem to be in
X509Anchors and maybe SystemCACertificates.keychain, along with many
others that the C API does not return). They all are moribund 1024-bit
roots which are now gone from the Apple store.

Since we can't seem to find a way to make the no-cgo code see them,
ignore them rather than skipping the test.

Fixes #21416

Change-Id: I24ff0461f71cec953b888a60b05b99bc37dad2ed
Reviewed-on: https://go-review.googlesource.com/c/156329
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-01-05 00:35:02 +00:00
Filippo Valsorda
9536c5fa69 crypto/x509: re-enable TestSystemRoots
Now that the cgo and no-cgo paths should be correct and equivalent,
re-enable the TestSystemRoots test without any margin of error (which
was tripping anyway when users had too many of a certain edge-case).

As a last quirk, the verify-cert invocation will validate certificates
that aren't roots, but are signed by valid roots. Ignore them.

Fixes #24652

Change-Id: I6a8ff3c2282136d7122a4e7e387eb8014da0d28a
Reviewed-on: https://go-review.googlesource.com/c/128117
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-12-05 22:54:01 +00:00
Martin Möhrmann
911a5fda13 crypto/x509: skip TestSystemRoots
cgo and non-cgo code paths can disagree
on the number of root certificates:
=== RUN   TestSystemRoots
--- FAIL: TestSystemRoots (0.31s)
    root_darwin_test.go:31:     cgo sys roots: 93.605184ms
    root_darwin_test.go:32: non-cgo sys roots: 213.998586ms
    root_darwin_test.go:44: got 168 roots
    root_darwin_test.go:44: got 427 roots
    root_darwin_test.go:73: insufficient overlap between cgo and non-cgo roots; want at least 213, have 168
FAIL
exit status 1

Updates #21416
Updates #24652

Change-Id: Idb6d35b17c142dfff79a10cf6b40a42d12f9d17e
Reviewed-on: https://go-review.googlesource.com/125259
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-07-27 16:44:57 +00:00
Brad Fitzpatrick
2642df9a2c crypto/x509: re-enable TestSystemRoots on darwin
It was apparently waiting on CL 36942, which was submitted.

Fixes #21416

Change-Id: I8f4ccc5a3176070abf0df019c82700c5761b5f53
Reviewed-on: https://go-review.googlesource.com/117055
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-06-14 00:25:20 +00:00
Martin Möhrmann
0fb0f575bc crypto/x509: skip TestSystemRoots
golang.org/cl/36941 enabled loading of all trusted certs on darwin
for the non-cgo execSecurityRoots.

The corresponding cgo version golang.org/cl/36942 for systemRootsPool
has not been merged yet.

This tests fails reliably on some darwin systems:
--- FAIL: TestSystemRoots (1.28s)
        root_darwin_test.go:31:     cgo sys roots: 353.552363ms
        root_darwin_test.go:32: non-cgo sys roots: 921.85297ms
        root_darwin_test.go:44: got 169 roots
        root_darwin_test.go:44: got 455 roots
        root_darwin_test.go:73: insufficient overlap between cgo and non-cgo roots; want at least 227, have 168
FAIL
FAIL    crypto/x509     2.445s

Updates #16532
Updates #21416

Change-Id: I52c2c847651fb3621fdb6ab858ebe8e28894c201
Reviewed-on: https://go-review.googlesource.com/57830
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2017-08-22 19:54:08 +00:00
Brad Fitzpatrick
3357daa96e crypto/x509: speed up and deflake non-cgo Darwin root cert discovery
Piping into security verify-cert only worked on macOS Sierra, and was
flaky for unknown reasons. Users reported that the number of trusted
root certs stopped randomly jumping around once they switched to using
verify-cert against files on disk instead of /dev/stdin.

But even using "security verify-cert" on 150-200 certs took too
long. It took 3.5 seconds on my machine. More than 4 goroutines
hitting verify-cert didn't help much, and soon started to hurt
instead.

New strategy, from comments in the code:

// 1. Run "security trust-settings-export" and "security
//    trust-settings-export -d" to discover the set of certs with some
//    user-tweaked trusy policy. We're too lazy to parse the XML (at
//    least at this stage of Go 1.8) to understand what the trust
//    policy actually is. We just learn that there is _some_ policy.
//
// 2. Run "security find-certificate" to dump the list of system root
//    CAs in PEM format.
//
// 3. For each dumped cert, conditionally verify it with "security
//    verify-cert" if that cert was in the set discovered in Step 1.
//    Without the Step 1 optimization, running "security verify-cert"
//    150-200 times takes 3.5 seconds. With the optimization, the
//    whole process takes about 180 milliseconds with 1 untrusted root
//    CA. (Compared to 110ms in the cgo path)

Fixes #18203

Change-Id: I4e9c11fa50d0273c615382e0d8f9fd03498d4cb4
Reviewed-on: https://go-review.googlesource.com/34389
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Quentin Smith <quentin@golang.org>
2016-12-20 04:48:09 +00:00
Brad Fitzpatrick
be8a6fd2e3 crypto/x509: reduce test's sought number of system certs
150 is too high for some people.

Reports of 132, 145, 149 on OS X.

Fixes #18203

Change-Id: I559639aba7e87e07d1a1249f8b212b3f34a078ab
Reviewed-on: https://go-review.googlesource.com/34019
Reviewed-by: Russ Cox <rsc@golang.org>
2016-12-07 17:09:37 +00:00
Quentin Smith
7e5b2e0ec1 crypto/x509: read Darwin trust settings for root CAs
Darwin separately stores bits indicating whether a root certificate
should be trusted; this changes Go to read and use those when
initializing SystemCertPool.

Unfortunately, the trust API is very slow. To avoid a delay of up to
0.5s in initializing the system cert pool, we assume that
the trust settings found in kSecTrustSettingsDomainSystem will always
indicate trust. (That is, all root certs Apple distributes are trusted.)
This is not guaranteed by the API but is true in practice.

In the non-cgo codepath, we do not have that benefit, so we must check
the trust status of every certificate. This causes about 0.5s of delay
in initializing the SystemCertPool.

On OS X 10.11 and older, the "security" command requires a certificate
to be provided in a file and not on stdin, so the non-cgo codepath
creates temporary files for each certificate, further slowing initialization.

Updates #18141.

Change-Id: If681c514047afe5e1a68de6c9d40ceabbce54755
Reviewed-on: https://go-review.googlesource.com/33721
Run-TryBot: Quentin Smith <quentin@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2016-12-01 19:24:34 +00:00
Martin Möhrmann
fdd0179bb1 all: fix typos and spelling
Change-Id: Icd06d99c42b8299fd931c7da821e1f418684d913
Reviewed-on: https://go-review.googlesource.com/19829
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-02-24 18:42:29 +00:00
David Crawshaw
439318dd50 crypto/x509: skip arm64 tests limited by iOS
Just like darwin/arm.

Change-Id: Ib0438021bfe9eb105222b93e5bb375c282cc7b8c
Reviewed-on: https://go-review.googlesource.com/8822
Reviewed-by: Minux Ma <minux@golang.org>
2015-04-13 11:55:32 +00:00
David Crawshaw
b1517c39fb crypto/x509: skip tests not made for darwin/arm
Change-Id: I8b18dc840425b72d7172a35cb0ba004bd156492d
Reviewed-on: https://go-review.googlesource.com/6252
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2015-02-27 19:38:33 +00:00
Mikio Hara
a1053ed610 crypto/x509: add missing copyright
Change-Id: Ida3b431a06527f6cd604ab4af5ce517959c8619b
Reviewed-on: https://go-review.googlesource.com/2306
Reviewed-by: Dave Cheney <dave@cheney.net>
2015-01-05 07:32:02 +00:00
Russ Cox
c007ce824d build: move package sources from src/pkg to src
Preparation was in CL 134570043.
This CL contains only the effect of 'hg mv src/pkg/* src'.
For more about the move, see golang.org/s/go14nopkg.
2014-09-08 00:08:51 -04:00