feat(offchain): add --allow-private-offchain-urls flag#2131
Draft
ArturWieczorek wants to merge 2 commits into
Draft
feat(offchain): add --allow-private-offchain-urls flag#2131ArturWieczorek wants to merge 2 commits into
ArturWieczorek wants to merge 2 commits into
Conversation
ArturWieczorek
force-pushed
the
allow-private-offchain-urls
branch
6 times, most recently
from
62bb83d to
288cbac
Compare
cardano-db-sync's off-chain pool and vote metadata fetchers reject URLs whose host or resolved IP is in a private, loopback, or link-local range as an SSRF mitigation (see parseOffChainUrl and newRestrictedManager in cardano-db-sync/src/Cardano/DbSync/ OffChain/Http.hs). Local-cluster test setups whose metadata is served from http://localhost:.../poolN.json or similar therefore cannot exercise the success path of the fetcher. This adds an opt-in CLI flag that replaces both layers of the restriction with a no-op for the duration of the run, intended for local-cluster testing only. Off by default; the existing restriction continues to apply to every production deployment. * New SyncNodeParams field enpAllowPrivateOffChainUrls plumbed through SyncOptions (soptAllowPrivateOffChainUrls) into the off-chain fetcher threads. * parseOffChainUrl now takes a Bool: when True the isLocalhostHost rejection is skipped. * newRestrictedManager now takes a Bool: when True it installs a permissiveRestriction (addressRestriction returning Nothing for every address) instead of offchainRestriction, so DNS-resolved private IPs are no longer blocked at connect time either. * New Hedgehog suite Cardano.DbSync.OffChain.HttpTest covering both modes for localhost/127./[::1]/10./192.168. plus a public URL and non-HTTP schemes.
Adds a table-driven Hedgehog property that exercises every classification branch of isPrivateAddr — IPv4-mapped IPv6 (the SSRF gap fixed in #2132), native IPv6, and IPv4 — with range-edge cases included to guard against
ArturWieczorek
force-pushed
the
allow-private-offchain-urls
branch
from
288cbac to
b86b22a
Compare
Contributor
|
looks good so far, but could we have it as a config option rather than a command line flag? |
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.
Description
cardano-db-sync's off-chain pool and vote metadata fetchers reject URLs whose host or resolved IP is in a private, loopback, or link-local range as an SSRF mitigation.
Local-cluster test setups whose metadata is served from http://localhost:.../poolN.json or similar therefore cannot exercise the success path of the fetcher.
This adds an opt-in CLI flag that replaces both layers of the restriction with a no-op for the duration of the run, intended
for local-cluster testing only. Off by default; the existing restriction continues to apply to every production deployment.
Checklist
fourmoluon version 0.17.0.0 (which can be run withscripts/fourmolize.sh)Migrations
If there is a breaking change, especially a big one, please add a justification here. Please elaborate
more what the migration achieves, what it cannot achieve or why a migration is not possible.