Repository navigation
fix(net-policy): NAT64's local-use prefix 64:ff9b:1::/48 is read for the IPv4 it carries - #90
Conversation
…the IPv4 it carries Signed-off-by: kevin9327 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55b8636aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (g === null) return null | ||
| const quad = (hi: number, lo: number) => [hi >> 8, hi & 255, lo >> 8, lo & 255].join('.') | ||
| const translated = g[0] === 0x64 && g[1] === 0xff9b && g.slice(2, 6).every((x) => x === 0) | ||
| const translated = g[0] === 0x64 && g[1] === 0xff9b && (g.slice(2, 6).every((x) => x === 0) || g[2] === 1) |
There was a problem hiding this comment.
Decode the /48 NAT64 layout before classifying addresses
When DNS64 returns a 64:ff9b:1::/48 address whose embedded IPv4 is public, RFC 6052 places the IPv4 bits around the reserved u octet and leaves g[6..7] zero, so this code extracts 0.0.0.0 and classifies the public address as private. In browse's own-proxy path, used when commandNetwork is open, that false classification enables allowPrivate for the run, letting the public page reach local/private destinations that were refused before this change. The new test fixtures put the IPv4 in the last 32 bits, so they do not exercise a valid /48 encoding.
AGENTS.md reference: AGENTS.md:L73-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, thanks. The /48 holds prefixes of any RFC 6052 length and the address doesn't say which, so reading only the last 32 bits was wrong for /48, /56 and /64 prefixes. Fixed in 48b4957:
- Every layout the address fits is read (bits 64-71 zero, suffix zero). A 0.0.0.0/8 reading is dropped when there are others, since it is just the /96 reading of a shorter layout.
- If all readings agree, that kind stands. Any metadata reading means metadata. Private against public is a new
unclearkind.destinationRefusalalways refuses it, even withallowPrivate, and it is neverprivateforopensPrivate. - Tests:
64:ff9b:1:808:8:800::(8.8.8.8 behind a /48) is public,64:ff9b:1:a9fe:a9:fe00::(/48) and64:ff9b:1:abcd:a9:fea9:fe00:0(/64) are metadata,64:ff9b:1:7f00:0:100::is private, and64:ff9b:1:0:8:808:a00:0(8.8.8.10 behind a /64, 10.0.0.0 read as a /96) is unclear and refused. Against the previous commit, 5 checks fail. net-policy now passes 73/73, browse 44/44, and typecheck passes.
AI-assisted reply; I checked the change and ran the tests.
… fits 64:ff9b:1::/48 holds operator prefixes of /48, /56, /64 or /96, and the address does not say which. Reading only the last 32 bits turned a public IPv4 behind a /48 prefix (64:ff9b:1:808:8:800::) into 0.0.0.0, private, which lets browse's own-proxy path open private destinations for it. Read each layout the address fits (reserved octet and suffix zero), drop 0.0.0.0/8 readings when others exist, and classify: one kind stands, any metadata reading refuses, private against public is 'unclear' and refused. Signed-off-by: kevin9327 <[email protected]>
Problem
addressKindreads the IPv4 address carried by the well-known NAT64 prefix64:ff9b::/96, but not by NAT64's local-use prefix64:ff9b:1::/48(RFC 8215). On a network whose NAT64 uses the local-use prefix,64:ff9b:1::a9fe:a9fereaches169.254.169.254but is classifiedpublic, so the policy proxy (browse tool, command network) lets it through to the metadata service. Likewise64:ff9b:1::7f00:1reaches loopback.The /48 holds operator prefixes of any RFC 6052 length (/48, /56, /64, /96), and the address does not say which. Under the shorter lengths, the IPv4 bits sit around the reserved octet (bits 64-71).
Fix
localUseIPv4sreads each layout the address fits: the reserved octet and suffix must be zero. A 0.0.0.0/8 reading is dropped when other readings exist, because it is only the /96 reading of a shorter layout.addressKind: if all readings agree, that kind stands. Any metadata reading meansmetadata. Private against public gives a newunclearkind, whichdestinationRefusalalways refuses (even withallowPrivate, with its own message).unclearis neverprivate, soopensPrivatecannot turn on private access for it.Test
test/net-policy.test.ts: /96 cases (metadata, private, public), /48 public64:ff9b:1:808:8:800::, /48 metadata, /64 metadata, /48 private, anunclearcase (64:ff9b:1:0:8:808:a00:0), and a refusal check forunclearwithallowPrivate.Results:
npm run typecheckpasses.npm teston Linux passes every suite up toinstall.test.ts, which uses BSDsed -i ''and fails on Linux independently of this change. The suites after it, run separately, pass.AI-assisted: this change was prepared with an AI coding assistant and reviewed and tested by me.