Add built-in root certificate fallbacks

[mem]

Blank-importing golang.org/x/crypto/x509roots/fallback bundles a set of root fallback certificates from Mozilla into the resulting binary. This allows the program to run in environments where the system root certificates are not available, for example inside a minimal container. These are fallbacks, meaning that if the system does have a set of root certificates, those will be given priority. The binary size will increase a little (~ 220 kB).

It's added to main.go instead of somewhere else because the recommendation is for the package to be imported from binaries, not from libraries. Calling x509.SetFallbackRoots (what the imported package does in its init function) more than once will cause the program to panic. In principle, the Go import system will prevent the package from being imported twice, so that shouldn't be a problem, but it's probably better to keep this very visible, therefore main.go.

What?

Why?

Checklist

• I have performed a self-review of my code.
• I have added tests for my changes.
• I have run linter locally (make lint) and all checks pass.
• I have run tests locally (make tests) and all tests pass.
• I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (47c0a26) 73.22% compared to head (c45d305) 73.22%.

❗ Current head c45d305 differs from pull request most recent head c89b34d. Consider uploading reports for the commit c89b34d to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3440   +/-   ##
=======================================
  Coverage   73.22%   73.22%           
=======================================
  Files         267      267           
  Lines       20083    20083           
=======================================
  Hits        14705    14705           
+ Misses       4465     4464    -1     
- Partials      913      914    +1     

Flag	Coverage Δ
ubuntu	73.16% <ø> (-0.01%)	⬇️
windows	73.06% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codebien]

Hey @mem, thanks for your contribution! 🙇

What is the expected logic if I copy a CA in one of the file system`s directories? https://go.dev/src/crypto/x509/root_linux.go What is the first, the CA in the folder or the fallback?

for example inside a minimal container.

It sounds like an infrastructure problem to me, and the issue should be resolved by copying or installing the certificate there. Can you clarify why in your use case it doesn't work? 🙏

[mem]

What is the expected logic if I copy a CA in one of the file system`s directories? https://go.dev/src/crypto/x509/root_linux.go What is the first, the CA in the folder or the fallback?

It works as before: this is just a fallback meant to be used when the system does not provide its own certificate store.

for example inside a minimal container.

It sounds like an infrastructure problem to me, and the issue should be resolved by copying or installing the certificate there. Can you clarify why in your use case it doesn't work? 🙏

While I might agree that you should set up your containers correctly, we do not control what other people do. Even if you create a container for k6 that contains a certificate store, it's perfectly possible to reuse that container (e.g. Docker's COPY) and not have a certificate store there.

The fallback package is continuously and automatically updated from Mozilla's store. This means the certificates are handled are any other dependency (e.g. you get dependabot notifications / updates). That means there's a security advantage to using this package vs trying to keep containers up-to-date. If you look at most containers in common use, they have out-of-date certificate stores.

[olegbespalov]

LGTM 👍 Thanks for the contribution 🙇

[codebien]

This means the certificates are handled are any other dependency (e.g. you get dependabot notifications / updates).

Good point, I like it.

@mem Adding the fallbacks does mean we can remove the ca-certificates from the Dockerfile, right?

k6/Dockerfile

Line 12 in 5e83d38

RUN apk add --no-cache ca-certificates && \

Now, I'm curious why the other Grafana's main projects are not doing this, and it probably makes even more sense for projects like Grafana or Mimir. @mem Do you know why? Or are you going to open the same PRs there? I would like to have a shared vision of this across the Grafana org.

[mem]

@mem Adding the fallbacks does mean we can remove the ca-certificates from the Dockerfile, right?

k6/Dockerfile

Line 12 in 5e83d38

RUN apk add --no-cache ca-certificates && \

That is correct. I didn't want to go overboard and remove that straight away. I have done that now.

Now, I'm curious why the other Grafana's main projects are not doing this, and it probably makes even more sense for projects like Grafana or Mimir. @mem Do you know why? Or are you going to open the same PRs there? I would like to have a shared vision of this across the Grafana org.

This is relatively new ("this year" new, not "yesterday" new), and it hasn't received a lot of "press".

[mem]

--- FAIL: TestClient_TlsParameters (0.00s)
    --- FAIL: TestClient_TlsParameters/ConnectTlsClientCertWithPasswordNoClientAuth (60.01s)
        teststate_test.go:155: 
            	Error Trace:	/home/runner/work/k6/k6/js/modules/k6/grpc/teststate_test.go:155
            	            				/home/runner/work/k6/k6/js/modules/k6/grpc/client_test.go:1053
            	Error:      	"GoError: context deadline exceeded: write tcp 127.0.0.1:51926->127.0.0.1:46399: write: connection reset by peer at reflect.methodValueCall (native)" does not contain "remote error: tls: bad certificate"
            	Test:       	TestClient_TlsParameters/ConnectTlsClientCertWithPasswordNoClientAuth

Any idea how the clientAuthCA contents was generated in that file?

I don't understand why this specific test is failing if it's supposed to be using an empty cert pool. That shouldn't have changed.

[olegbespalov]

I don't understand why this specific test is failing if it's supposed to be using an empty cert pool. That shouldn't have changed.

Unfortunately, you hit our flaky test. The issue itself somehow goes down to the grpc library grpc/grpc-go#6593. I was starting to work on that, but unfortunately, I didn't succeed since it requires more changes in the grpc-go library. It is still somehow on my list, but in a low-priority.

Any idea how the clientAuthCA contents was generated in that file?

This certificate is supposed to work only for email signing. We have a low-priority task to "script" the process to make it more transparent and redundant #3549.

[codebien]

LGTM, mostly waiting for @olegbespalov's suggestion to be addressed.