| Message ID | 2021112123191459526248@chinatelecom.cn |
|---|---|
| Headers | show |
| Series | bug fix: avoid install bad dp flow | expand |
On 11/21/21 16:19, lic121 wrote: > Version 4: > - Cover case where tcp_hdrlen > tcp_pkt_size > - Other small adjustments > > ovs may install bad datapath flow when meet malformed pkts. As a > result, it may allows some unwanted pkts pass. This could be a point > of attack. > > lic121 (3): > upcall: prevent from installing flows when inconsistence > tests: fix packet data endianness > upcall: considering dataofs when parsing tcp pkt > > lib/flow.c | 20 ++++++++++------- > ofproto/ofproto-dpif-upcall.c | 26 +++++++++++++++++++--- > tests/flowgen.py | 2 +- > tests/ofproto-dpif.at | 50 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 86 insertions(+), 12 deletions(-) > Thanks for the new version! With some small cosmetic changes I applied patches #2 and #3. Also backported down to all supported branches (down to 2.13). This should fix the main parsing problem. And I found the patch that actually broke the parsing more than 7 years ago: 5a51b2cd3483 ("lib/ofpbuf: Remove 'l7' pointer.") I added it to the 'Fixes' tag in the patch #3. I didn't actually look closely to the patch #1 yet, so I'll need to get back to it a bit later. Best regards, Ilya Maximets.
>On 11/21/21 16:19, lic121 wrote: >> Version 4: >> - Cover case where tcp_hdrlen > tcp_pkt_size >> - Other small adjustments >> >> ovs may install bad datapath flow when meet malformed pkts. As a >> result, it may allows some unwanted pkts pass. This could be a point >> of attack. >> >> lic121 (3): >> upcall: prevent from installing flows when inconsistence >> tests: fix packet data endianness >> upcall: considering dataofs when parsing tcp pkt >> >> lib/flow.c | 20 ++++++++++------- >> ofproto/ofproto-dpif-upcall.c | 26 +++++++++++++++++++--- >> tests/flowgen.py | 2 +- >> tests/ofproto-dpif.at | 50 +++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 86 insertions(+), 12 deletions(-) >> > >Thanks for the new version! With some small cosmetic changes >I applied patches #2 and #3. Also backported down to all >supported branches (down to 2.13). This should fix the main >parsing problem. > >And I found the patch that actually broke the parsing more >than 7 years ago: > 5a51b2cd3483 ("lib/ofpbuf: Remove 'l7' pointer.") > >I added it to the 'Fixes' tag in the patch #3. > >I didn't actually look closely to the patch #1 yet, so I'll >need to get back to it a bit later. > Thanks for review the patches. Yes, the #3 patch fixs the main problem. The #1 patch is important as well. Without patch #1, we will have to keep an key on every patch, which affects flow parsing, to prevent it from breaking the consistentance between kmod and vswitchd. >Best regards, Ilya Maximets. >