Skip to content

Commit 139d6ee

Browse files
samthanawallagopherbot
authored andcommitted
cmd/go: restore netrc preferences for GOAUTH and fix domain lookup
Store netrc lines into the credential map backward so that earlier lines take priority over later lines. This matches Go 1.23 netrc lookup which stopped at the first match it found. Additionally, this fixes a security issue related to domain parsing which could have allowed servers to read credentials belonging to other servers. The fix was to switch from using path.Dir(currentPrefix) to strings.Cut(currentPrefix, "/") Thanks to Juho Forsén of Mattermost for reporting this issue. Fixes #71249 Fixes CVE-2024-45340 Change-Id: I175a00d6d7f4d31c9e4d79b7cf1c2a0ad35b2781 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1781 Reviewed-by: Tatiana Bradley <[email protected]> Commit-Queue: Roland Shoemaker <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/643097 Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Michael Knyszek <[email protected]>
1 parent 2b2314e commit 139d6ee

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

src/cmd/go/internal/auth/auth.go

+11-6
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"log"
1313
"net/http"
1414
"os"
15-
"path"
1615
"path/filepath"
1716
"slices"
1817
"strings"
@@ -73,7 +72,12 @@ func runGoAuth(client *http.Client, res *http.Response, url string) {
7372
if err != nil {
7473
base.Fatalf("go: could not parse netrc (GOAUTH=%s): %v", cfg.GOAUTH, err)
7574
}
76-
for _, l := range lines {
75+
// Process lines in reverse so that if the same machine is listed
76+
// multiple times, we end up saving the earlier one
77+
// (overwriting later ones). This matches the way the go command
78+
// worked before GOAUTH.
79+
for i := len(lines) - 1; i >= 0; i-- {
80+
l := lines[i]
7781
r := http.Request{Header: make(http.Header)}
7882
r.SetBasicAuth(l.login, l.password)
7983
storeCredential(l.machine, r.Header)
@@ -137,11 +141,13 @@ func runGoAuth(client *http.Client, res *http.Response, url string) {
137141
func loadCredential(req *http.Request, url string) bool {
138142
currentPrefix := strings.TrimPrefix(url, "https://")
139143
// Iteratively try prefixes, moving up the path hierarchy.
140-
for currentPrefix != "/" && currentPrefix != "." && currentPrefix != "" {
144+
for {
141145
headers, ok := credentialCache.Load(currentPrefix)
142146
if !ok {
143-
// Move to the parent directory.
144-
currentPrefix = path.Dir(currentPrefix)
147+
currentPrefix, _, ok = strings.Cut(currentPrefix, "/")
148+
if !ok {
149+
return false
150+
}
145151
continue
146152
}
147153
for key, values := range headers.(http.Header) {
@@ -151,7 +157,6 @@ func loadCredential(req *http.Request, url string) bool {
151157
}
152158
return true
153159
}
154-
return false
155160
}
156161

157162
// storeCredential caches or removes credentials (represented by HTTP headers)

src/cmd/go/internal/auth/auth_test.go

+23-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,29 @@ func TestCredentialCache(t *testing.T) {
2525
got := &http.Request{Header: make(http.Header)}
2626
ok := loadCredential(got, tc.machine)
2727
if !ok || !reflect.DeepEqual(got.Header, want.Header) {
28-
t.Errorf("loadCredential:\nhave %q\nwant %q", got.Header, want.Header)
28+
t.Errorf("loadCredential(%q):\nhave %q\nwant %q", tc.machine, got.Header, want.Header)
29+
}
30+
}
31+
32+
// Having stored those credentials, we should be able to look up longer URLs too.
33+
extraCases := []netrcLine{
34+
{"https://api.github.com/foo", "user", "pwd"},
35+
{"https://api.github.com/foo/bar/baz", "user", "pwd"},
36+
{"https://example.com/abc", "", ""},
37+
{"https://example.com/?/../api.github.com/", "", ""},
38+
{"https://example.com/?/../api.github.com", "", ""},
39+
{"https://example.com/../api.github.com/", "", ""},
40+
{"https://example.com/../api.github.com", "", ""},
41+
}
42+
for _, tc := range extraCases {
43+
want := http.Request{Header: make(http.Header)}
44+
if tc.login != "" {
45+
want.SetBasicAuth(tc.login, tc.password)
46+
}
47+
got := &http.Request{Header: make(http.Header)}
48+
loadCredential(got, tc.machine)
49+
if !reflect.DeepEqual(got.Header, want.Header) {
50+
t.Errorf("loadCredential(%q):\nhave %q\nwant %q", tc.machine, got.Header, want.Header)
2951
}
3052
}
3153
}

src/cmd/go/testdata/script/goauth_netrc.txt

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
# credentials passed in HTTPS requests to VCS servers.
33
# See golang.org/issue/26232
44

5-
[short] skip
6-
75
env GOPROXY=direct
86
env GOSUMDB=off
97

@@ -55,11 +53,14 @@ go get vcs-test.golang.org/auth/or401
5553
env NETRC=$WORK/missing
5654
! go get vcs-test.golang.org/auth/or401
5755
stderr '^\tserver response: ACCESS DENIED, buddy$'
58-
5956
-- go.mod --
6057
module private.example.com
6158
-- $WORK/empty --
6259
-- $WORK/netrc --
6360
machine vcs-test.golang.org
6461
login aladdin
6562
password opensesame
63+
# first one should override this one
64+
machine vcs-test.golang.org
65+
login aladdin
66+
password ignored

0 commit comments

Comments
 (0)