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.

Sign up for GitHub

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

Jump to bottom

Backport of [NET-8091] Add file-system-certificate config entry for API gateway into release/1.18.x #20982

Closed
wants to merge 1 commit into from
Closed

Backport of [NET-8091] Add file-system-certificate config entry for API gateway into release/1.18.x #20982

wants to merge 1 commit into from

Conversation

hc-github-team-consul-core
Copy link
Collaborator

Backport

This PR is auto-generated from #20873 to be assessed for backporting due to the inclusion of the label backport/1.18.

🚨

Warning automatic cherry-pick of commits failed. If the first commit failed,
you will see a blank no-op commit below. If at least one commit succeeded, you
will see the cherry-picked commits up to, not including, the commit where
the merge conflict occurred.

The person who merged in the original PR is:
@missylbytes
This person should manually cherry-pick the original PR into a new backport PR,
and close this one when the manual backport PR is merged in.

merge conflict error: POST https://api.github.com/repos/hashicorp/consul/merges: 409 Merge conflict []

The below text is copied from the body of the original PR.

Description

Today, inline-certificate is the only supported way of specifying a TLS certificate + private key for an api-gateway. As the name suggests, the certificate + private key values are "inlined" in the configuration entry.

This PR adds a new file-system-certificate configuration entry that instead accepts file paths to a certificate + private key that are accessible in the local file system of the api-gateway at runtime. This functions in the same way that terminating-gateway does today (docs) but adds support for rotation by watching the file system for changes to the referenced certificate + private key.

Ideally, we will be able to add this same support for rotation in terminating-gateway in the future as rotating the certificate and private key today require a restart of the terminating-gateway, which is not ideal (docs).

Testing & Reproduction steps

Testing on VMs

Check out this repo from @nathancoleman

  1. Follow the directions in the README.
  2. You can check the secret at http://localhost:19000/config_dump under dynamic_active_secrets.
image

  1. echo secret_val | base64 -d and you should see the value of cert.crt

  2. Replace the value of cert.crt with this certificate:

-----BEGIN CERTIFICATE-----
MIICnTCCAkOgAwIBAgIRALTzFaRsW70V2hhpw6XdmKEwCgYIKoZIzj0EAwIwgbkx
CzAJBgNVBAYTAlVTMQswCQYDVQQIEwJDQTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNj
bzEaMBgGA1UECRMRMTAxIFNlY29uZCBTdHJlZXQxDjAMBgNVBBETBTk0MTA1MRcw
FQYDVQQKEw5IYXNoaUNvcnAgSW5jLjFAMD4GA1UEAxM3Q29uc3VsIEFnZW50IENB
IDMwNzU0NzMzNjE5NTQ2OTU4MjIxMTI5MjE3NzM2MzUxMzM4MDc0NzAeFw0yNDA0
MDkxODIxNDNaFw0yNTA0MDkxODIxNDNaMBwxGjAYBgNVBAMTEXNlcnZlci5kYzEu
Y29uc3VsMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAESdiYfmA1LURAFLGt59hu
U3UgTMcSAIf3Hl+KYrDRWStjviB4ueemTI+8pXjd8gETXKxp+i5vxIBF9bW95zSb
2KOBxzCBxDAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsG
AQUFBwMCMAwGA1UdEwEB/wQCMAAwKQYDVR0OBCIEIJ32ryqpgyOlctlfqniwLBCt
HosjcftQwK1rjoI6LFnCMCsGA1UdIwQkMCKAICJ9E1X61c/+gkiHJmmiQkVx3g89
MkOYHfJlRoTQlpNpMC0GA1UdEQQmMCSCEXNlcnZlci5kYzEuY29uc3Vsgglsb2Nh
bGhvc3SHBH8AAAEwCgYIKoZIzj0EAwIDSAAwRQIhAKvSL4YDwQPMNgGMibtRTYRs
cHZzR4Vv1JN6D2FDmVYUAiBT+6ySH0bXK9/kFA+RkTbPUwDzfe+qlzs+frK5tqE+
Dw==
-----END CERTIFICATE--
  1. Then check the value of the dynamic_active_secrets again, it should match the above cert.

Testing on K8S

You cannot do the same type of replacement for K8S, so if you would like to test K8S check out the K8S PR. You will mainly verify that the certificate is a file-system-certificate as opposed to an inline-certificate.

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern
Overview of commits

@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Apr 15, 2024
github-team-consul-core-pr-approver
github-team-consul-core-pr-approver approved these changes Apr 15, 2024
View reviewed changes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@github-actions github-actions bot added the theme/envoy/xds Related to Envoy support label Apr 15, 2024
@vercel vercel bot temporarily deployed to Preview – consul April 15, 2024 20:51 Inactive
@missylbytes missylbytes force-pushed the backport/file-system-certificate/formally-top-minnow branch from 6b52198 to bcb9ca2 Compare April 18, 2024 16:40
@missylbytes missylbytes marked this pull request as ready for review April 18, 2024 16:41
@missylbytes missylbytes requested review from a team as code owners April 18, 2024 16:41
@missylbytes missylbytes force-pushed the backport/file-system-certificate/formally-top-minnow branch from bcb9ca2 to 3b857bf Compare April 18, 2024 16:57
@nathancoleman @missylbytes @jkirschner-hashicorp
[NET-8091] Add file-system-certificate config entry for API gateway (#…
859215b 
…20873)

* Define file-system-certificate config entry

* Collect file-system-certificate(s) referenced by api-gateway onto snapshot

* Add file-system-certificate to config entry kind allow lists

* Remove inapplicable validation

This validation makes sense for inline certificates since Consul server is holding the certificate; however, for file system certificates, Consul server never actually sees the certificate.

* Support file-system-certificate as source for listener TLS certificate

* Add more required mappings for the new config entry type

* Construct proper TLS context based on certificate kind

* Add support or SDS in xdscommon

* Remove unused param

* Adds back verification of certs for inline-certificates

* Undo tangential changes to TLS config consumption

* Remove stray curly braces

* Undo some more tangential changes

* Improve function name for generating API gateway secrets

* Add changelog entry

* Update .changelog/20873.txt

Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com>

* Add some nil-checking, remove outdated TODO

* Update test assertions to include file-system-certificate

* Add documentation for file-system-certificate config entry

Add new doc to nav

* Fix grammar mistake

* Rename watchmaps, remove outdated TODO

---------

Co-authored-by: Melisa Griffin <melisa.griffin@hashicorp.com>
Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com>
@missylbytes missylbytes force-pushed the backport/file-system-certificate/formally-top-minnow branch from 3b857bf to 859215b Compare April 18, 2024 17:41
@missylbytes
Copy link
Contributor

This will not be backported to 1.18 at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-team-consul-core-pr-approver github-team-consul-core-pr-approver github-team-consul-core-pr-approver approved these changes

@nathancoleman nathancoleman Awaiting requested review from nathancoleman

Assignees

@missylbytes missylbytes

Labels
theme/api Relating to the HTTP API interface theme/envoy/xds Related to Envoy support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hc-github-team-consul-core @missylbytes @github-team-consul-core-pr-approver @nathancoleman