evilsocket / evilsocket/opensnitch

Client.isAsking is a single global flag, not per-connection — concurrent connections silently get DefaultAction while any one AskRule is pending

Open
#1,644 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.