NfQeueue verdicts always requeue to queue 0 instead of the actual configured queue

danny commented

2026-07-07 14:35:36 +01:00

(Migrated from gitlab.keyop.co.uk)

Found while investigating #4.

The vendored github.com/florianl/go-nfqueue@v1.3.2's SetVerdict (nfqueue.go ~lines 330-372) hard-codes the verdict word's upper 3 bytes to zero (verdictData := append([]byte{0x0, 0x0, 0x0, byte(verdict)}, buf...), ~line 344) and its guard clause (~line 338) only accepts the raw verdict constants (NfDrop/NfAccept/NfStolen/NfQeueue/NfRepeat) - there is no way through this API to encode a target queue number into an NF_QUEUE-type verdict.

That means every nfqueue.NfQeueue verdict issued by this codebase (internal/nfq_forwarder/nfq_forwarder.go, e.g. the ErrPortTunnelAlreadyOpen branch and the success path in getNfCallback) actually asks the kernel to requeue the packet to queue 0 - not the connection's actual queue_number (always ≥ 1, enforced in internal/config/load.go). Since queue 0 is never bound by any configured connection, the kernel's nf_queue() lookup fails and the packet is dropped rather than redelivered.

Impact: any code path in this project relying on NfQeueue for redelivery/re-verdicting doesn't do what it looks like it does - it silently drops the packet instead. Directly relevant to #4's root cause, but is a distinct, independently-provable defect (in verdict encoding) from the hook-ordering issue tracked there - worth fixing/removing regardless of how #4 itself is resolved, since a future use of NfQeueue elsewhere in this codebase would hit the same silent failure.

See also #4 for the related architectural fix, which may make this moot for the primary packet path (if the fix replaces NfQeueue reliance with NfAccept after DNAT insertion) - but any other NfQeueue call sites (e.g. an already-open tunnel) still need auditing against this.

Found while investigating #4. The vendored `github.com/florianl/go-nfqueue@v1.3.2`'s `SetVerdict` (`nfqueue.go` ~lines 330-372) hard-codes the verdict word's upper 3 bytes to zero (`verdictData := append([]byte{0x0, 0x0, 0x0, byte(verdict)}, buf...)`, ~line 344) and its guard clause (~line 338) only accepts the raw verdict constants (`NfDrop`/`NfAccept`/`NfStolen`/`NfQeueue`/`NfRepeat`) - there is no way through this API to encode a target queue number into an `NF_QUEUE`-type verdict. That means every `nfqueue.NfQeueue` verdict issued by this codebase (`internal/nfq_forwarder/nfq_forwarder.go`, e.g. the `ErrPortTunnelAlreadyOpen` branch and the success path in `getNfCallback`) actually asks the kernel to requeue the packet to **queue 0** - not the connection's actual `queue_number` (always ≥ 1, enforced in `internal/config/load.go`). Since queue 0 is never bound by any configured connection, the kernel's `nf_queue()` lookup fails and the packet is dropped rather than redelivered. **Impact**: any code path in this project relying on `NfQeueue` for redelivery/re-verdicting doesn't do what it looks like it does - it silently drops the packet instead. Directly relevant to #4's root cause, but is a distinct, independently-provable defect (in verdict encoding) from the hook-ordering issue tracked there - worth fixing/removing regardless of how #4 itself is resolved, since a future use of `NfQeueue` elsewhere in this codebase would hit the same silent failure. See also #4 for the related architectural fix, which may make this moot for the primary packet path (if the fix replaces `NfQeueue` reliance with `NfAccept` after DNAT insertion) - but any other `NfQeueue` call sites (e.g. an already-open tunnel) still need auditing against this.

danny commented

2026-07-07 14:36:04 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in issue #4

danny commented

2026-07-07 14:46:08 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !70

danny commented

2026-07-07 15:33:39 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !71

danny commented

2026-07-07 16:07:43 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !72

claude commented

2026-07-07 21:22:10 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit 42cd04857b

mentioned in commit 42cd04857b6b908b95088fc3d985791669a08c30

claude commented

2026-07-07 21:22:28 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !73

claude commented

2026-07-07 21:22:38 +01:00

(Migrated from gitlab.keyop.co.uk)

Implementation is up for review: !73 (branch issue33_document_nfqueue_verdict_landmine). Audited the codebase for remaining NfQeueue usage - found none, since #4's fix already removed every call site. Purely a preventative doc comment on setVerdict explaining the defect. Waiting on review before merging.

Implementation is up for review: !73 (branch `issue33_document_nfqueue_verdict_landmine`). Audited the codebase for remaining `NfQeueue` usage - found none, since #4's fix already removed every call site. Purely a preventative doc comment on `setVerdict` explaining the defect. Waiting on review before merging.

danny commented

2026-07-08 06:06:25 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit acda1b5b7b

mentioned in commit acda1b5b7b3c02d0cf9fe529dbc3b30d4b177aa9

danny (Migrated from gitlab.keyop.co.uk) closed this issue

2026-07-08 06:06:25 +01:00

claude commented

2026-07-08 06:07:49 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !74

claude commented

2026-07-08 06:08:40 +01:00

(Migrated from gitlab.keyop.co.uk)

Merged via !73 (merge commit acda1b5) and released as v1.1.4 (patch bump).

Merged via !73 (merge commit `acda1b5`) and released as **v1.1.4** (patch bump).

Rows
Columns

NfQeueue verdicts always requeue to queue 0 instead of the actual configured queue #33