-
Notifications
You must be signed in to change notification settings - Fork 0
security(webhooks): enforce resolved-address delivery boundary #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
50
commits into
develop
Choose a base branch
from
sentinel-ssrf-webhooks-1668440070062097017
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
ce31e62
🛡️ Sentinel: [CRITICAL] Prevent SSRF via webhook URLs
seonghobae 693745c
test(security): reproduce webhook SSRF at network boundary
seonghobae e84c47c
test(security): make webhook SSRF RED exercise authenticated org
seonghobae 8bef144
feat(security): add hardened webhook delivery adapter contract
seonghobae 9ebff18
chore(security): run one-shot webhook SSRF green repair
seonghobae 8addf68
chore(ci): use available macOS runner for one-shot security repair
seonghobae f9b77e2
fix(security): keep webhook policy failures on async delivery boundary
seonghobae eeaf690
ci: re-kick required checks to bypass flake
seonghobae 1c7c1fa
ci: re-kick required checks to bypass flake
seonghobae 4cef318
ci: re-kick required checks to bypass flake
seonghobae b487901
ci: re-kick required checks to bypass flake
seonghobae caf6711
ci: re-kick required checks to bypass flake
seonghobae 3ba6e57
test(security): specify webhook transport SSRF boundary
seonghobae 0c09675
fix(security): add DNS-pinned webhook transport boundary
seonghobae 4d62767
test(security): cover webhook transport failure edges
seonghobae 24af8b7
test(security): run webhook transport contract in unit suite
seonghobae 758f4c6
refactor(security): remove unreachable webhook URL branch
seonghobae e25f3c0
test(security): reach full webhook transport edge coverage
seonghobae ac46051
test(coverage): include webhook transport in coverage gate
seonghobae 15e8974
test(webhooks): cover NAT64 translation prefixes
seonghobae b4f27be
fix(webhooks): block NAT64 translation prefixes
seonghobae d9a572a
test(webhooks): prove shipped SSRF boundary
seonghobae 071d5b4
test(webhooks): gate SSRF integration
seonghobae a67b873
fix(webhooks): reject non-public URL literals at registration boundary
seonghobae 6c323a8
ci: re-kick required checks to bypass flake
seonghobae 07160fc
ci: re-kick required checks to bypass flake
seonghobae 36e25e6
ci: re-kick required checks to bypass flake
seonghobae 1c3bd9c
test(e2e): restore unrelated modulepreload assertions
seonghobae 2c06dd2
docs(security): remove hostname-blocklist SSRF doctrine
seonghobae fa176f1
test(coverage): include webhook transport security boundary
seonghobae 5134449
test(security): cover NAT64 embedded private destinations
seonghobae 1f5db1f
test(webhooks): restore deterministic delivery retry RED
seonghobae 8d9c299
test(webhooks): exercise application retry with blocked persisted target
seonghobae 6a8321a
ci: re-kick required checks to bypass flake
seonghobae 1ce30af
repair(webhooks): restore reviewed coverage and retry evidence
seonghobae 1008dc2
ci: re-kick required checks to bypass flake
seonghobae 560b18d
repair(webhooks): restore exact transport evidence after re-kick
seonghobae 1de667a
ci: re-kick required checks to bypass flake
seonghobae 26171d0
repair(ci): restore SSRF evidence after non-neutral re-kick
seonghobae fd5250f
test(webhooks): align translation-prefix policy with IANA registries
seonghobae dfb358f
fix(webhooks): classify translation prefixes by public reachability
seonghobae e6fbe62
fix(webhook): isolate IPv4 and IPv6 blocklists to prevent false posit…
seonghobae b51866e
ci: re-kick required checks to bypass flake
seonghobae b1447c8
repair: remove branch-local webhook doctrine
seonghobae dcb58b0
test(webhooks): restore translation and mapped-address contract
seonghobae 7ae6c45
fix(webhooks): enforce translation-prefix address policy
seonghobae 3c96711
ci: re-kick required checks to bypass flake
seonghobae 6074d17
repair(webhooks): restore translation-policy security boundary
seonghobae 4784761
ci: re-kick required checks to bypass flake
seonghobae ab0ff34
repair(webhooks): restore reviewed translation boundary
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| import { lookup as dnsLookup } from 'node:dns/promises'; | ||
| import { BlockList, isIP } from 'node:net'; | ||
| import { request as httpsRequest } from 'node:https'; | ||
|
|
||
| const BLOCKED4 = new BlockList(); | ||
| const BLOCKED6 = new BlockList(); | ||
| const block4 = (network, prefix) => BLOCKED4.addSubnet(network, prefix, 'ipv4'); | ||
| const block6 = (network, prefix) => BLOCKED6.addSubnet(network, prefix, 'ipv6'); | ||
|
|
||
| for (const [network, prefix] of [ | ||
| ['0.0.0.0', 8], ['10.0.0.0', 8], ['100.64.0.0', 10], ['127.0.0.0', 8], | ||
| ['169.254.0.0', 16], ['172.16.0.0', 12], ['192.0.0.0', 24], ['192.0.2.0', 24], | ||
| ['192.88.99.0', 24], ['192.168.0.0', 16], ['198.18.0.0', 15], ['198.51.100.0', 24], | ||
| ['203.0.113.0', 24], ['224.0.0.0', 4], ['240.0.0.0', 4], | ||
| ]) block4(network, prefix); | ||
|
|
||
| for (const [network, prefix] of [ | ||
| ['::', 96], ['::1', 128], ['::ffff:0:0', 96], ['64:ff9b:1::', 48], | ||
| ['100::', 64], ['2001:2::', 48], ['2001:10::', 28], ['2001:20::', 28], | ||
| ['2001:db8::', 32], ['2002::', 16], ['fc00::', 7], ['fe80::', 10], | ||
| ['fec0::', 10], ['ff00::', 8], | ||
| ]) block6(network, prefix); | ||
|
|
||
| const RFC6052_WKP = new BlockList(); | ||
| RFC6052_WKP.addSubnet('64:ff9b::', 96, 'ipv6'); | ||
|
|
||
| const unbracket = (hostname) => hostname.startsWith('[') && hostname.endsWith(']') | ||
| ? hostname.slice(1, -1) | ||
| : hostname; | ||
|
|
||
| function expandIpv6Words(address) { | ||
| const halves = address.toLowerCase().split('::'); | ||
| const parseHalf = (half) => { | ||
| if (!half) return []; | ||
| return half.split(':').flatMap((part) => { | ||
| if (!part.includes('.')) return [Number.parseInt(part, 16)]; | ||
| const octets = part.split('.').map(Number); | ||
| return [(octets[0] << 8) | octets[1], (octets[2] << 8) | octets[3]]; | ||
| }); | ||
| }; | ||
| const left = parseHalf(halves[0]); | ||
| const right = parseHalf(halves[1] || ''); | ||
| const zeroCount = halves.length === 2 ? 8 - left.length - right.length : 0; | ||
| return halves.length === 2 | ||
| ? [...left, ...Array(zeroCount).fill(0), ...right] | ||
| : left; | ||
| } | ||
|
|
||
| function rfc6052EmbeddedIpv4(address) { | ||
| const words = expandIpv6Words(address); | ||
| if (words.length !== 8) return null; | ||
| return [words[6] >> 8, words[6] & 0xff, words[7] >> 8, words[7] & 0xff].join('.'); | ||
| } | ||
|
|
||
| export function isPublicWebhookAddress(address) { | ||
| const family = isIP(address); | ||
| if (!family) return false; | ||
| if (family === 6 && RFC6052_WKP.check(address, 'ipv6')) { | ||
| const embeddedIpv4 = rfc6052EmbeddedIpv4(address); | ||
| return embeddedIpv4 !== null && isPublicWebhookAddress(embeddedIpv4); | ||
| } | ||
| const blocked = family === 4 ? BLOCKED4 : BLOCKED6; | ||
| return !blocked.check(address, family === 4 ? 'ipv4' : 'ipv6'); | ||
| } | ||
|
|
||
| export function parseWebhookUrl(urlText) { | ||
| let url; | ||
| try { | ||
| url = new URL(String(urlText)); | ||
| } catch { | ||
| throw new TypeError('webhook URL is invalid'); | ||
| } | ||
| if (url.protocol !== 'https:') throw new TypeError('webhook URL must use https'); | ||
| if (url.username || url.password) throw new TypeError('webhook URL must not contain credentials'); | ||
| if (!url.hostname) throw new TypeError('webhook URL must contain a host'); | ||
| return url; | ||
| } | ||
|
|
||
| function withTimeout(promise, timeoutMs, message) { | ||
| let timer; | ||
| const timeout = new Promise((_, reject) => { | ||
| timer = setTimeout(() => reject(new Error(message)), timeoutMs); | ||
| }); | ||
| return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); | ||
| } | ||
|
|
||
| export async function resolvePublicWebhookTarget(urlText, { | ||
| lookup = dnsLookup, | ||
| dnsTimeoutMs = 1000, | ||
| } = {}) { | ||
| const url = parseWebhookUrl(urlText); | ||
| const hostname = unbracket(url.hostname); | ||
| const literalFamily = isIP(hostname); | ||
| const resolved = literalFamily | ||
| ? [{ address: hostname, family: literalFamily }] | ||
| : await withTimeout( | ||
| lookup(hostname, { all: true, verbatim: true }), | ||
| dnsTimeoutMs, | ||
| 'webhook DNS resolution timed out', | ||
| ); | ||
|
|
||
| if (!Array.isArray(resolved) || resolved.length === 0) { | ||
| throw new Error('webhook host did not resolve'); | ||
| } | ||
| const unique = []; | ||
| const seen = new Set(); | ||
| for (const result of resolved) { | ||
| const address = result?.address; | ||
| const family = Number(result?.family) || isIP(address); | ||
| if ((family !== 4 && family !== 6) || !isPublicWebhookAddress(address)) { | ||
| throw new Error('webhook host resolved to a non-public address'); | ||
| } | ||
| const key = `${family}:${address}`; | ||
| if (!seen.has(key)) { | ||
| seen.add(key); | ||
| unique.push({ address, family }); | ||
| } | ||
| } | ||
| return { url, hostname, addresses: unique }; | ||
| } | ||
|
|
||
| export async function postWebhookOnce({ | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| url: urlText, | ||
| headers, | ||
| body, | ||
| lookup = dnsLookup, | ||
| request = httpsRequest, | ||
| dnsTimeoutMs = 1000, | ||
| connectTimeoutMs = 1500, | ||
| requestTimeoutMs = 3000, | ||
| maxResponseHeaderBytes = 16384, | ||
| }) { | ||
| const target = await resolvePublicWebhookTarget(urlText, { lookup, dnsTimeoutMs }); | ||
| const { address, family } = target.addresses[0]; | ||
| const controller = new AbortController(); | ||
| const overallTimer = setTimeout(() => controller.abort(new Error('webhook request timed out')), requestTimeoutMs); | ||
|
|
||
| try { | ||
| return await new Promise((resolve, reject) => { | ||
| let settled = false; | ||
| const finish = (fn, value) => { | ||
| if (settled) return; | ||
| settled = true; | ||
| fn(value); | ||
| }; | ||
| const req = request({ | ||
| protocol: 'https:', | ||
| hostname: target.hostname, | ||
| port: target.url.port || 443, | ||
| path: `${target.url.pathname}${target.url.search}`, | ||
| method: 'POST', | ||
| headers, | ||
| maxHeaderSize: maxResponseHeaderBytes, | ||
| rejectUnauthorized: true, | ||
| servername: isIP(target.hostname) ? undefined : target.hostname, | ||
| signal: controller.signal, | ||
| lookup: (_hostname, _options, callback) => callback(null, address, family), | ||
| }, (response) => { | ||
| const status = response.statusCode ?? 0; | ||
| finish(resolve, { status, ok: status >= 200 && status < 300 }); | ||
| response.destroy(); | ||
| }); | ||
|
|
||
| let connectTimer; | ||
| req.once('socket', (socket) => { | ||
| connectTimer = setTimeout(() => req.destroy(new Error('webhook connect timed out')), connectTimeoutMs); | ||
| socket.once('secureConnect', () => clearTimeout(connectTimer)); | ||
| }); | ||
| req.once('error', (error) => { | ||
| clearTimeout(connectTimer); | ||
| finish(reject, error); | ||
| }); | ||
| req.end(body); | ||
| }); | ||
| } finally { | ||
| clearTimeout(overallTimer); | ||
| } | ||
| } | ||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.