fix(net): error on per-user sub-CIDR uid overflow instead of silent collision (1.1.18)#220
Merged
Merged
Conversation
…ollision (1.1.18)
PerUserNetPure::composeIpv4/composeIpv6 mapped a uid into its sub-CIDR
slot with `uid & (slotCount - 1)` — a modulo wrap. Two uids differing
by a multiple of 2^slotBits got the SAME sub-CIDR, silently defeating
the per-operator network separation the function provides. With the
example 10.66.0.0/16 -> /24 (8 slot bits) every uid >= 256 aliased a
lower one (uid 1000 and 1256 both -> 10.66.232.0/24).
The existing tests deliberately ENCODED this wrap ("uid 256 wraps back
to slot 0"), so it was a known design choice, not an accidental bug —
but per docs/trust-model.md the per-user network split is honest-
operator convenience, and silently handing two honest operators the
same subnet is an isolation failure. Per the maintainer's call, convert
it to a loud error.
Now both functions return an error when `uid >= 2^slotBits`:
"uid <N> exceeds the <B>-bit per-user slot space (max uid <M>); widen
the master CIDR or shrink the sub-prefix". The error names the uid so
operators can act.
Safety: composition is opt-in (only when network_master_cidr_v4/v6 is
set), and the authz path's config carries only zfs/path prefixes (no
network CIDR), so the 1.1.12 -> 1.1.17 privops gates are unaffected.
composeForUid already returns its network error with an "ipv4:"/"ipv6:"
prefix; an over-tight config that appeared to work for one operator now
reports the limit at crate run.
Tests: per_user_net_pure_test replaces the wrap-encoding assertion with
v4+v6 overflow-error cases (boundary uid OK, +1 errors, error names the
uid) and re-bases typical/adjacency/non-canonical on slot spaces that
fit their uids; per_user_env_pure_test widens the example v4 master to
/8 (10.3.232.0/24 for uid 1000). Verified via standalone harness
(kyua/atf absent in this env); both test files shim-syntax-checked.
docs/trust-model.{md,uk.md}: Applies-to -> 1.1.18, subtitle series
-> 1.1.17, + a sizing note in the per-user-namespacing section.
CHANGELOG [1.1.18] + --version bump.
https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PerUserNetPure::composeIpv4/composeIpv6mapped a uid into its sub-CIDR slot withuid & (slotCount - 1)— a modulo wrap. Two uids differing by a multiple of2^slotBitsgot the same sub-CIDR, silently defeating the per-operator network separation the function exists to provide.With the example
10.66.0.0/16→/24(8 slot bits) every uid ≥ 256 aliased a lower one — uid 1000 and uid 1256 both got10.66.232.0/24.The existing tests deliberately encoded this wrap (
// uid 256 wraps back to slot 0), so it was a known design choice — but perdocs/trust-model.mdthe per-user network split is honest-operator convenience, and silently handing two honest operators the same subnet is an isolation failure. Per the maintainer's decision (asked + answered: error on overflow), this converts the silent collision into a loud configuration error.The fix
Both functions now return an error when
uid >= 2^slotBits:The error names the offending uid so operators can act.
Operator impact
Deployments that configured
network_master_cidr_v4/_v6must size the slot space (subPrefix − masterPrefixbits) to their uid range:/16→/24/8→/24An over-tight config that appeared to work for a single operator now reports the limit at
crate run. (It was already broken for multi-operator — two colliding uids shared a subnet.)Safety
g_perUserAuthzCfgcarries only the zfs/path prefixes, not network CIDRs, socomposeForUidnever composes a network sub-CIDR on the authz path → the 1.1.12 → 1.1.17 privops gates can't be impacted.composeForUidalready surfaces its network error with anipv4:/ipv6:prefix.Tests
per_user_net_pure_test: replaced the wrap-encoding assertion with v4 + v6 overflow-error cases (boundary uid OK, uid+1 errors, error names the uid); re-based typical/adjacency/non-canonical on slot spaces that fit their uids (/8master).per_user_env_pure_test: widened the example v4 master to/8→10.3.232.0/24for uid 1000.composeIpv4/6(kyua/atf are absent in this re-provisioned env); both ATF test files shim-syntax-checked.Test plan
g++ -fsyntax-onlyclean onper_user_net_pure.cpp.kyua) runs the real ATF cases.Docs
docs/trust-model.{md,uk.md}: Applies-to → 1.1.18, gate-series subtitle → 1.1.17, and a sizing note in the per-user-namespacing section.CHANGELOG [1.1.18]+--versionbump.https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
Generated by Claude Code