Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/x509: add SetFallbackRoots and golang.org/x/crypto/x509roots/fallback package #43958

Open
breml opened this issue Jan 27, 2021 · 74 comments
Assignees
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues Proposal-FinalCommentPeriod
Milestone

Comments

@breml
Copy link
Contributor

breml commented Jan 27, 2021

Proposal

Go programs currently rely on the operating system to provide CA root certificate information in some sort of certificate system store. There are situations, where no such up-to-date CA root certificates are available like (Docker) containers built FROM scratch or out of date environments like poorly maintained or no longer updatable systems (e.g. older hardware appliances).

For some environments it is possible for the user of a Go program to add additional CA root certificates via SSL_CERT_FILE or SSL_CERT_DIR, but this is not the case for all supported environments (e.g. Windows) and it is definitively not user friendly.

Therefore, it is desirable for Go programs to have some mechanism to directly embed CA root certificate information into the program itself, so that they don't have to rely on system store to provide CA root certificates that may be absent (or out of date).

I make the following assumptions, which I think are reasonable:

  • adding the CA root certificates into a program should be optional, as it increases the size of the program by approximately 250K (uncompressed), and most programs run in properly maintained systems with an up-to-date certificate system store
  • it should be possible to embed CA root certificates when building an arbitrary program
  • it should be possible for a program to always embed CA root certificates without requiring any special build step
  • the program should always prefer data from the system if available, as it is likely to be more up to date than that included in the program
  • the program should allow to override the preference and force the usage of the embeded data (controled by an environment variable, e.g. GO_ROOTCERTS_ENABLE)

Given those assumptions, I propose adding a new package crypto/x509/rootcerts. Importing this package (as import _ "crypto/x509/rootcerts") will cause CA root certificates to be embedded in the program. Also, building with the build tag rootcerts will force the package to be imported, and therefore will cause CA root certificates to be embedded in the program. The embedded CA root certificates will be used if and when no certificate system store is available (or the user forces the usage of the embedded data).

Source for the CA Root Certificates

I propose to use the Mozilla Included CA Certificate List, more specifically the PEM of Root Certificates in Mozilla's Root Store with the Websites (TLS/SSL) Trust Bit Enabled as the source for the CA root certificates.

The Mozilla Included CA Certificate List is the source for the CA root certificates embeded in the well known products of the Mozilla Foundation like for example Firefox (web browser) or Thunderbird (email client).

In contrast to most of the other software vendors, Mozilla maintains its Included CA Certificate List publicly and distributes it under an open source license (Mozilla Public License Version 2). This is also the reason why most of the Linux distributions, as well as
other free unix derivates and wide spread tools, use this list of CA root certificates as part of their distribution.

Some examples:

In summary in my opinion it is safe to say that the Mozilla Included CA Certificate List is well established and widely used.
In fact, if a Go program is run on Linux or an other free Unix derivate, chances are high that the root certificates used by the program are already provided by the Mozilla Included CA Certificate List.

Why include into the Go Standard Library

As the sample implementation (link in Annex below) clearly demostrates, that it is possible to write a 3rd party Go package, which achieves the same goal as the proposed package crypto/x509/rootcerts would. The main difference between a package in the standard library and a 3rd party package is: TRUST.

The root certificates are the top-most certificates in the trust chain and used to ensure the trustworthiness of the certificates signed by them either directly (intermediate certificates) or indirectly (through intermediate certificates). Therefore for a package containing and replacing the root certificates, trust is essential.

The same way, most users of Linux trust the CA root certificates provided by their distribution, it is very likely, that user would trust the CA root certificates provided by a package included in the Go standard library.

Additionally, the possibility to include the CA root certificates during build time, without altering the source code, is not possible with a 3rd party package but only if this package is included into the Go standard library and the build tag is implemented in to Go tool chain.

Update of the CA Root Certificates

The CA Root Certificates included in the standard library are updated with every release of Go (with the current schedule every 6 months). This would work the same way as it currently does for the package time/tzdata. The update frequency of the Included CA Certificate List is roughly every few months (2020: 5 times, 2019: 4 times, according to curl ca extract), which seems to be similar to the update frequency of the time zone data information.

In regards to updating the CA root certificates compiled into a Go binary, the same limitations apply as for the time/tzdata package. The information compiled into a binary is not updated. That being said, for the situations, this package is intended for, it is still an improvement because containers built FROM scratch are also not updated by default and out of date / not updatable systems obviously also do not get updates for the CA root certificates.

Annex

There is a sample implementation of this approach at github.com/breml/rootcerts with some additional reasoning about when to use such a package and what to keep in mind.

This proposal as well as the sample implementation are highly influenced by the proposal #38017 - time/tzdata and the implementation of the package time/tzdata by @ianlancetaylor

cc: @FiloSottile, @katiehockman, @mvdan

@mvdan
Copy link
Member

mvdan commented Jan 27, 2021

At least from my personal experience, I've used scratch images quite a lot for deployments of "pure Go" services, and I needed ca-certificates more often than I needed tzdata. They both fit the same bill in terms of system files which the standard library might depend on, depending on what the program is doing. A significant amount of what I've written in the past ends up needing ca-certificates in the end (e.g. because of outgoing HTTPS requests), so it's almost a default that I make sure it's available beforehand.

This is all to say - if tzdata was included, I think rootcerts should too, given how I think it's at least just as important.

@seankhliao seankhliao changed the title crypto/x509: add crypto/x509/rootcerts package and rootcerts tag to embed CA root certificates in program proposal: crypto/x509: add crypto/x509/rootcerts package and rootcerts tag to embed CA root certificates in program Jan 27, 2021
@gopherbot gopherbot added this to the Proposal milestone Jan 27, 2021
@seankhliao
Copy link
Member

examples of other third party packges that provide this:
https://github.com/certifi/gocertifi
https://github.com/gwatts/rootcerts
https://github.com/alexflint/stdroots

I'm not particularly convinced that having it in std is a good idea, since out of date tzdata is usually just a minor bug but out of date ca certs is a security issue. Maybe somewhere under x/crypto.

@mvdan
Copy link
Member

mvdan commented Jan 27, 2021

I agree with what the proposal says, though. The current solutions people are using are also potentially vulnerable to not getting new CA roots for a long time. This includes cases where the binary runs on a system that's simply not being updated regularly.

At the end of the day, it's up to the developer to keep up to date with Go versions and rebuild/redeploy their binaries as needed. If that maintenance is not happening, the result is the same whether or not this proposal is accepted and used by them.

@breml
Copy link
Contributor Author

breml commented Jan 27, 2021

examples of other third party packges that provide this:
https://github.com/certifi/gocertifi
https://github.com/gwatts/rootcerts
https://github.com/alexflint/stdroots

One small difference between https://github.com/breml/rootcerts and the other 3rd party packages mentioned above is, that it can be used in a "none intrusive" way, because no application code needs to be changed. A simple blank import (import _ "crypto/x509/rootcerts") in package main is enough.

This is implemented the same way it is done in time/tzdata (see: https://github.com/breml/rootcerts/blob/master/rootcerts.go#L28-L34).

@ianlancetaylor ianlancetaylor added the Proposal-Crypto Proposal related to crypto packages or other security issues label Jan 27, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 27, 2021
@rolandshoemaker
Copy link
Member

I think there are two interesting parts to this proposal: implementing an API to set the default root pool, and adding a standard library package that contains some set of roots. While I am generally in favor of the first part, I am somewhat opposed to the second.

I think adding an API to crypto/x509 that allows setting the default root pool (rather than always using the system pool when no roots are specified) could be quite valuable, and would more cleanly and safely provide the functionality you (and some other existing packages) are getting with go:linkname tricks. I think the major downside of adding this kind of functionality though is allowing the user(/developer) to understand where their root pool is being set, and opens up the rabbit hole of dependencies setting a malicious set of roots (although it could be argued that this is already the case with the go:linkname trick, so we'd just be adding a more explicit easy to understand API to replace this inherently unsafe approach).

Speaking as a member of the security team, who would likely end up being responsible for maintaining it, I'm hesitant to implement a root store in the standard library. Even if we are just copying some existing root program, there are a number of somewhat complicated policy enforcement decisions we'd become embroiled in. I'm also not sure what the update process for the store would be, for instance if a root was to become distrusted in the NSS program, would we need to issue a security release of Go in order to provide an updated root store to users? I think if we were to do something like this it'd most likely need to live in a x/ repository as a module so that updates could be made in a less rigorous cadence. (Side note: just using the NSS trust store is not as simple on its face as it seems, while it is possible to extract a list of 'trusted' certificates there are somewhat complex policies around what certificates may be used for what, and a chunk of policy that is implemented in code, meaning that just pulling their list and taking it at face value is likely to result in trusting things we may not always want to trust.)

@breml
Copy link
Contributor Author

breml commented Jan 30, 2021

@rolandshoemaker Thanks for your thoughtful feedback.

The main intend of my proposal is a standard library package that contains some set of roots.

What I care about is, a simple, save and trusted way to embed a trusted set of root certificates into a Go binary.
I more than once observed inexperienced developers handling the missing ca-certificates.crt problem in awful ways (e.g. committing the file together with the code, where it gets forgotten and therefore is never updated). To be able to import the root certs in the same simple and straight forward way as the time zone data is very compelling.

Go positions it self as the language of choice for the cloud and cloud native applications. To cite @spf13 (Steve Francia in https://www.infoq.com/articles/go-language-13-years/):

Go’s strength is still the cloud/server applications that Go is such a good fit for, ...

The second, much more significant phase, will be the industry shifting to take advantage of the unique cloud offerings, increasingly moving to cloud-native application development. In these cases, Go is the clear choice.

Lots of these cloud native applications will run in container environments and lots of them will need to talk to external APIs, which then will need root certificates to establish the secure connection. There I see a clear benefit in the Go standard library providing a simple way of including the CA root certificates into the Go binary. It is maybe a little bit of a stretch, but if a Go application is run in a container FROM scratch, then the Go application sort of becomes the operating system and operating systems normally provide a set of root certificates.

I clearly understand your concern for you as a member of the security team and I completely agree, that is likely your team, who will end up being responsible for the maintenance. But if we look at this from users point of view, then we are definitively in a better place, when the Go security team takes care of this, than if a random open source contributor publishes a package with this functionality. If this proposal gets accepted, I will happily remove/deprecate my package.
With the still rapid growing number of Go developers around the world I feel the maintenance of these root certificates inside of the Go standard library is worth the effort to increase the overall security and trust of the Go and especially the cloud native / containerized eco system.

For the update process as well as the location of such a package, it is my belief, that the package should life in the standard library (and not in x/) for the following reasons:

  • My observation in projects in the industry is, that updating the Go tool chain (and with this the standard library) is a well understood process. Updating the Go tool chain is (due to the great work of the Go team and the compatibility guarantee) a friction less process, which is often even automated.
  • In contrary, the updating of Go modules is unfortunately sort of a forgotten and less understood topic. I often observe, that the dependencies are only updated, when this is necessary due to functional requirements. The minimal version selection does not help here and I fear that a root certificates package in x/ will get way less often updated than the same package in the standard library.
  • It is already possible to day to ship minor updates for Go and it is done quite often. E.g. for Go 1.15 we currently are at 1.15.7, which makes more than a minor release per month. So I do not really see a problem to ship a new minor release, if there is a certificate, that needs to become distrusted.

In regards to the NSS trust, I did not go into full details here, but it is my observation, that this problem has relaxed over the last couple years. On one hand, all the mentioned Linux distributions have solved this problem one way or the other, so this is a pretty well understood problem with battle proved solutions available. On the other hand does Mozilla provide the PEM of Root Certificates in Mozilla's Root Store with the Websites (TLS/SSL) Trust Bit Enabled (see: CA/Included Certificates). In my understanding, this is already the correct list of certificates with the correct trust bits enabled.

The other part you have identified in my proposal (API to set the default root pool) is less relevant for the use cases I am outlining in the proposal, for the following reasons:

  1. On Unix/Linux based systems, Go already today offers the possibility to add additional certificates to the default pool by setting the respective environment variables.
  2. There are other proposals covering somewhat related aspects:
    1. crypto/x509: add ability to reload root certificates crypto/x509: add ability to reload root certificates #41888
    2. crypto/x509: Convenient, efficient way to reload a certificate pool after on-disk changes: crypto/x509: Convenient, efficient way to reload a certificate pool after on-disk changes #35887
  3. For me, the way the system roots are loaded is not important. The go:linkname trick has been bluntly copied from time/tzdata, so nothing I invented myself.

@rsc
Copy link
Contributor

rsc commented Aug 10, 2022

How often do root sets change?
How quickly would one go stale in an old Go distribution?
How many different ones are there?
Is there an obvious one that we could pin ourselves to, like we pin to the standard timezone database.

@seankhliao
Copy link
Member

Chrome's new initiative to maintain its own root store may be of interest https://www.chromium.org/Home/chromium-security/root-ca-policy/

@joeshaw
Copy link
Contributor

joeshaw commented Aug 10, 2022

How often do root sets change?

curl maintains a bundle of the Mozilla root certs: https://curl.se/docs/caextract.html

It may not be comprehensive but there were 5 updates in 2021 and 5 so far in 2022.

How many different ones are there?

The big ones are Mozilla, Google, Microsoft and Apple. There may be additional ones.

Is there an obvious one that we could pin ourselves to, like we pin to the standard timezone database.

The Mozilla one is the best candidate here. Their policy is documented here and they have an open process for inclusion. Most open source software and Linux distributions use the Mozilla store.

@breml
Copy link
Contributor Author

breml commented Aug 10, 2022

My "proof of concept" (https://github.com/breml/rootcerts), mentioned above, does use the root certificates from Mozilla, similar to the way it is done for curl.

This root set does contain 146 certificates as of now. I quickly analyzed the list from Included CA Certificates (CSV) from https://wiki.mozilla.org/CA/Included_Certificates (with the email only certificates filtered out). The average validity of these certificates is 23.5 years (with a minimum of 9 years and a maximum of 35 years).

So the number of certificates that are needed to be updated will average in the range given by @joeshaw (~6 year).

In contrast to tzdata updates, this seams to be in a similar range (see: http://mm.icann.org/pipermail/tz-announce/):

2022 ytd: 1
2021: 5
2020: 6
2019: 3
2018: 9
2017: 3
2016: 10
2015: 7
2014: 10
2013: 9

@rsc
Copy link
Contributor

rsc commented Aug 12, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Aug 17, 2022

/cc @golang/security

@rsc
Copy link
Contributor

rsc commented Aug 17, 2022

One reason this would be interesting to resolve is that currently "single-Go-binary containers" cannot make HTTPS connections. You have to side-load the root set or derive the container from a full Linux distribution. If we could optionally put the root set into the binary, that would make single-binary containers useful for more programs.

@FiloSottile
Copy link
Contributor

I am conflicted about this.

I agree it makes single-binary deployments more convenient, and it feels like a natural feature to provide.

On the other hand

  1. root stores are moving more and more away from being a list of homogeneous trusted anchors, and towards rich metadata that is only expressed in the code of the verifier they ship with (name constraints, time validity, CT requirements...) which is why we invested so much last year in moving to the platform verifier
  2. root stores are aggressively moving towards higher root agility, meaning that past numbers on the frequency of root changes are not indicative of the next five years, and we'd be committing to maintaining this pretty much forever
  3. distrust changes MUST be shipped as security releases (this is a whole debate in the Linux distribution world, and we don't want to become the next chapter of the problem of slow to update root stores), which will increase the security release load
  4. it's not clear what store we should ship: the canonical answer is the Mozilla one, and it's not a bad answer; the Chrome one might be interesting too, but I'm not sure they would want us to decouple it from the verifier
  5. trust is fundamentally a platform concern (which Windows and macOS and iOS understand) and the correct answer to this is arguably for container runtimes to synthesize an /etc/ssl/roots.pem file the same way the synthesize /etc/resolv.conf

This might make more sense as an auto-updated x/crypto package, which then would encourage adding x509.SetDefaultRoots() but I don't love the semantics of a runtime-changable singleton like that. I can see libraries arbitrarily using it in init() leading to conflicts and unexpected behavior that is hard to debug.

@qdm12
Copy link

qdm12 commented Aug 17, 2022

From my narrowed github user point of view, having it as a module dependency together with dependabot is nice, so I'm alerted when it gets outdated. If we want it as part of x/crypto it would make tooling such as dependabot not alert us for outdated CAs.

@rolandshoemaker
Copy link
Member

I agree with all of the points @FiloSottile makes (unsurprisingly.) This is really a platform problem, and in an ideal world platforms would solve it for us. That said this has been a problem for a while, and as far as I can tell, there has been no real movement towards a real platform fix for containers and such. It is probably better that we provide at least some centralized solution for now, in order to dissuade people from doing the less ideal runtime hacks that are described further up this thread.

I think if we are to go down this path it should absolutely be a separate x/crypto (or perhaps a completely new x/) module, which simply provides a pre-populated x509.CertPool. This would let us decouple any urgent updates from the Go release cadence (and if done outside of x/crypto any other changes people may be less interested in pulling down). Auto-updating the module would also allow us to embed some sort of auto-destruct staleness check, preventing usage of the module if it is too out of date.

Where the list comes from is a complicated question, Mozilla is almost definitely the easiest solution, and has a pretty good history of not being full of terrible stuff. There are some not-chrome-but-adjacent Google folks working on related stuff who it might be worth talking to (I'll need to trawl my inbox to try to remember who exactly to ping.)

I agree that the prospect of adding a SetDefaultRoots type API is not ideal, and will likely result in some people setting roots in places they probably shouldn't, but this can essentially already happen with runtime linking, without a clear API around the behavior of multiple modules attempting to change the default pool. At least with a clear public API we can document the expected behavior, i.e. only allowing a single call to the method, panicking otherwise ¯_(ツ)_/¯.

@breml
Copy link
Contributor Author

breml commented Aug 18, 2022

From my narrowed github user point of view, having it as a module dependency together with dependabot is nice, so I'm alerted when it gets outdated. If we want it as part of x/crypto it would make tooling such as dependabot not alert us for outdated CAs.

If I understand this correctly, this is related to the fact, that there are no tagged releases for x/crypto, correct? For me, this raises the following questions:

  • Is it planned to add version tags to x/crypto in the future (if not, why)?
  • Does none of the x/ modules have version tags? Would it be possible to have a x/ package for the certificates, that has version tags (to allow automated tooling to track updates)?

@rsc
Copy link
Contributor

rsc commented Aug 31, 2022

It sounds like we may want to do something to nudge people's behavior here toward things we'd prefer.

It also sounds like we could add a very tiny hook in crypto/tls (Roland said SetDefaultRoots) which can be called just once (twice is a panic), and then the actual cert sets can be provided out of standard library by x/crypto, which is easier to update. Do I have that general sketch right?

Separately, Filippo said that verifiers are starting to add different kinds of new logic. So is the "pluggable root set" the right approach or is it "pluggable ____" where we still need to fill in the blank? Maybe a verifier function or interface makes more sense than a raw CertPool?

@rsc
Copy link
Contributor

rsc commented Sep 7, 2022

/cc @rolandshoemaker

@FiloSottile
Copy link
Contributor

Yeah, that's a good call, maybe this should be SetDefaultVerifier(func(c *Certificate, opts VerifyOptions) (chains [][]*Certificate, err error)) which reflects better the shape of the default "root set" (actually, the platform verifier API) on non-Linux OSes.

Only two three concerns:

  1. Packages setting it unconditionally, including on macOS/Windows where it's unnecessary, and getting a strictly worse verifier (less compatible, and less secure). This is mitigated by only making our x/crypto implementation do anything on non-macOS/Windows systems.
  2. If this is called, is there still a way to access the actual default verifier? For example an application might want to extend the default verifier by wrapping it.
  3. This is generally spooky action at a distance and it would be confusing if it caused issues because it can't be detected by reviewing the verification callsite, but on balance it's probably unavoidable and worth it.

I guess if this replaces the default verifier, then it becomes the behavior attached to the SystemCertPool, and SystemCertPool + additional roots becomes "run this and then run a separate verification with the additional roots. I'm ok with it even if SystemCertPool becomes a bit of a misnomer.

@rolandshoemaker
Copy link
Member

SetDefaultVerifier is an interesting idea, it would definitely allow creating a verifier that more closely matches the logic of some of the existing root programs (wherein trust decisions are often defined in code, rather than consumable configs). As it is a significantly more powerful API though, it obviously creates significantly more trapdoors to subtly break the certificate verification process (either via malicious action or not.)

I'll pushback a little on this idea though. Most of the additional logic that Mozilla adds on top of their list is restrictions on various borderline roots, where the CA has done something bad but because of wide legacy deployment, they feel the need to maintain trust for it in very specific circumstances. Is this something we need to support? Possibly, but we could take on significantly less complexity by simply trimming these roots out and providing binary trust decisions.

I'm not super strongly opinionated, I tend to lean towards less complexity, but I agree that providing a way of modifying the default system verifier would definitely be extremely powerful, allowing some interesting behaviors down the road.

@breml
Copy link
Contributor Author

breml commented Sep 12, 2022

My understanding so far is, that in my initial proposal, one may enable the default roots by simply adding import _ crypto/x509/rootcerts (or providing the build flag -rootcerts), like this:

package main

import (
	_ "crypto/x509/rootcerts"
	"log"
	"net/http"
)

func main() {
	res, err := http.Get("https://www.google.com/robots.txt")
	if err != nil {
		log.Fatal(err)
	}

	log.Infof("%d", res.StatusCode)
}

With SetDefaultRoots, I imagine the code to look like this:

package main

import (
	"crypto/x509"
	"log"
	"net/http"
)

func init() {
	x509.SetDefaultRoots()
}

func main() {
	res, err := http.Get("https://www.google.com/robots.txt")
	if err != nil {
		log.Fatal(err)
	}

	log.Infof("%d", res.StatusCode)
}

I am a little bit lost on the SetDefaultVerifier suggestion. How would this exactly be used?

Additionally, I see the following issue: If we have a very thin hook in crypto/x509 to set the default root certs and the root certs themselves are located in a different package (e.g. in x/crypto), the root certs would always be included into the final binary, as soon as there is a dependency to the crypto/x509 package. Or will the compiler be able to detect, that SetDefaultRoots is not called and therefore the binary data for the root certs is nowhere referenced and can therefore be striped from the final binary?
In the original proposal, the decision, if the root certs should be included into the binary is left to the the user.

@stapelberg
Copy link
Contributor

How are we doing on this discussion? Do people agree with the most recent API proposal in #43958 (comment) ?

Yes, the proposed rootcerts.FetchRootsAsPEM API would work for me AFAICT :)

@rolandshoemaker
Copy link
Member

Yup, I think this is pretty much what we've settled on.

@breml
Copy link
Contributor Author

breml commented Nov 3, 2022

Yes, I agree with #43958 (comment).

The only thing that I still request to include is the support for SSL_ROOT_FALLBACK environment variable to force the usage of the embedded root certificates (e.g. on a system with outdated root certificates).

@rsc
Copy link
Contributor

rsc commented Nov 8, 2022

Is SSL_ROOT_FALLBACK a standard environment variable in other non-Go systems? If not, we should probably use a variable with a GO prefix, and probably just a setting in the GODEBUG variable.

@rsc
Copy link
Contributor

rsc commented Nov 8, 2022

I am not sure that golang.org/x/rootcerts/bundle is the right import path.

First, I don't think we want to maintain a whole repo just for root certs.
Probably we should use an x/crypto submodule if we need to separate them.

Second, for a package that is imported purely for side effects,
"bundle" does not really indicate what those side effects are.

Since the role of the package is to register with x509.SetFallbackRoots,
perhaps the package should be

import _ "golang.org/x/crypto/x509roots/fallback"

@stapelberg
Copy link
Contributor

I like the fallback name! Definitely more descriptive than bundle :)

@breml
Copy link
Contributor Author

breml commented Nov 9, 2022

Is SSL_ROOT_FALLBACK a standard environment variable in other non-Go systems? If not, we should probably use a variable with a GO prefix, and probably just a setting in the GODEBUG variable.

No, SSL_ROOT_FALLBACK is not a standard environment variable as far as I know. For me, a GODEBUG variable is fine as well, as long as the forced usage of the fallback root certs can be controlled from the outside (at start time of the application).

First, I don't think we want to maintain a whole repo just for root certs.
Probably we should use an x/crypto submodule if we need to separate them.

For me the important part is not about having separate repositories (this can be decided based on e.g. maintenance effort per repo), the important thing is, that want to be able to version the golang.org/x/crypto/x509roots independently from golang.org/x/crypto in order to leverage govulncheck and dependabot whenever there is a new set of roots available. It is my understanding, that this is perfectly possible with submodules and hierarchical git tags. So for me, golang.org/x/crypto/x509roots for the submodule and golang.org/x/crypto/x509roots/fallback for the automated registering of the roots is fine.
Speaking of the package name, what about golang.org/x/crypto/x509/roots and golang.org/x/crypto/x509/roots/fallback? This would work as well, wouldn't it?

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

Sounds like other than the package name people are happy with this proposal.

@rolandshoemaker
Copy link
Member

+1 for using a GODEBUG variable to force the fallback.

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@gopherbot
Copy link

Change https://go.dev/cl/449235 mentions this issue: crypto/x509: implement SetFallbackRoots

@FiloSottile
Copy link
Contributor

I don't want to get in the way of this getting accepted before the freeze, and this is a minor note, but after looking at the implementation I would rather not have the force fallback GODEBUG.

On macOS and Windows, the fallback would normally not be reachable, because the OS verifier is always there. What does x509usefallbackroots do on those platforms? Does it make the fallback available anyway?

What happens if x509usefallbackroots is set but SetFallbackRoots was not called? An error? A panic? Is it ignored?

SSL_CERT_FILE=/dev/null SSL_CERT_DIR=/dev/null is certainly not the prettiest, but at least it has very clear semantics: it only applies to Linux because SSL_CERT_FILE/DIR do, and if there are no fallback roots you get an error.

@breml
Copy link
Contributor Author

breml commented Nov 13, 2022

On macOS and Windows, the fallback would normally not be reachable, because the OS verifier is always there. What does x509usefallbackroots do on those platforms? Does it make the fallback available anyway?

In my opinion, the GODEBUG variable should force the usage of the embedded certificates regardless of the operating system. There exist very old installations of especially Windows systems in the wild, where the root certificates provided by the system might be outdated as well. In such a case, the GODEBUG force fallback should allow to force the respective Go application to use the embedded certificates regardless of the fact, that a Windows system always provides some set of root certificates via the OS verifier. For me, the GODEBUG option indicates that the user wants to use the embedded certificates in any case.

What happens if x509usefallbackroots is set but SetFallbackRoots was not called? An error? A panic? Is it ignored?

I am fine with the GODEBUG x509usefallbackroots getting ignored, if no fallback roots are set.
If we decide to go with the panic, it is important in my opinion, that this panic is happening during initialization of the application. I don't like an application to panic only after some time, when a certificate is actually used (imaging a long running service, which occasionally queries an API via HTTPS and then only fails when the first call to the API is executed).
I have not looked into the implementation, so I can not judge, if an error is a viable solution.

SSL_CERT_FILE=/dev/null SSL_CERT_DIR=/dev/null is certainly not the prettiest, but at least it has very clear semantics: it only applies to Linux because SSL_CERT_FILE/DIR do, and if there are no fallback roots you get an error.

Again, I don't think the fallback certificates should be limited to *nix OS.

@rsc
Copy link
Contributor

rsc commented Nov 16, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: crypto/x509: add crypto/x509/rootcerts package and rootcerts tag to embed CA root certificates in program crypto/x509: add crypto/x509/rootcerts package and rootcerts tag to embed CA root certificates in program Nov 16, 2022
@rsc rsc modified the milestones: Proposal, Backlog Nov 16, 2022
@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 17, 2022

I've been mostly sitting on the fence about the GODEBUG behavior, but I think we need to make a final decision and I'm going to propose the following (which is, essentially, what is implemented in my CL):

  • When calling SetFallbackRoots with GODEBUG= x509usefallbackroots=0, if the system pool is empty it will be replaced with the fallback pool. On Windows and macOS we will inherently never fallback, since we don't know anything about the content of the system pool, as we use platform APIs for verification (so we essentially always consider the pool non-empty.)
  • When calling SetFallbackRoots with GODEBUG= x509usefallbackroots=1, the system root pool will be replaced regardless of its content or the platform. On macOS and Windows this will inherently disable use of the platform verification APIs since the special system pool will be replaced with a generic pool (we could also create a hybrid pool, with would result in parallel verification both with the fallback pool, and the platform APIs, but I think this is more confusing.)
  • If SetFallbackRoots is not called with GODEBUG= x509usefallbackroots=1, SystemCertPool acts as if a fallback pool was never set, and behaves normally.

This gives us (mostly) uniform behavior across platforms, and is the most obvious behavior. Given this is a GODEBUG flag, even though I'm sure people will end up relying on the semantics of this behavior in strange unexpected ways, we'll have a little bit more leeway to change it in the future in case we realize down the road this is wrong.

@andig
Copy link
Contributor

andig commented Nov 17, 2022

I like the fallback name! Definitely more descriptive than bundle :)

I was gonna ask if OS certs would be used if present. That answers it- yes.
I really like this change. One of hour top support issue with Docker is people mounting anything into /etc and then complaining about apparently unrelated networking errors. I assume this change should fix the situation.

@pete-woods
Copy link

I'm not 100% sure I follow all the discussions, but one thing in particular we (at CircleCI) would like out of this (besides the FROM scratch Docker scenario) is to force the Go based certificate store, even when os-provided certificates are present. E.g. someone runs our software in an old Docker image with an out of date set of rootcerts in.

gopherbot pushed a commit that referenced this issue Nov 18, 2022
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>
@breml
Copy link
Contributor Author

breml commented Nov 19, 2022

Hi @pete-woods

I'm not 100% sure I follow all the discussions, but one thing in particular we (at CircleCI) would like out of this (besides the FROM scratch Docker scenario) is to force the Go based certificate store, even when os-provided certificates are present. E.g. someone runs our software in an old Docker image with an out of date set of rootcerts in.

#43958 (comment) summarizes how this will work. So I guess, your use case is (only) partly solved. Whenever a user needs to force the Go embedded certificates (e.g. because the os-provided roots are out of date), he will need to pass the GODEBUG= x509usefallbackroots=1 environment variable.
I pushed for a feature like the one you are requesting, but I had no support, so this was not included.

@breml
Copy link
Contributor Author

breml commented Nov 19, 2022

@pete-woods Thinking of it a little bit more, there might be a way of setting the environment variable from code in an import, that is loaded before SetFallbackRoots is called (similar to how it is done in the unit tests in 04d6aa6#diff-aab8875f722653c06a57f1a4509bdad43391f1e2dc9555644a56beeaf96bb808R91).

@rsc
Copy link
Contributor

rsc commented Dec 2, 2022

The crypto/x509 API has landed and should be in Go 1.20. For the purposes of release notes we have to say what the supporting package will be. Roland and I talked and agreed on golang.org/x/crypto/x509roots/fallback.

@rsc rsc changed the title crypto/x509: add crypto/x509/rootcerts package and rootcerts tag to embed CA root certificates in program crypto/x509: add SetFallbackRoots and golang.org/x/crypto/x509roots/fallback package Dec 2, 2022
@prattmic prattmic mentioned this issue Dec 6, 2022
428 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues Proposal-FinalCommentPeriod
Projects
Status: Accepted
Development

No branches or pull requests