Skip to content

Commit 17b8ad4

Browse files
santhoshmprabhusanthoshmprabhu
committed
fix: address comments
1 parent 9a436c4 commit 17b8ad4

File tree

2 files changed

+40
-35
lines changed

2 files changed

+40
-35
lines changed

‎bpf-prog/block-iptables/bpf/src/block_iptables.bpf.c‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@
1313
#define IPT_SO_SET_REPLACE 64
1414
#define TASK_COMM_LEN 16
1515
#define COMM_COUNT 3
16+
#define IPPROTO_IP 0
17+
#define IPPROTO_IP6 41
18+
#define AF_NETLINK 16
19+
#define NETLINK_NETFILTER 12
20+
#define NETLINK_MSG_COUNT 4
21+
#define NFNL_SUBSYS_NFTABLES 10
22+
#define NFT_MSG_NEWRULE 6
23+
24+
#define CILIUM_AGENT "cilium-agent"
25+
#define IP_MASQ "ip-masq"
26+
#define AZURE_CNS "azure-cns"
1627

1728
char __license[] SEC("license") = "Dual MIT/GPL";
1829
volatile const u64 host_netns_inode = 4026531840; // Initialized by userspace
@@ -25,13 +36,15 @@ struct {
2536
__uint(pinning, LIBBPF_PIN_BY_NAME);
2637
} event_counter SEC(".maps");
2738

39+
// This function checks if the parent process of the current task is allowed to install iptables rules.
40+
// It checks the parent's command name against a predefined list of allowed prefixes.
2841
int is_allowed_parent ()
2942
{
3043
struct task_struct *task = (struct task_struct *)bpf_get_current_task();
3144
struct task_struct *parent_task = NULL;
3245

3346
char parent_comm[TASK_COMM_LEN] = {};
34-
const char target_prefixes[COMM_COUNT][TASK_COMM_LEN] = {"cilium-agent", "ip-masq", "azure-cns"};
47+
const char target_prefixes[COMM_COUNT][TASK_COMM_LEN] = {CILIUM_AGENT, IP_MASQ, AZURE_CNS};
3548

3649
// Safely get parent task_struct
3750
parent_task = BPF_CORE_READ(task, real_parent);
@@ -106,8 +119,8 @@ int BPF_PROG(iptables_legacy_block, struct socket *sock, int level, int optname)
106119
return 0;
107120
}
108121

109-
if (level == 0 /*IPPROTO_IP*/ || level == 41 /*IPPROTO_IP6*/) {
110-
if (optname == IPT_SO_SET_REPLACE) {
122+
if (level == IPPROTO_IP || level == IPPROTO_IP6) {
123+
if (optname == IPT_SO_SET_REPLACE) {
111124
if (is_host_ns() && !is_allowed_parent()) {
112125
increment_event_counter();
113126
return -EPERM;
@@ -125,14 +138,14 @@ int BPF_PROG(iptables_nftables_block, struct sock *sk, struct sk_buff *skb) {
125138
bpf_probe_read_kernel(&family, sizeof(family), &sk->sk_family);
126139
}
127140

128-
if (family != 16) // Not AF_NETLINK
141+
if (family != AF_NETLINK)
129142
return 0;
130143

131144
if (sk != NULL) {
132145
bpf_probe_read_kernel(&proto, sizeof(proto), &sk->sk_protocol);
133146
}
134147

135-
if (proto != 12) // Not NETLINK_NETFILTER
148+
if (proto != NETLINK_NETFILTER)
136149
return 0;
137150

138151
if (!is_host_ns()) {
@@ -153,7 +166,7 @@ int BPF_PROG(iptables_nftables_block, struct sock *sk, struct sk_buff *skb) {
153166
return 0;
154167

155168
#pragma unroll
156-
for (int i = 0; i < 4; i++) {
169+
for (int i = 0; i < NETLINK_MSG_COUNT; i++) {
157170
if (skb_len < sizeof(struct nlmsghdr))
158171
return 0;
159172

@@ -164,7 +177,7 @@ int BPF_PROG(iptables_nftables_block, struct sock *sk, struct sk_buff *skb) {
164177
__u8 cmd = type & 0xFF;
165178
__u32 nlmsg_len = nlh.nlmsg_len;
166179

167-
if (subsys_id == 10 && cmd == 6) {
180+
if (subsys_id == NFNL_SUBSYS_NFTABLES && cmd == NFT_MSG_NEWRULE) {
168181
if(is_allowed_parent()) {
169182
return 0;
170183
} else {

‎bpf-prog/block-iptables/cmd/block-iptables/main.go‎

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -77,39 +77,42 @@ func setupFileWatcher(configFile string) (*fsnotify.Watcher, error) {
7777
return watcher, nil
7878
}
7979

80-
// handleFileEvent processes file system events
81-
func handleFileEvent(event fsnotify.Event, configFile string, bp bpfprogram.Attacher) {
82-
// Check if the event is for our config file
83-
if filepath.Base(event.Name) != filepath.Base(configFile) {
84-
return
85-
}
86-
87-
log.Printf("Config file changed: %s (operation: %s)", event.Name, event.Op)
88-
89-
// Small delay to handle rapid successive events
90-
time.Sleep(100 * time.Millisecond)
91-
80+
func checkFileStatusAndUpdateBPF(configFile string, bp bpfprogram.Attacher) {
9281
// Check current state and take action
9382
fileState := isFileEmptyOrMissing(configFile)
9483
switch fileState {
9584
case 1: // File is empty
9685
log.Println("File is empty, attaching BPF program")
97-
if err := bp.Attach(); err != nil {
86+
if err := bp.Attach(); err != nil { // No-op if already attached
9887
log.Printf("Failed to attach BPF program: %v", err)
9988
}
10089
case 0: // File has content
10190
log.Println("File has content, detaching BPF program")
102-
if err := bp.Detach(); err != nil {
91+
if err := bp.Detach(); err != nil { // No-op if already detached
10392
log.Printf("Failed to detach BPF program: %v", err)
10493
}
10594
case -1: // File is missing
10695
log.Println("Config file was deleted, detaching BPF program")
107-
if err := bp.Detach(); err != nil {
96+
if err := bp.Detach(); err != nil { // No-op if already detached
10897
log.Printf("Failed to detach BPF program: %v", err)
10998
}
11099
}
111100
}
112101

102+
// handleFileEvent processes file system events
103+
func handleFileEvent(event fsnotify.Event, configFile string, bp bpfprogram.Attacher) {
104+
// Check if the event is for our config file
105+
if filepath.Base(event.Name) != filepath.Base(configFile) {
106+
return
107+
}
108+
109+
log.Printf("Config file changed: %s (operation: %s)", event.Name, event.Op)
110+
111+
// Small delay to handle rapid successive events
112+
time.Sleep(100 * time.Millisecond)
113+
checkFileStatusAndUpdateBPF(configFile, bp)
114+
}
115+
113116
// run is the main application logic, separated for easier testing
114117
func run(config *BlockConfig) error {
115118
log.Printf("Using config file: %s", config.ConfigFile)
@@ -123,19 +126,8 @@ func run(config *BlockConfig) error {
123126
bp := config.AttacherFactory()
124127
defer bp.Close()
125128

126-
// Initial state check
127-
fileState := isFileEmptyOrMissing(config.ConfigFile)
128-
switch fileState {
129-
case 1: // File is empty
130-
log.Println("File is empty, attaching BPF program")
131-
if err := bp.Attach(); err != nil {
132-
return fmt.Errorf("failed to attach BPF program: %w", err)
133-
}
134-
case 0: // File has content
135-
log.Println("Config file has content, BPF program will remain detached")
136-
case -1: // File is missing
137-
log.Println("Config file is missing, waiting for file to be created...")
138-
}
129+
// Check initial state of the config file
130+
checkFileStatusAndUpdateBPF(config.ConfigFile, bp)
139131

140132
// Setup file watcher
141133
watcher, err := setupFileWatcher(config.ConfigFile)

0 commit comments

Comments
 (0)