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

envoyconfig: fallback to global custom ca when no policy ca is defined #2235

Merged
merged 3 commits into from
May 28, 2021

Conversation

calebdoxsey
Copy link
Contributor

Summary

Fallback to the custom ca defined in the global options when no policy ca is defined.

There's a non-trivial chance this may break existing users, since previously we only validated internal connections between pomerium services using this certificate authority, and now we will be validating all upstream services.

Related issues

Fixes https://github.com/pomerium/internal/issues/417

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@calebdoxsey calebdoxsey added the bug Something isn't working label May 24, 2021
@calebdoxsey calebdoxsey requested a review from a team as a code owner May 24, 2021 15:33
@calebdoxsey calebdoxsey requested a review from wasaga May 24, 2021 15:33
@codeclimate
Copy link

codeclimate bot commented May 24, 2021

Code Climate has analyzed commit f5ffc32 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@wasaga wasaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code LGTM.

  • Please add an explicit warning log entry during that route X will have global CA applied, so that users who run into upstream unavailability could figure out why without reading docs.

Copy link
Contributor

@travisgroth travisgroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with @desimone yesterday and augmenting the system cert pool rather than replacing it makes the most sense for this option. That's how the internal golang grpc and http clients currently function.

Doing something like

func GetCertPool(ca, caFile string) (*x509.CertPool, error) {
for building the envoy upstream tls context would be the desired behavior.

@calebdoxsey
Copy link
Contributor Author

We can't use that function. It has no way of serializing back to PEM.

I guess I'll concatenate the PEM files, though I don't know enough about the format to know if that actually works or not.

@calebdoxsey calebdoxsey merged commit 9b61d04 into master May 28, 2021
@calebdoxsey calebdoxsey deleted the cdoxsey/417-ca branch May 28, 2021 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants