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] Hijack traffic using bpf_sk_assign at tproxy_wan_ingress #375

Closed
jschwinger233 opened this issue Dec 20, 2023 · 5 comments · Fixed by #383
Closed

[Proposal] Hijack traffic using bpf_sk_assign at tproxy_wan_ingress #375

jschwinger233 opened this issue Dec 20, 2023 · 5 comments · Fixed by #383
Assignees

Comments

@jschwinger233
Copy link
Member

jschwinger233 commented Dec 20, 2023

Proposal

I traced how TCP hijacking is done, it seems to me that no real tproxy is implemented.
If I understand correctly, consider a dae config with dip(1.1.1.1) -> proxy, curl 1.1.1.1 from host is going through the datapath:

  1. skb reaches eth0, tproxy_wan_egress@eth bpf_redirects the skb to eth0 in inbound direction
  2. tproxy_wan_ingress@eth performs SNAT and DNAT: local_addr:local_port -> 1.1.1.1:80 turns to 1.1.1.1:80 -> proxy_addr:12345
  3. skb is pushed to stack, complete TCP 3-way handshake with dae server.

I was wondering if we can simplify this by using bpf_sk_assign at tproxy_wan_ingress, where we can make sure the skb will be delivered to dae server after pushing to stack, without changing addresses.

Use Cases

No response

Potential Benefits

This can simplify the implementation, make SNAT/DNAT obsolete, and reduce code complexity. Removing these conntrack information and bpf map lookup are likely to improve the performance to some extent.

On the other hand, the return datapath can even benefit more. Currently return packets have to reach eth0 for rev-NAT, this proposal is making that unnecessary: return packets can be routed to source processes in a single-time stack traversal.

Scope

TCP can definitely be affected. UDP can be done in the same way, but doesn't have to be done at same time.

Reference

No response

Implementation

No PR for now. If maintainers see this proposal reasonable and appealing, I can draft a PR for review (in ... weeks)

The implementation shall include at least 2 (3?) parts:

  1. bpf: remove NAT, call skc_lookup_tcp + bpf_sk_assign instead
  2. dae server: make sure the one binding :12345 is a tproxy server (IP_TRANSPARENT)
  3. (optional) benchmark? It would surprise me if performance is even worse after this improvement, but it does happen, I'm happen to drop this proposal.
@dae-prow
Copy link
Contributor

dae-prow bot commented Dec 20, 2023

Thanks for opening this issue!

@jschwinger233
Copy link
Member Author

jschwinger233 commented Dec 20, 2023

Just realized tproxy_lan_ingress indeed is implemented as described above, but not for tproxy_wan_ingress. Is it possible to do the same on wan_ingress? What was the design intention behind these differences?

@jschwinger233
Copy link
Member Author

Relevant commit: 8c65f8f

@mzz2017
Copy link
Contributor

mzz2017 commented Dec 21, 2023

我之前试过使用 bpf_sk_assign,但失败了,所以演变成现在的复杂 dnat。

理论上是可行的,但当时在这上面花费了较多时间,如果你有时间的话可以尝试帮忙推进,非常感谢!

@mzz2017
Copy link
Contributor

mzz2017 commented Dec 21, 2023

我的考虑并不单单是性能,在代码复杂度上使用 bf_assign 也是更优选择。当时似乎在打 mark 之后走 ip rule 上遇到了问题,流量没有返回。我认为这有可能是和 tc 挂载点在出方向的位置有关系。

Vigilans referenced this issue in hack3ric/mimic Mar 31, 2024
Current implementation simply throws away these packets. This commit is the first step of implementing re-sending them after handshake.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants