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: Support ingress rule matching for bastion mode #1244

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shayonj
Copy link

@shayonj shayonj commented May 10, 2024

Context

👋🏾 Hello from Tines! I work on the Platform Team, and we're big fans and heavy users of CF Tunnel. In a nutshell, we use Tunnel in our multi-tenant environment to proxy our customers' HTTP workloads from our cloud instances to their on-prem/self-hosted instances through Cloudflared Tunnel. We've built a small orchestration layer on top of the Tunnel that dynamically proxies our customers' requests from our HTTP client middleware via cloudflared access (entry node) to the customers' network (exit node).

Our setup operates in bastion mode, meaning the proxied request has a single destination.

Problem

We are looking to eliminate any possibility of "data exfiltration." As you might guess, the exit node where the cloudflared tunnel runs can access virtually any service within its network boundary.

Tunnel Ingress Rules is an excellent feature that allows traffic routing to different services based on hostname. It also enables "blackhole-ing" incoming requests that do not match a certain hostname by declaring http_status:404 as the service, for example.

However, ingress rules do not work with bastionMode. We could avoid using bastion mode, but given the scale (many multi tenant clusters) and dynamism of how we run Tunnels, this would require us to provision a new hostname/DNS per Tunnel in a multi-tenant env to leverage ingress rules in non-bastion mode, further necessitating some UX/DX changes.

Proposal

My proposal is to extend the existing functionality of bastionMode. If bastionMode is set to true, and a hostname and service name are provided as part of the ingress rules config, then we attempt to match the hostname against the header Cf-Access-Jump-Destination in the incoming HTTP request. If the Rule matches, then we accordingly proxy the bastion request to the mentioned service. If a request does not match any service, then it falls into the usual catch-all behavior.

I have tested this setup in our environment, and it works as expected. That said, I'd like to propose this as a feature request for the project. I would also love to learn if there are opportunities to improve this approach or consider other options.

Given our reliance on Tunnel, we would also be happy to discuss what continued support and maintenance would look like.

Example config

tunnel: aaaa-bbbb-cccc

ingress:
  - hostname: test-foobar-service.com
    service: http_status:200
  - hostname: "foobar.tunnel-dev.com"
    service: "http://localhost:9000"
  - hostname: "foobar1.tunnel-dev.com"
    service: "http://localhost:9002"
    originRequest:
      bastionMode: true
  - service: http_status:404

#1243

@@ -28,7 +28,6 @@ var (
)

const (
ServiceBastion = "bastion"
Copy link
Author

Choose a reason for hiding this comment

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

Consolidate the use via config.BastionFlag and also to avoid import cycle not allowed in test which got raised when I was referencing ingress from carrier

default:
return fmt.Errorf("Unrecognized service: %s, %t", rule.Service, originProxy)
}
return nil
}
Copy link
Author

Choose a reason for hiding this comment

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

extracted the logic into handleHTTPBasedService and handleStreamBasedService

@shayonj
Copy link
Author

shayonj commented May 14, 2024

👋🏾 Apologies for the direct ping @chungthuang - I was curious if you were able to share any insight into the timeline around when can I expect some feedback on this? Just trying to decide accordingly if I could wait on a release or proceed with a temporary fork for now. Thank you so much for your work 🙏🏾

@shayonj
Copy link
Author

shayonj commented May 21, 2024

@jcsf @DevinCarr if you are able to help take a look at the PR and/or share any insights into the review process that would be great 🙏🏾

@janani-cr
Copy link

@shayonj Thanks for your contribution, we'll review this PR in our next meeting which will most likely not align with your timeline, hence our recommendation would be for you to do a fork to carry on with your work.

@shayonj
Copy link
Author

shayonj commented May 21, 2024

Thank you so much @janani-cr, really appreciate it 🙏🏾

@shayonj
Copy link
Author

shayonj commented Jul 8, 2024

👋🏾 Hi @janani-cr - just curious if you are able to share any insight into by when we can expect a review on the PR? Thank you

@shayonj
Copy link
Author

shayonj commented Sep 17, 2024

👋🏾 Just a friendly ping on the above. Would love to know by when we could receive a review, thank you!

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

Successfully merging this pull request may close these issues.

2 participants