OCPBUGS-67363: fixes ip traffic fragmentation verification#730
OCPBUGS-67363: fixes ip traffic fragmentation verification#730raphaelvrosa wants to merge 1 commit into
Conversation
Impact: Attackers can access blocked ports by fragmenting IP packets in specific ways that exploit the firewall's failure to validate IP fragmentation headers before extracting Layer 4 port information. Root Cause: The eBPF code assumes all packets have complete TCP/UDP/SCTP headers immediately following the IP header, without checking if the packet is a non-first fragment that contains only payload data. Solution: - Check iph->frag_off before extracting L4 headers - Deny all non-first fragments (secure by default) - First fragments (offset = 0) still processed normally with L4 inspection The solution was verified and validate locally with kind and ovn-kubernetes as CNI. The ebpf program was loaded, and traffic was blocked. The verification of the fragmentation issue could not be fully verified, as docker and host were probably dropping fragmented packets before it reached the cluster. Signed-off-by: Raphael Rosa <raprosa@redhat.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@raphaelvrosa: This pull request references Jira Issue OCPBUGS-67363, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/invalid-bug
WalkthroughAdds IPv4 fragmentation constants and updates ip_extract_l4info() to detect non-first fragments, returning a new error code that both ipv4_firewall_lookup() and ipv6_firewall_lookup() handle by immediately denying the packet. Separately, the Makefile's CERT_MANAGER_VERSION is bumped from v1.9.1 to v1.20.2. ChangesNon-first IP Fragment Denial
Cert-manager Version Bump
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Packet
participant ip_extract_l4info
participant ipv4_firewall_lookup
participant ipv6_firewall_lookup
Packet->>ip_extract_l4info: parse IP header
ip_extract_l4info->>ip_extract_l4info: check fragment offset
alt non-first fragment
ip_extract_l4info-->>ipv4_firewall_lookup: return -2
ipv4_firewall_lookup->>ipv4_firewall_lookup: SET_ACTIONRULE_RESPONSE(DENY, INVALID_RULE_ID)
ip_extract_l4info-->>ipv6_firewall_lookup: return -2
ipv6_firewall_lookup->>ipv6_firewall_lookup: SET_ACTIONRULE_RESPONSE(DENY, INVALID_RULE_ID)
else extraction failure
ip_extract_l4info-->>ipv4_firewall_lookup: return generic error
ip_extract_l4info-->>ipv6_firewall_lookup: return generic error
end
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci-robot
added
jira/valid-bug
|
@raphaelvrosa: This pull request references Jira Issue OCPBUGS-67363, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: raphaelvrosa The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bpf/ingress_node_firewall_kernel.c`:
- Around line 309-318: The current fragment-handling branch in
ip_extract_l4info() only denies IPv4 non-first fragments via the -2 return path,
but IPv6 Fragment headers are still not detected. Update the IPv6 parsing path
in bpf/ingress_node_firewall_kernel.c so ip_extract_l4info() walks IPv6
extension headers, explicitly checks for NEXTHDR_FRAGMENT, and identifies
non-first fragments as a deny case; then make the existing caller logic in the
l4_result check return DENY for those IPv6 fragments instead of falling through
to SET_ACTION(UNDEF).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d4ba6ea4-4664-4622-95ab-3845b7991dd2
⛔ Files ignored due to path filters (4)
pkg/ebpf/bpf_arm64_bpfel.ois excluded by!**/*.opkg/ebpf/bpf_powerpc_bpfel.ois excluded by!**/*.opkg/ebpf/bpf_s390_bpfeb.ois excluded by!**/*.opkg/ebpf/bpf_x86_bpfel.ois excluded by!**/*.o
📒 Files selected for processing (3)
Makefilebpf/ingress_node_firewall.hbpf/ingress_node_firewall_kernel.c
| int l4_result = ip_extract_l4info(data, data_end, &proto, &dstPort, &icmpType, | ||
| &icmpCode, 0); | ||
| if (unlikely(l4_result < 0)) { | ||
| if (l4_result == -2) { | ||
| // Non-first IP fragment: deny by security policy | ||
| return SET_ACTIONRULE_RESPONSE(DENY, INVALID_RULE_ID); | ||
| } | ||
| ingress_node_firewall_printk("failed to extract l4 info"); | ||
| return SET_ACTION(UNDEF); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm there's no other IPv6 extension-header walking logic elsewhere in the bpf dir
rg -n 'nexthdr|NEXTHDR|ipv6hdr' bpf/Repository: openshift/ingress-node-firewall
Length of output: 1822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant IPv4/IPv6 parsing and lookup flow.
sed -n '80,170p' bpf/ingress_node_firewall_kernel.c
printf '\n---\n'
sed -n '280,340p' bpf/ingress_node_firewall_kernel.c
printf '\n---\n'
sed -n '340,420p' bpf/ingress_node_firewall_kernel.c
# Look for any IPv6 fragment/extension-header handling in the repo.
printf '\n=== search ===\n'
rg -n 'NEXTHDR_FRAGMENT|fragment|ext header|extension header|ipv6 fragment|IPV6_EXT' bpf/ .Repository: openshift/ingress-node-firewall
Length of output: 18706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how UNDEF propagates to the final packet decision.
rg -n 'SET_ACTION\(UNDEF\)|UNDEF|DENY|ALLOW|SET_ACTIONRULE_RESPONSE' bpf/ingress_node_firewall_kernel.c bpf/ingress_node_firewall.h
printf '\n=== action helpers ===\n'
sed -n '1,140p' bpf/ingress_node_firewall.h
printf '\n=== lookup callers ===\n'
rg -n 'ipv6_firewall_lookup|ipv4_firewall_lookup|SET_ACTIONRULE_RESPONSE|SET_ACTION\(' bpf/ingress_node_firewall_kernel.cRepository: openshift/ingress-node-firewall
Length of output: 7842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the final action mapping for XDP and TCX paths.
sed -n '440,575p' bpf/ingress_node_firewall_kernel.cRepository: openshift/ingress-node-firewall
Length of output: 4810
IPv6 fragments still bypass port matching
ip_extract_l4info() only returns -2 for IPv4; the IPv6 path never checks NEXTHDR_FRAGMENT or walks extension headers. IPv6 packets with a Fragment header fall through as UNDEF and are passed, so this does not close the fragmentation bypass for IPv6. Add IPv6 extension-header parsing and deny non-first fragments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bpf/ingress_node_firewall_kernel.c` around lines 309 - 318, The current
fragment-handling branch in ip_extract_l4info() only denies IPv4 non-first
fragments via the -2 return path, but IPv6 Fragment headers are still not
detected. Update the IPv6 parsing path in bpf/ingress_node_firewall_kernel.c so
ip_extract_l4info() walks IPv6 extension headers, explicitly checks for
NEXTHDR_FRAGMENT, and identifies non-first fragments as a deny case; then make
the existing caller logic in the l4_result check return DENY for those IPv6
fragments instead of falling through to SET_ACTION(UNDEF).
raphaelvrosa
changed the title
|
@raphaelvrosa: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Impact: Attackers can access blocked ports by fragmenting IP packets in specific ways that exploit the firewall's failure to validate IP fragmentation headers before extracting Layer 4 port information.
Root Cause: The eBPF code assumes all packets have complete TCP/UDP/SCTP headers immediately following the IP header, without checking if the packet is a non-first fragment that contains only payload data.
Solution:
The solution was verified and validate locally with kind and ovn-kubernetes as CNI. The ebpf program was loaded, and traffic was blocked.
The verification of the fragmentation issue could not be fully verified, as docker and host were probably dropping fragmented packets before it reached the cluster.
- What this PR does and why is it needed
- Special notes for reviewers
- How to verify it
- Description for the changelog
Summary by CodeRabbit
Bug Fixes
Chores