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

external data TLS should not depend on caBundle if we can use system verifier #2452

Closed
sozercan opened this issue Dec 12, 2022 · 9 comments
Closed
Labels

Comments

@sozercan
Copy link
Member

sozercan commented Dec 12, 2022

Describe the solution you'd like
from @enj:

I do not understand why this is a requirement. Normally when making a TLS connection with no pre-configured CA bundle, the system verifier is used (by leaving tls.Config.RootCAs as nil).
open-policy-agent/frameworks#270 (comment)

This is a scenario where a real-signed certs can be used (for example, with Let's Encrypt) and the system verifier can validate

@maxsmythe
Copy link
Contributor

@enj any docs for system verifier?

@enj
Copy link

enj commented Dec 12, 2022

@maxsmythe the x509.Certificate.Verify code is the best way to understand what will happen on different configs based on the OS, the docs are open ended because of the large number of branches:

// If opts.Roots is nil, the platform verifier might be used, and
// verification details might differ from what is described below. If system
// roots are unavailable the returned error will be of type SystemRootsError.

Generally speaking, leaving tls.Config.RootCAs as nil is safe when connecting to a web server unless you have explicit knowledge that the TLS connection needs to use a key from a particular CA (i.e. your browser is not checking that github.com is pinned to some particular CA, just that the platform verifier/system root CAs is happy with it). This is one of the reasons that even distroless images contain the web PKI roots - without them too many things break.

@maxsmythe
Copy link
Contributor

Oh gotcha, I thought "system verifier" was some K8s-specific thing for verifying workloads (similar to Istio).

Sounds like you're saying "use default trusted system certs"

I suppose I don't have anything against using OS-trusted-certs in principle, since root CAs would need to be compromised for it to be risky.

Is there any risk of users finding this behavior unexpected? What behavior does the VWH system have for this?

@enj
Copy link

enj commented Dec 13, 2022

Using system roots by default is what I have traditionally seen, VWHs have that behavior as well.

@stale
Copy link

stale bot commented Feb 11, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 11, 2023
@ritazh ritazh removed the stale label Feb 13, 2023
@stale
Copy link

stale bot commented Apr 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 17, 2023
@sozercan sozercan removed the stale label Apr 25, 2023
@stale
Copy link

stale bot commented Jun 24, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 24, 2023
@maxsmythe
Copy link
Contributor

still interesting

@stale stale bot removed the stale label Jun 26, 2023
@stale
Copy link

stale bot commented Aug 29, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 29, 2023
@stale stale bot closed this as completed Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants