fix(example): close the four examples/ review findings - #68
Merged
Conversation
bufio + ReadString left the password in a 4 KiB reader buffer and two immutable strings, none of them wipeable — in the example whose whole point is that the password stops existing. Read it a byte at a time into a SecureBuffer instead, with the terminal in raw mode so it is not echoed either. Review findings password-login/main.go:68 and :69.
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.
The
examples/module had four open findings and no fixes had touched it. For alibrary whose examples are its documentation, that was the largest gap left.
password-login—dc28356:69—bufio.NewReader().ReadString()left the password in a 4 KiBreader buffer and two immutable Go strings. A string cannot be wiped and none
of those copies were reachable to try, so only the final
[]bytewas erased —in the example whose stated point is that the password stops existing.
:68— typed with terminal echo on.Now read a byte at a time straight into a
SecureBuffer, raw mode when stdin isa TTY. No
bufio, no intermediate string, noappendgrowth.hardened-ssh-agent—9a7ffe3:149— the README claimed parity withssh-agent, which callsgetpeereidon every accept. This one served anyone who could reach thesocket. Peer uid must now equal ours, root excepted, exactly as
ssh-agentdoes.
:108— the socket directory was named after the pid. Pids are small andenumerable, so a local process could pre-create the directory for likely pids
and deny the agent a socket. Now
MkdirTemp.Peer credentials come from
SO_PEERCREDon Linux andLOCAL_PEERCREDondarwin/FreeBSD. OpenBSD and NetBSD have no typed accessor in
x/sys, so theyrefuse to start rather than serve connections they cannot attribute.
Verified
password rejected, EOF with no trailing newline, and >256 bytes rejected
rather than truncated — truncation would let a prefix log in.
TestPeerUID_ReportsOurOwnUIDcovers the new gate. A brokenpeerUIDwouldreject every connection and present as a networking fault, not a security
control misfiring. The reject path needs a second uid and is not reachable
from a test process.
where relevant).
go vet,go build -mod=readonly,go test -race ./...cleanunder
GOWORK=off.go.modgainsgolang.org/x/termand promotesgolang.org/x/systo direct.The
secmemandsecmem-cryptorequire lines are untouched.