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

proposal: x509.Certificate.Verify should provide an option for both using the host's root CA set and an additional set of user-provided root CAs #34937

Closed
philipatl opened this issue Oct 16, 2019 · 3 comments

Comments

@philipatl
Copy link

@philipatl philipatl commented Oct 16, 2019

(similar thing would apply to tls.Config's RootCAs option)

Currently, you can attempt to do this by getting SystemCertPool() and then adding additional certs to this pool, and then setting this as the Roots member. One problem with this is on Windows, where SystemCertPool() does not work. Even if you try to work around this by implementing your own SystemCertPool for Windows (via CertEnumCertificatesInStore etc), it could be incomplete due to Windows' behavior of adding trusted roots to its store on-demand.

The neat thing is that Go's systemVerify (called when no *CertPool is given) will use the Windows CryptoAPI, which during the verification process will pull in trusted root CAs into the Windows store if they are not there already. With this in mind, changing the capabilities of tls.Config (and x509.VerifyOptions) to take not only a *CertPool but also a flag that indicates if the system roots should be used as well, would give more capabilities to Go programs built for Windows.

This would probably go a long way to ameliorating the issues most Windows people face with SystemCertPool. See issues #16736 and #18609.

@gopherbot gopherbot added this to the Proposal milestone Oct 16, 2019
@gopherbot gopherbot added the Proposal label Oct 16, 2019
@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Dec 2, 2019

By the time you are calling x509.Certificate.Verify, it should be easy to call it twice, once with your roots and once with the system ones. The hard part is getting to x509.Certificate.Verify from crypto/tls, but tls.Config is already overcrowded and we are not adding a knob there.

I agree #16736 is unfortunate, and we should try fixing it again if the OS provides what we need.

Is there any case in which making two calls to x509.Certificate.Verify wouldn't work, aside from crypto/tls without VerifyPeerCertificate?

@philipatl

This comment has been minimized.

Copy link
Author

@philipatl philipatl commented Dec 3, 2019

No, all the pieces were there but I couldn't put it together. But given your link to the VerifyPeerCertificate example, I was able to implement something that will work around my problems (as you say, by calling Verify twice). Thanks.

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Dec 3, 2019

Great, happy to hear the example is useful. I agree this is not very convenient, but the convenient solution (adding a knob to tls.Config) is too heavy-handed, so since there's a workaround, closing.

@FiloSottile FiloSottile closed this Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.