fix(security): require https origins and refuse a prefix-resident cosign - #792
Open
rustytrees wants to merge 1 commit into
Open
fix(security): require https origins and refuse a prefix-resident cosign#792rustytrees wants to merge 1 commit into
rustytrees wants to merge 1 commit into
Conversation
Three integrity gaps on the trust path, none of which needed a bug to exploit — just a manifest or a package doing what it is already allowed to. **Cleartext origins.** The redirect loop already declines an https→http downgrade mid-chain, but the *initial* scheme came straight from a manifest. A tap that writes `http://` for a formula or cask URL got a plaintext fetch, and a cask may legitimately carry no `sha256`, leaving nothing but the transport to substitute against. Every entry point now refuses a cleartext origin. Loopback is exempt so the fixture-server tests keep working without an escape hatch a real deployment could trip over — `cli/tap.zig` already refuses `http://` when registering a tap, so this is that rule applied to the URLs those taps hand back. **cosign resolved from the prefix.** `cosign` is looked up on PATH, which includes `<prefix>/bin` — a directory malt installs packages into. A shim landing there turns every later `mt version update` into a rubber stamp, because the only signal read back is the child's exit status. The lookup now mirrors execvp, resolves both sides, and refuses a verifier inside the prefix. Both paths are realpath'd: comparing a resolved binary against an unresolved prefix is spelling-sensitive (on macOS `/tmp` is a symlink to `/private/tmp`), which the test caught. **install.sh checksum lookup.** `grep "$ARCHIVE_NAME"` treated the name as a regex and matched anywhere on the line, so a longer filename containing this one also hit and made EXPECTED multi-line. checksums.txt is cosign-verified first, so this was never exploitable — it just should not depend on that ordering. Now a fixed-string match on the two-space GoReleaser separator.
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.
Third slice of #789 — the integrity gaps on the trust path. None of these needed a bug to exploit; they follow from a manifest or a package doing what it is already allowed to do.
Cleartext origins are refused
The redirect handling in
net/client.zigis careful: credentials are dropped on a cross-domain hop and an https→http downgrade mid-chain is refused outright. But the initial scheme came straight from a manifest. A tap that writeshttp://for a formula or cask URL got a plaintext fetch — and since a cask may legitimately carry nosha256, on that path there can be nothing but the transport standing between the manifest and an installed artifact.Every entry point (
get,getWithHeaders,getToWriter,head) now refuses a cleartext origin.cli/tap.zigalready rejectshttp://when registering a tap; this is the same rule applied to the URLs those taps hand back.Loopback (
127.0.0.1,localhost,::1) is exempt so the fixture-server tests keep working. That felt better than an env-var opt-out, which a real deployment could trip over — and the tests pin that the carve-out is a host match, not a substring, sohttp://evil.tld/127.0.0.1/…andhttp://127.0.0.1.evil.tld/…are both still refused.cosign is not trusted from inside the prefix
cosignis resolved offPATH, which includes<prefix>/bin— a directory malt installs packages into. A shim landing there turns every latermt version updateinto a rubber stamp, because the only signal this module reads back is the child's exit status.The lookup now mirrors execvp (first PATH hit wins), resolves the candidate, and refuses one that lands inside the prefix. Both sides are realpath'd: comparing a resolved binary against an unresolved prefix is spelling-sensitive — on macOS
/tmpis a symlink to/private/tmp— which the test caught before this went anywhere.New
CosignUntrustederror with its own message, rather than folding intoCosignNotFound; "install cosign" would be the wrong advice when cosign is present but in the wrong place. Zig's exhaustive switches pointed at both call sites that had to decide.install.sh checksum lookup
grep "$ARCHIVE_NAME"treated the filename as a regex and matched anywhere on the line, so a longer name containing this one would also hit and makeEXPECTEDmulti-line.checksums.txtis cosign-verified before this runs, so it was never exploitable — it just should not depend on that ordering. Now a fixed-string match anchored on the two-space GoReleaser separator.Verification
zig build test— full suite green./scripts/test/install_sh_test.sh— 13/13 (covers the changed line)./scripts/lint-spawn-invariants.sh— clean./scripts/smokes/smoke_security.sh— 8/8