daemon: fix data race in getClientConfig iterating rules during load

[tredondo]

Bug

On a host with a large rule corpus (~2100 *.json files under rules.Path), the daemon panics during startup, reliably, every time the UI is already running:

panic: runtime error: index out of range [1002] with length 1002
goroutine 98 [running]:
github.com/evilsocket/opensnitch/daemon/ui.(*Client).getClientConfig(...)
        /root/build/opensnitch/daemon/ui/notifications.go:44 +0x33a
github.com/evilsocket/opensnitch/daemon/ui.(*Client).Subscribe(...)
        /root/build/opensnitch/daemon/ui/notifications.go:349 +0x85
created by github.com/evilsocket/opensnitch/daemon/ui.(*Client).onStatusChange in goroutine 30
        /root/build/opensnitch/daemon/ui/client.go:238 +0xa9

systemd Restart=on-failure then puts the daemon into a permanent crash loop (counter climbed to 8+ in our testing) — the firewall never comes up.

Indices in the panic line vary across restarts (1002, 1222, 1353, 1415, ...) — they're snapshots of partial rule loads.

Root cause

Loader.GetAll() returns the loader's internal map by reference under an RLock that's released the instant the call returns:

func (l *Loader) GetAll() map[string]*Rule {
    l.RLock()
    defer l.RUnlock()
    return l.rules
}

getClientConfig() then calls it twice — once for len() to size a slice, once to range over — without holding any lock:

rulesTotal := len(c.rules.GetAll())
ruleList := make([]*protocol.Rule, rulesTotal)
idx := 0
for _, r := range c.rules.GetAll() {
    ruleList[idx] = r.Serialize()       // ← panic when idx == rulesTotal
    idx++
}

If the loader is still inserting entries between the two GetAll() calls — which is the common case at startup — the second call's map has grown, the iteration produces more entries than the pre-sized slice, and ruleList[idx] walks off the end.

The race is reachable because reloadConfiguration calls c.Connect() (which starts the connection poller) before c.rules.Reload(newConfig.Rules.Path). With a small rule set, Reload finishes before the poller's first tick and there's no observable race. With a few thousand rules, the poller wins, the UI subscribes back, and getClientConfig runs in a separate goroutine while the loader is still synchronously parsing files.

go test -race catches the underlying data race too: mapIterNext (reader, post-GetAll) vs. mapassign_faststr in replaceUserRule (writer, holding l.Lock()).

Fix

daemon/rule/loader.go — GetAll() now returns a shallow snapshot of the map under the read lock. Same return type, same *Rule pointers, just a freshly allocated map container so len() and iteration are guaranteed consistent for the caller, with no lock-discipline requirement.

daemon/ui/notifications.go — call GetAll() once into a local variable; size and iterate that one map. Minimal change to keep the diff focused.

Loader.GetAll has exactly one caller in the daemon today (getClientConfig), so the contract change is contained. Other callers (none) would have been broken by the previous behavior anyway, since iterating the returned map without holding any lock is unsafe.

The *Rule values in the snapshot are still shared with the loader, but rules are replaced wholesale (replaceUserRule writes a new pointer into the map) rather than mutated in place, so the snapshot remains internally consistent for the read-only use cases this method serves.

Test

TestRuleLoaderGetAllSnapshot in daemon/rule/loader_test.go exercises the exact pattern that used to crash: writers churn the map via replaceUserRule/Delete, readers do the len()-then-iterate dance, and the test asserts the two counts agree. Verified that the test:

• Fails on master (without the fix) under -race: race detector flags mapIterNext vs. mapassign_faststr, plus the inconsistency check trips.
• Passes with the fix, both with and without -race.

What this PR doesn't fix

• The nil-deref in firewall.Serialize() from the same getClientConfig site reported in [Bug Report] daemon v1.7.0.0-1 panic runtime error INVALIDARGUMENT #1380 — different root cause (firewall not initialized yet), worth its own change.
• The ordering bug in reloadConfiguration where c.Connect() runs before c.rules.Reload(). Fixing the ordering would make the race unreachable too, but is a broader surgery; fixing the locking contract at GetAll addresses the bug at the right layer.

[tredondo]

Marking this ready for review after testing it locally. Commentary from Fable 5:

---

Live production validation of this fix on the affected host (Ubuntu 26.04, daemon 1.8.0, 2,281 rule files, GUI connected over unix socket).

Without the fix (stock 1.8.0 deb)

Restarting the daemon while the GUI is running crashed it 4 out of 4 attempts, ~1s after Loading rules from /etc/opensnitchd/rules/ ..., alternating between the two flavors of the race:

fatal error: concurrent map iteration and map write
  daemon/ui/notifications.go:43  (getClientConfig iterating rules)
  daemon/rule/loader.go:370      (loader writing the map, goroutine 1)

panic: runtime error: index out of range [1513] with length 1513
  github.com/evilsocket/opensnitch/daemon/ui.(*Client).getClientConfig
  github.com/evilsocket/opensnitch/daemon/ui.(*Client).Subscribe

(observed lengths across restarts: 1071, 1131, 1513 — snapshots of partial rule loads, as described in the PR body)

With systemd Restart=on-failure this became a permanent crash loop (restart counter reached 15) while the GUI stayed up, and with DefaultAction: deny all traffic was effectively blackholed until the GUI was killed.

With the fix

Cherry-picked 5e12a0b onto the v1.8.0 tag (applies cleanly), rebuilt the daemon, installed on the same host:

• 5/5 systemctl restart opensnitch with the GUI running and reconnecting during the ~60s rule load: no panics.
• 20 consecutive rule add/delete fsnotify reload cycles while the GUI was re-subscribing (concurrent getClientConfig serialization): no panics.
• TestRuleLoaderGetAllSnapshot passes repeatedly under -race; the full ./rule/ suite passes (one pre-existing, unrelated flake in the temporary-rules expiry test, timing-sensitive).

[gustavo-iniguez-goya]

thanks @tredondo . Interestingly I could only reproduce it with LogLevel set to 0.