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: Lock Down App Container Ports without Breaking TCP Routes #406

Open
geofffranks opened this issue Apr 29, 2024 · 1 comment
Open

Comments

@geofffranks
Copy link
Contributor

geofffranks commented Apr 29, 2024

Proposed Change

App containers currently have 4 ports exposed to the diego cell's IP and local network:

  • Direct access to the app process's :8080
  • Direct access to the diego-sshd process's :2222
  • Envoy proxied TLS/mTLS that goes to the app process on localhost:8080
  • Envoy proxied SSH that goes to the diego-sshd process on localhost:2222

The containers.proxy.enable_unproxied_port_mappings property on diego-release's rep job will disable the direct access ports from getting NAT rules generated for each container. However this breaks TCP routes, as TCP-router does not know how to configure haproxy for TLS/mTLS traffic.

We would like to add TLS/mTLS support to tcp-router to allow the use of containers.proxy.enable_unproxied_port_mappings with TCP routes. This has the additional benefit of ensuring that all on-the-wire traffic between TCP router and apps is TLS-encrypted.

We would also like CF to adopt this more secure posture once this is resolved, and disable the unproxied ports by default, while providing an ops file to make those ports available if needed.

This will require a number of changes to routing-release, diego-release + cf-deployment:

  1. Update tcp-router to handle TLS based routes. For cross compatibility with diego-release versions and supporting TCP routes registered rom route-registrar which do not have an envoy proxy available, we will adding the backend_tls_port and instance_id fields to the route model to determine how the route should behave.
    • backend_tls_port will determine what port to connect to for TLS traffic - the magic value -1 will be used to indicate that the route is intentionally not using TLS. The magic value 0 will be used to indicate that the route is unintentionally not using TLS. Any other value is the port number to use on the backend. The intentional/unintentional TLS route determination will be used to identify whether operators have opted out of TLS for TCP routes, or if there are older route-registrar/route-emitter processes incapable of using TLS routes, resulting in TCP routes not being encrypted as is likely desired.
    • instance_id should be the container instance GUID, used for route-integrity checking to ensure the tcp-router talks to the correct backend, the same way gorouter ensure handles route-integrity.
  2. Update tcp-router to support TLS route info provided by routing-api to connect to the TLS enabled backends using TLS or mTLS.
    • New properties for the client cert+key + trusted CA will need to be added
    • If certs are present but no CA is present, an error will be thrown during BOSH deployment.
    • If neither cert nor CA is present, no TLS routes will be used, it should fall back to the backend_port field of every route
    • If a CA is present and the backend_tls_port for a route is 0 or negative, use backend_port. Otherwise use the backend_tls_port.
    • if client certs are present, add them to the tcp-router TLS config
    • validate the instance_id field of the route backend matches what was advertised by the backend TLS data
  3. Update routing-api to add the backend_tls_port and instance_id fields to the tcp-route model, and ensure they can be set + fetched properly.
  4. Update route-emitter to supply backend_tls_port and instance_id fields to tcp routes, when tcp.enable_tls is set to true on the route-emitter. This will allow operators to opt out of the TLS overhead on TCP routes if desired. In this case instance_id will be empty, and backend_tls_port will be set to -1.
  5. Update route-registar to supply a -1 value for backend_tls_port and empty instance_id value, as there is no envoy proxy around to handle the TLS termination.
  6. Update cf-deployment:
    • Set rep's containers.proxy.enable_unproxied_port_mappings to false by default
    • Set route-emitter's tcp.enable_tls to true by default
    • Add + plumb the client certs + CA certs for tcp-router + the diego-instance-ca
    • Add ops files to disable TLS for TCP routes, and to re-enable unproxied port mappings.

Backwards Compatibility

  • tcp-router will support both TLS + non-TLS TCP routes.
  • Processes registering TCP routes will supply TLS data when enabled, or -1 for the backend_tls_port if not enabled.
  • routing-api will accept TCP routes with or without TLS data.
  • If a route was not explicitly configured to be non-TLS, warning log messages will be emitted indicating the lack of encryption that was likely desired.
  • During a deployment, the tcp-router + routing-api are updated to add TLS support prior to route-emitter processes. Old routes will continue to emit as normal and be treated as unintentinally not using TLS triggering the above log message. When diego cells update, route-registrar will switch to either intentionally not using TLS to suppress the log messages, or to switch over to using TLS. In three all scenarios the tcp route will continue to function. If route-emitter or route-registrar processes are updated beforetcp-router and routing-api, these routes will continue to behave as they did prior to the upgrade, using non-TLS routing.

Acceptance criteria

  1. cf-deployment enables TLS for TCP routes by default.
  2. cf-deployment disables unproxied container ports by default.
  3. cats passes for http + tcp routing tests
  4. ops files exist to disable TLS on tcp routes as well as to re-enable unproxied container ports.

Related links

No response

@geofffranks geofffranks changed the title Lock Down App Container Ports without Breaking TCP Routes Proposal: Lock Down App Container Ports without Breaking TCP Routes Apr 29, 2024
@geofffranks
Copy link
Contributor Author

Potentially insead of the -1/0 magic values, we can make the field a *uint value, and treat nil as the unintentional non-TLS route, and 0 as an intentional non-TLS route.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

1 participant