docs(sandbox): the forensic-egress allowlist is a destination filter, not read-only - #3075
Merged
Merged
Conversation
… not read-only The allowlist was headed "Retrieval-only domains available to proxy-aware forensic samples", and the sandbox README described the same thing as a "retrieval proxy". Neither is enforced. Squid gates these names with `dstdomain` on CONNECT and then parses no HTTP inside the established tunnel, so it cannot distinguish a fetch from an upload, a clone from a push, or a raw read from a Gist/release/API write. Plain HTTP to port 80 against the same names is permitted too, since the allow rule is not CONNECT-restricted. The size caps do not close that either: `request_body_max_size` and `reply_body_max_size` act on message bodies squid parses, and a CONNECT tunnel has none, so tunneled traffic is unbounded in both directions. What actually remains is the access_log entry for the CONNECT -- host, port and byte counts, i.e. after-the-fact detection of volume, not prevention and not content. Says so, in both places, and notes that .github.io is shared multi-tenant hosting so an attacker-controlled <name>.github.io is inside the list by construction. Documentation only -- no enforcement change, and this deliberately forecloses none of the directions in #3072 (TLS interception, splitting read-only from write-capable destinations). Squid ignores `#` lines in an ACL file, and nothing but squid reads this file. Refs #3072
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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
1 participant
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.
Refs #3072 — deliberately not
Closes. This lands only the cheap half of that issue: the stated-intent/actual-enforcement mismatch. The enforcement gap itself (an allowlisted domain is a bidirectional channel) is untouched, and both of #3072's real directions — TLS interception, or splitting read-only from write-capable destinations — stay open and unforeclosed.What was wrong
sandbox/forensic-egress-allowed-domains.txtwas headed# Retrieval-only domains available to proxy-aware forensic samples., anddocs/sandbox/README.mddescribed the same component as a "retrieval proxy". Neither property is enforced anywhere:acl allowed_forensic_domains dstdomain "/etc/honeypot-sandbox/allowed-domains.txt", gatingCONNECT. Once the TLS tunnel is established squid parses no HTTP inside it — it cannot distinguish a fetch from an upload, agit clonefrom agit push, or a raw read from a Gist/release/API write.safe_portsincludes 80 and the allow rule is notconnect_method-restricted, so plain-HTTPPOSTto an allowlisted domain is permitted as well.reply_body_max_size/request_body_max_sizedo not bound the tunnel. They act on HTTP message bodies squid parses, and aCONNECTtunnel has none — so tunneled traffic is size-unbounded in both directions, not capped at 1 MB.access_logentry for theCONNECT: destination host, port, transferred byte counts. After-the-fact detection of volume, not prevention, and not content..github.iocompounds it: GitHub Pages is shared multi-tenant hosting, so an attacker-controlled<name>.github.iois inside the list by construction.What this changes
Documentation only. The allowlist header now states the operator intent and the enforcement boundary, and the README's provisioning step says the allowlist bounds where a sample may connect, not what it may do there.
No behavioural change: squid ignores
#lines in an ACL file, andsandbox/install-forensic-egress.sh→forensic-egress-squid.confis the only consumer of this file.