feat(workflow): operator CIDR allowlist for webhook SSRF check - #6553
Open
sanjay3290 wants to merge 5 commits into
Open
feat(workflow): operator CIDR allowlist for webhook SSRF check#6553sanjay3290 wants to merge 5 commits into
sanjay3290 wants to merge 5 commits into
Conversation
Co-authored-by: Sanjay Ramadugu <sramadugu1@gmail.com> Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
Co-authored-by: Sanjay Ramadugu <sramadugu1@gmail.com> Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
…n in workflow Co-authored-by: Sanjay Ramadugu <sramadugu1@gmail.com> Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
Co-authored-by: Sanjay Ramadugu <sramadugu1@gmail.com> Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
Co-authored-by: Sanjay Ramadugu <sramadugu1@gmail.com> Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
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.
Closes #6500.
call_webhook's SSRF check rejects every private/reserved range, including100.64.0.0/10. On a Tailscale deployment that range is the operator's internal network, socall_webhookcannot reach anything they run. This adds an opt-in, operator-controlled CIDR allowlist. The default is empty, so behaviour is byte-for-byte unchanged for anyone who does not set the variable.What changed
buzz-core::networkgainsIpCidr(withcontains),parse_allowed_cidrs, andis_blocked_ip(ip, allowed) == is_private_ip(ip) && !allowed.contains(ip). Hand-rolled masking, no new dependency.buzz-workflow::executorgainsssrf_verdict(host, addrs, allowed_spec).check_ssrfis now resolve-then-delegate, readingBUZZ_WORKFLOW_WEBHOOK_ALLOWED_CIDRS.tracing::warn!naming the variable.buzz-corehas notracingdependency and does no logging itself..env.exampledocuments the variable.parse_allowed_cidrsfails closed: an unparseable or over-wide entry is dropped rather than widened, and a bare address with no/prefixis dropped rather than treated as a host route.Scope
This is option 1 of the two the issue asks for. Option 2 — surfacing blocked destinations as an explicit step error instead of a silent non-delivery — is #5122 and is not addressed here. The issue says option 1 alone unblocks the reported case.
Verification
ssrf_verdictand its five tests are behind#[cfg(feature = "reqwest")], matching every other item in that block. Two things follow, and both matter when reading a green pipeline:./scripts/run-tests.sh unitdoes not runbuzz-workflowat all. Its suites are buzz-core, buzz-auth, buzz-voice, buzz-cli, buzz-db, buzz-conformance, buzz-push-gateway, buzz-backend-kubernetes, buzz-agent. Only thebuzz-corehalf of this change is covered there.cargo clippy --workspacecannot see a cfg mismatch on this function, because feature unification turnsreqweston. An earlier revision of this branch was missing the#[cfg]and passed workspace clippy whilecargo clippy -p buzz-workflow --all-targets -- -D warningsfailed on dead code. Reviewers should run the per-crate form.Run
cargo test -p buzz-workflow --features reqwestto exercise the workflow-side tests; without the feature they are filtered out (measured: 5 run with it, 0 without).Gates run on the branch and again on the branch merged with
main:Mutation probes, with the baseline restored and re-confirmed green between each:
is_blocked_ipdrops the allowlist termparse_allowed_cidrsreturns an empty dropped listIpCidr::containsalways returns truessrf_verdictcall fromcheck_ssrfenv::varargument with a literal""The last row is the one worth calling out. Deleting the call site is caught by the dead-code lint, but silently disabling the feature by breaking the variable name is not — that is what
check_ssrf_reads_allowlist_env_varexists to catch. It drivescheck_ssrfthrough a current-thread runtime under a mutex, and needs no network:"127.0.0.1:80".to_socket_addrs()is a pure parse.