fix: dockerfile cleanup and https for IP lookup endpoints#1078
Open
memosr wants to merge 1 commit into
Open
Conversation
Two small but real fixes: 1. geth/Dockerfile (line 32) — Add missing /* to apt cache cleanup. Without the glob, the rm -rf cannot delete the directory (it's not empty), leaving all apt package index files behind and bloating the image. The reth and nethermind Dockerfiles in this repo already use the correct pattern (rm -rf /var/lib/apt/lists/*). 2. op-node-entrypoint and base-consensus-entrypoint — Convert 4 IP lookup URLs from http:// to https:// in both files. The returned IP is used directly as the node's advertised P2P address. Using HTTP means a network-level attacker could intercept the request and inject a forged IP, causing the node to broadcast a wrong address to the entire peer network. All four services (ifconfig.me, api.ipify.org, ipecho.net, v4.ident.me) support HTTPS.
Collaborator
🟡 Heimdall Review Status
|
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.
Summary
Two small but real fixes in different files:
geth/Dockerfile— Fix broken apt cache cleanup that leaves package index files in the imageop-node-entrypoint+base-consensus-entrypoint— Upgrade IP lookup URLs fromhttp://tohttps://(security fix)Fix 1 — Dockerfile apt cleanup
File:
geth/Dockerfile(line 32)Without the glob,
rm -rfcannot delete the directory itself (it's non-empty), leaving all apt package index files behind and bloating the resulting image.The other client Dockerfiles in this repo already use the correct pattern:
reth/Dockerfile:30— ✅/var/lib/apt/lists/*reth/Dockerfile:61— ✅/var/lib/apt/lists/*nethermind/Dockerfile:38— ✅/var/lib/apt/lists/*Only
geth/Dockerfilewas missing the glob.Fix 2 — HTTPS for IP lookup (security)
Files:
op-node-entrypoint(lines 7-10),base-consensus-entrypoint(lines 7-10)Why this matters
The values returned from these endpoints are stored in
$PUBLIC_IPand used directly as the node's advertised P2P address. Over plain HTTP, a network-level attacker positioned between the node and any of these services could:^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$regex checkAll four services support HTTPS — there's no downside to making the switch.
Verification