evilsocket / evilsocket/opensnitch
Client.isAsking is a single global flag, not per-connection — concurrent connections silently get DefaultAction while any one AskRule is pending
- Dominant language
- Python
- Stars
- 14.1k
- Forks
- 665
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
`ui.Client.isAsking` (`daemon/ui/client.go`) is a single `bool` field on the `Client`
struct, not scoped per-connection. `main.go`'s `acceptOrDeny()` uses it as a simple
gate before calling `uiClient.Ask()`:
```go
// main.go, acceptOrDeny()
if uiClient.Connected() == false || uiClient.GetIsAsking() == true {
applyDefaultAction(packet, con)
log.Debug("UI is not running or busy, connected: %v, running: %v", uiClient.Connected(), uiClient.GetIsAsking())
return nil
}
uiClient.SetIsAsking(true)
defer uiClient.SetIsAsking(false)
...
r = uiClient.Ask(con)
```
`Ask()` uses a 120-second timeout (`client.go`):
```go
ctx, cancel := context.WithTimeout(context.Background(), time.Second*120)
```
**Effect:** while any single connection's `AskRule` round-trip is in flight (which can
legitimately take up to 120 seconds if a human hasn't answered yet, or is answering a
different prompt), *every other new connection* that arrives during that window takes
the `isAsking == true` branch and gets `DefaultAction` applied immediately — silently,
with no `AskRule` ever sent to the UI, and no log output above `Debug` level (the
shipped default is `LogLevel: 2` / `IMPORTANT`, which does not show this).
With the shipped default `DefaultAction: allow`, this means: during any single pending
decision, an unbounded number of unrelated *new* outbound connections from *other*
processes silently pass through with no user visibility, for up to 2 minutes at a time.
On a normal interactive desktop generating multiple new connections per second, this
window recurs continuously in practice, not as a rare edge case.
## Why this matters
Firewall software's worst failure mode is "silently not filtering while appearing
healthy." This bug produces exactly that: every other signal a management UI can
observe (daemon connectivity, nftables/eBPF status, the daemon process being alive and
responsive) stays fully healthy throughout, because the daemon genuinely is healthy —
it's just serializing verdict requests on a single global flag instead of per
connection.
## Reproduction
1. Run opensnitchd with a UI/bridge connected but not actively/instantly answering
prompts (e.g. no human present, or a slow human).
2. Trigger a connection to a genuinely novel destination so it needs an `AskRule`
(`isAsking` becomes `true`, up to 120s).
3. While that's pending, trigger a *second* connection to a different novel
destination from a different process.
4. Observe: the second connection is not asked about at all. It resolves instantly
per `DefaultAction`. Only a `Debug`-level log line
(`UI is not running or busy, connected: true, running: true`) records it —
note the log format string's "running" label is actually `GetIsAsking()`, which
is also a bit confusing independent of the underlying bug.
## Suggested fix direction
Replace the single global `isAsking bool` with either:
- a bounded worker pool / semaphore that serializes `Ask()` calls without silently
dropping concurrent requests to `DefaultAction` (e.g. queue them, or run multiple
`Ask()` calls concurrently up to some cap), or
- at minimum, elevate the silent-default path to a `Warning`/`Important`-level log
line so it's visible without manually raising `LogLevel`, since it represents a
real (if working-as-designed) firewall bypass event.
Happy to help test a patch — found this while building a third-party GUI on top of
opensnitchd and needed to understand exactly when interactive verdict requests can be
silently skipped.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in daemon/ui/client.go and main.go's acceptOrDeny(), tracing isAsking, SetIsAsking, GetIsAsking, and the 120-second Ask() timeout. Define how concurrent AskRule requests should be queued or bounded without silently applying DefaultAction, and verify that unrelated connections no longer bypass the UI; if any requests remain intentionally skipped, ensure the path is visible at the shipped log level.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100