Harden proxy, forwarding, and build security - #18
Open
Sniperlyf3 wants to merge 3 commits into
Open
Conversation
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.
Motivation
https://proxy URLs actually negotiate TLS so CONNECT requests and proxy credentials are not sent over plaintext.tailcat.govulncheckingolang.org/x/crypto.Description
dialHTTPConnectProxynow performs a proper TLS dial when given anhttpsproxy URL (addedcrypto/tlsusage) so the proxy handshake is encrypted before sending the HTTP CONNECT and credentials, and a regression testTestDialHTTPSConnectProxyUsesTLSwas added.listenUnixnow usesos.Lstatand only removes an existing path if it is a socket node, refusing to remove regular files or symlinks, and the testTestListenUnixRefusesToRemoveNonSocketwas added.build.shdefault to a pinned commit recorded intailcat.refinstead of upstreammain, and CI job environment usage was updated to reference that pin.golang.org/x/cryptotov0.56.0to address GO-2026-6354/GO-2026-6355, updatedgo.mod/go.sum, and added an in-repoSECURITY_REVIEW.mddocumenting the threat model, findings, and accepted risks.Testing
go test ./...and the repository Go tests passed.go vet ./...and it completed with no issues.go test ./cmd/meowshell -run 'TestDialHTTPSConnectProxyUsesTLS' -vandgo test ./cmd/meowshell -run 'TestListenUnixRefusesToRemoveNonSocket' -v, both passed.go run golang.org/x/vuln/cmd/govulncheck@latest ./...after thex/cryptoupgrade and no reachable vulnerabilities remained; also ranbash -n build.shandgit diff --checkwhich succeeded;dotnet testwas not run because the container lacks thedotnetCLI.Codex Task