fix(storage): reject embedded IPv4 rebinding hosts - #1112
Draft
seonghobae wants to merge 10 commits into
Draft
Conversation
Close the buyer-visible gap where each product could invent S3 or S3-compatible endpoint, credential, encryption, integrity, retention, and rollback rules. Central .github now owns a provider-neutral object_storage policy plus a fail-closed validator; Naruon keeps its adapter (ContextualWisdomLab/naruon#1364). Closes #1019
Reject localhost, metadata, IPv4, Unicode, and case-alias hosts; bound TCP ports; refuse non-finite JSON; close nested schema objects; and fail closed on malformed observability labels without TypeError.
The one-shot repair workflow already applied its hardening and would fail or revert ARCHITECTURE.md if rerun. Removing it clears the Scorecard Token-Permissions finding on workflow-default contents: write. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Strix proved decimal IPv4 hosts such as 2851992574 passed is_exact_dns_host under an explicit allowlist. Reject IP literals, decimal aliases, hexadecimal aliases, and dotted-numeric names. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reject multicast .local, metadata, and cluster-local names from exact allowlists, and reject RFC 6761/6762 internal suffixes when private-network trust is denied. Require tenant-purpose binding, align the portable schema with the executable values, and add the product write/read/delete acceptance template so sibling adapters can prove compatibility. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Close the remaining #1019 SSRF holes on the object-storage contract: pin resolved addresses, reject DNS-rebinding helper suffixes, and deny Kubernetes .svc names unless explicitly named. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Helper-TLD denylists still admitted 169.254.169.254.attacker.example and 32-bit decimal or hex aliases under an ordinary suffix. Fail closed on those embeddings while keeping single octet DNS labels valid. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nip.io-style A-B-C-D and AWS ip-A-B-C-D labels still passed the dotted embed check. Treat those labels as rebinding hosts while leaving three-part or non-octet hyphen labels valid. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.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
2 participants
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.
Buyer-visible gap
#1030 at
ed8d782calready requires DNS pinning, rejects helper suffixes such as.nip.io, and couplesconsumedto explicit zero retention. Rebinding hostnames that are not those helper TLDs still passedis_exact_dns_host:169.254.169.254.attacker.example2851992574.attacker.exampleA-B-C-D/ip-A-B-C-Dlabels such as169-254-169-254.attacker.exampleandip-169-254-169-254.ec2.internalA later TTL or attacker-controlled suffix can still point an allowlisted name at link-local metadata.
What this PR adds
0xhexadecimal label that encodes a 32-bit IPv4 address (> 255and<= 0xFFFFFFFF)ip-A-B-C-Dform1.s3.amazonaws.comand non-octet hyphen labels such as1-2-3.example.comvalid0xFFFFFFFFare not treated as IPv4 aliasesNaruon remains the adapter owner (ContextualWisdomLab/naruon#1364). Do not close #1019 until that consumer's current-head write/read/delete lane is green.
Verification
Local
coverage run -m pytest tests && coverage report --fail-under=100(1190 passed) andinterrogateboth passed at 100%.Merge
Successor to #1030. Do not merge #1030 at
ed8d782cwithout this embedding check. Do not self-approve.