-
Notifications
You must be signed in to change notification settings - Fork 29
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
SNI filter sni.lua can't filter chrome browser with reassembled TCP segments? #193
Comments
the first TCP segments contains handshake and clienthello and up to extension |
wrapping up our discussion on Matrix.. it might be resolved on kernel 6.9 by this: https://elixir.bootlin.com/linux/v6.9/source/net/core/dev.c#L4914 |
@lneto I tried upstream stable kernel 6.10.11 release, same problem
|
I'm not sure why it's not triggering skb_linearize(), I would need to have a better look at this.. thinking also on a simple hack on Lua side to keep context of the parser.. will try it out later.. |
is there some issue with |
the issue is probably with my understanding of what should actual happen ;-).. I was expecting the following flow.. do_xdp_generic however, I'm not sure why it's not following this.. I need to spend some brain cycles to understand it better =) |
I am learning and I like to understand it better too :). if the |
perhaps this is the guy we want to use.. https://ebpf-docs.dylanreimerink.nl/linux/helper-function/bpf_xdp_get_buff_len/ also, page 15 from https://lpc.events/event/11/contributions/939/attachments/771/1551/xdp-multi-buff.pdf I still need to digest.. but I think it should be the path to follow.. |
precisely.. we have had the same issue with luaxtable.. you might want to take a look.. |
these helpers doesn't seem to be merged in the tree.. UPDATE: sorry, only adjust_data that isn't.. we'll keep digging later.. |
it might be our guy.. https://elixir.bootlin.com/linux/v6.10/source/net/core/filter.c#L4175 |
can you try to add |
a silly question.. you are using
|
yes |
I added |
I added |
I think we could call |
yup, I would print it as well.. |
perhaps we need this flag.. can you try this out? https://ebpf-docs.dylanreimerink.nl/linux/program-type/BPF_PROG_TYPE_XDP/#xdp-fragments |
can you try more info here.. https://lore.kernel.org/netdev/[email protected]/ perhaps we need to use bpf_xdp_load_bytes as well.. |
Btw, what's your NIC? see https://ebpf-docs.dylanreimerink.nl/linux/program-type/BPF_PROG_TYPE_XDP/#__tabbed_1_2 Can you check your MTU config as well? |
I added bpf_printk to print length, and xdp.frags, I can see the printed length, but none of them exceeded MTU 1460 size, all are small, I need to adjust the https.c so I can clearly tell the printed length is from chrome, now there are some other noisy printout that can confuse me.
|
Mine is broadcom and MTU is set to 1500
I can switch to a mini PC which has Intel I211 Gigabit and it uses igb driver, is that ok? since we are using XDP generic skb mode, does it matter which NIC? |
Can you also try to run it on veth? So we can discard the interference of your NIC driver.. |
since the test involves using chrome browser, maybe there is someway to use curl client to simulate chrome browser, I imagine veth would in some kind of container setup in one machine |
Yes, please..
I assumed it doesn't as well.. but I'm not sure regarding the multi-buf support |
I think you can just use |
it might be realistic for me to try |
That's a good question.. I'm not sure.. |
BTW, is |
now I got BPFire in KVM running and the
|
there is noisy data length print and I could not tell which data length printout refer to chrome :), do you have better idea to condition the bpf_printk to maybe only print out for chrome ? |
You can pass them to Lua adding two new |
1500 seems too little, right? TCP length in your example is 1781 bytes, right? Can you try to increase it? let's say..
|
yes I already tried larger MTU, no success |
I think I need to understand and read more about the xdp frags so I know what I am trying :) |
@vincentmli my understanding is that you need
|
thanks! I will try it, I am following the initial XDP multi buffer support and reading all the discussion involved and hasn't finished yet, it takes time but I am getting better understanding :) vincentmli/BPFire#44 |
I followed your suggestion to set the MTU to 4096, and adjusted the code below to print MTU
code
trace log
|
this tcpdump capture from the chrome client, so the clienthello is already segmented by the chrome client due to client MTU, even if BPFire
|
and even if I can force chrome client send large than > mtu 1500 packet, this would not work in practice, can't force each user computer to do that :) |
I tried a new Linux client with MTU set to 9000, and use hping3 to send > 1500 bytes
bpfire trace log:
it looks to me generic xdp driver already handles linearized skb since the |
so, none exceeded the "default" MTU size.. my understanding is that wouldn't be non-linear anyway.. |
my understanding (from the issue description) was that chrome client was already sending TCP frags into a large ETH frame.. if it's not the case, I don't think XDP will be aware of the frags because it runs before the network stack.. in this case, I see two paths:
|
yup, indeed I've the same understanding.. it looks like in the case of generic XDP it's resolved..
I think I got it wrongly.. I thought the case I was expecting for this to take effect was when I will try to run some tests on my end as well.. but it sounds to me that we should fallback to luanetfilter in the case we don't have TCP frags in XDP.. we will add the network stack overhead, but I'm not sure it will be so different than handling frags on our parser.. it would need to be measured.. if you can run some benchmark on it, it would be very valuable.. =) |
@vincentmli btw, in the case of hping you see the TCP segments in Wireshark? |
I used hping to only send one large tcp payload, it is one single tcp segment in tcpdump, no multiple tcp segmentation, wireshark should be same as tcpdump to show one tcp segment. |
XDP multi buffer /jumbo support would be unrealistic for consumer network environment because consumer computer does not generate jumbo frames. it would be beneficial in enterprise grade data center. I think probably move SNI parsing to lunanetfilter is ok for my bpfire home firewall use case regardless of the performance :) |
wondering if you have actually reached the condition of TCP frags, then.. |
indeed.. in my particular case, I'm concerned about running this in network operators' edge..
I think on combining both.. handling non-frag on XPD and frag on luanetfilter =).. so we speedup whatever we can.. otherwise, we let it go through the network stack and handle it on netfilter.. btw, @jperon has already ported the SNI filter to luanetfilter.. but he's using moonscript.. hehe.. you can take a look at https://github.com/luainkernel/snihook |
could snihook be integrated to be part of lunatik and included in lunatik source so I don't have to build separate snihook addon for bpfire? |
I'm thinking on extending the |
I’ll be glad to help if I understand well what I should port back! |
I like the idea of extending filter with both XDP and netfilter too, I prefer less software dependency since snihook requires other luarock and moonscript |
Thank you! It's basically to add a netfilter hook on filter/sni.lua.. so we can have handlers both to XDP and Netfilter calling the same parser.. I can help on Matrix if you need =) |
when use chrome browser, the ssl client hello is sent in two TCP segments, and reassembled, it appears sni.lua can't handle TCP reassembled clienthello SNI?
The text was updated successfully, but these errors were encountered: