Repository navigation
netutils: do not panic on a non-IPv4 MD5 address on OpenBSD - #3613
Merged
Merged
Conversation
On OpenBSD, SetTCPMD5SigSockopt installs the key through PF_KEY, and rfkeyRequest builds the sockaddr_in for each end with newSockaddrIn. newSockaddrIn calls net.ParseIP(addr).To4() and indexes the result without checking it, so any address that is not a plain IPv4 address makes it index a nil slice and panic. The server hands it two such addresses. A neighbor with an IPv6 address and an auth password reaches it through addAuthKeysToListeners, and a dynamic neighbor whose peer group has a password reaches it with the prefix string, e.g. "10.0.0.0/24". Either configuration takes gobgpd down instead of logging the "failed to set md5" warning the callers already have for this case. Return an error from newSockaddrIn when the address has no IPv4 form, and pass it up from rfkeyRequest before anything is written to the PF_KEY socket.
Member
|
Thanks |
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.
On OpenBSD,
SetTCPMD5SigSockoptinstalls the MD5 key through PF_KEY, andrfkeyRequestbuilds thesockaddr_infor each end withnewSockaddrIn. That function doesnet.ParseIP(addr).To4()and indexes the result without checking it, so any address that is not a plain IPv4 address panics withindex out of range [3] with length 0.The server passes it two kinds of such addresses: an IPv6 neighbor with
auth-password(throughaddAuthKeysToListeners), and a dynamic neighbor whose peer group has a password, where the prefix string such as10.0.0.0/24is passed as the address. Either one crashes gobgpd, even though every caller already logs a "failed to set md5" warning when an error comes back. With this change,newSockaddrInreturns an error when the address has no IPv4 form, andrfkeyRequestreturns it before anything is written to the PF_KEY socket.newSockaddrInis unexported and lives underinternal/, so changing its signature affects nothing outside this file, and the empty-address case works as before.CI only cross-builds openbsd, and I have no OpenBSD machine, so here is how I checked it. The new test is
//go:build openbsd.GOOS=openbsd go vetandGOOS=openbsd go test -cboth succeed. To run it, I copiedsockopt_openbsd.goand the test into a scratch package on Linux under neutral file names with the build tag removed, next to the other untagged netutils files. The code under test is plain Go and fails before it touches the socket. With the currentsockopt_openbsd.go,TestSaAddRejectsNonIPv4Addresspanics innewSockaddrIn(called fromrfkeyRequestviasaAdd). With the patched file, the test passes.GOOS=openbsd,freebsdandlinuxgo build ./...are also clean.Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.