Skip to content

Commit bb00376

Browse files
committed
fix(egress): judge IP literals synchronously and match every IPv4 spelling
Two more from review, both verified with probes first. The deferral added last round applied to every liftable refusal whenever an IP range was configured — literals included. A literal has already been judged against its own address, so a lookup can add nothing, and deferring accepted literals outside every configured range: with only `10.0.0.0/8` allowlisted, `https://192.168.1.1` came back valid. It now defers hostnames only. `matchesRangeAllowlist` compared the raw parsed address, so a resolver answering with `::a00:1` — which is 10.0.0.1 — was refused by a `10.0.0.0/8` entry. It over-blocks rather than under-blocks, but an allowlisted destination became unreachable depending on what DNS returned. `canonicalAddress` already existed for the metadata comparison and simply was not used here. Folding needed a carve-out the metadata path did not: `::` and `::1` are the unspecified and loopback addresses, not an IPv4 carried inside IPv6. Folding `::1` to `0.0.0.1` would have let a `0.0.0.0/8` entry match loopback and stopped `::1/128` matching it. Both directions are pinned by tests, alongside the four spellings an IPv4 range must accept.
1 parent 0a50b90 commit bb00376

4 files changed

Lines changed: 62 additions & 4 deletions

File tree

apps/sim/lib/core/security/input-validation.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,22 @@ describe('validateUrlWithDNS', () => {
473473
expect(result.error).toContain('private or reserved address')
474474
})
475475

476+
it('refuses an IP literal outside the configured range without deferring', () => {
477+
// A literal was judged against its own address, so a lookup could add
478+
// nothing — deferring it would accept literals outside every range.
479+
envFlagsMock.egressAllowedIpRanges = '10.0.0.0/8'
480+
try {
481+
expect(
482+
validateExternalUrl('https://192.168.1.1/x', 'url', 'configuredEndpoint').isValid
483+
).toBe(false)
484+
expect(validateExternalUrl('https://10.0.0.5/x', 'url', 'configuredEndpoint').isValid).toBe(
485+
true
486+
)
487+
} finally {
488+
envFlagsMock.egressAllowedIpRanges = undefined
489+
}
490+
})
491+
476492
it('permits a private IP once the operator allowlists its range', async () => {
477493
envFlagsMock.egressAllowedIpRanges = '192.168.0.0/16'
478494
try {

apps/sim/lib/core/security/input-validation.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import { evaluateUrl, isLiftableByVouching, policyDefersToAddress } from '@sim/security/egress'
3+
import { isIpLiteral, unwrapIpv6Brackets } from '@sim/security/ssrf'
34
import {
45
describeEgressDenial,
56
type EgressProfile,
@@ -521,7 +522,15 @@ export function validateExternalUrl(
521522
// host permitted only by EGRESS_ALLOWED_IP_RANGES cannot be recognised until
522523
// DNS runs, and refusing here would stop it being configured at all.
523524
// validateUrlWithDNS makes the authoritative call before anything is dialled.
524-
if (policyDefersToAddress(policy) && isLiftableByVouching(decision.reason)) {
525+
//
526+
// Only for a hostname. A literal was judged against its own address, so there
527+
// is nothing a lookup could add and deferring would accept a literal outside
528+
// every configured range.
529+
if (
530+
!isIpLiteral(unwrapIpv6Brackets(parsed.hostname)) &&
531+
policyDefersToAddress(policy) &&
532+
isLiftableByVouching(decision.reason)
533+
) {
525534
return { isValid: true }
526535
}
527536

packages/security/src/egress.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,33 @@ describe('operator allowlist — the self-hosted posture', () => {
155155
})
156156
})
157157

158+
describe('an IPv4 range matches every spelling of the same address', () => {
159+
const ranged = createEgressPolicy({ allowedRanges: '10.0.0.0/8' })
160+
161+
it.each([
162+
['10.0.0.1', 'plain IPv4'],
163+
['::a00:1', 'the IPv4-compatible form a resolver can return'],
164+
['::ffff:10.0.0.1', 'the IPv4-mapped form'],
165+
['::10.0.0.1', 'IPv4-compatible written long-hand'],
166+
])('permits %s — %s', (address) => {
167+
expect(decide(ranged, 'https://svc.internal/', address).allowed).toBe(true)
168+
})
169+
170+
it('does not fold the addresses that are IPv6 in their own right', () => {
171+
// `::1` is loopback, not 0.0.0.1 carried inside IPv6 — folding it would let
172+
// a 0.0.0.0/8 entry match it, and stop `::1/128` matching it.
173+
const loopback = createEgressPolicy({ allowedRanges: '::1/128' })
174+
expect(decide(loopback, 'https://svc.internal/', '::1').allowed).toBe(true)
175+
176+
const zeroPage = createEgressPolicy({ allowedRanges: '0.0.0.0/8' })
177+
expect(decide(zeroPage, 'https://svc.internal/', '::1').allowed).toBe(false)
178+
})
179+
180+
it('still refuses an address outside the range in any spelling', () => {
181+
expect(reason(ranged, 'https://svc.internal/', '::c0a8:101')).toBe('address-blocked')
182+
})
183+
})
184+
158185
describe('the same operator config is inert on the hosted posture', () => {
159186
it.each([
160187
['http://host.docker.internal/', '192.168.65.254'],

packages/security/src/egress.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,10 @@ function matchesHostAllowlist(host: string, policy: EgressPolicy): boolean {
245245

246246
function matchesRangeAllowlist(address: string, policy: EgressPolicy): boolean {
247247
if (policy.allowedRanges.length === 0) return false
248-
const clean = unwrapIpv6Brackets(address)
249-
if (!ipaddr.isValid(clean)) return false
248+
// Canonical form, so an operator's IPv4 CIDR still matches a resolver that
249+
// answered with the IPv4-compatible IPv6 spelling of the same address.
250+
const clean = canonicalAddress(address)
251+
if (clean === null) return false
250252
const parsed = ipaddr.process(clean)
251253
return policy.allowedRanges.some(
252254
(range) =>
@@ -268,7 +270,11 @@ function canonicalAddress(address: string): string | null {
268270
const parsed = ipaddr.process(clean)
269271
if (parsed.kind() === 'ipv6') {
270272
const parts = (parsed as ipaddr.IPv6).parts
271-
if (parts.slice(0, 6).every((part) => part === 0)) {
273+
const embedded = ((parts[6] << 16) >>> 0) + parts[7]
274+
// `::` and `::1` are the unspecified and loopback addresses, not an IPv4
275+
// carried inside IPv6 — folding them would turn `::1` into `0.0.0.1` and
276+
// stop an operator's `::1/128` entry matching it.
277+
if (parts.slice(0, 6).every((part) => part === 0) && embedded > 1) {
272278
return ipaddr
273279
.fromByteArray([
274280
(parts[6] >> 8) & 0xff,

0 commit comments

Comments
 (0)