Skip to content

Commit 8336dfd

Browse files
samthanawallagopherbot
authored andcommittedJan 16, 2025
[release-branch.go1.24] 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 <tatianabradley@google.com> Commit-Queue: Roland Shoemaker <bracewell@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com> (cherry picked from commit 76833d221aa3ccc978b6f41bd24e26babf771375) Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1840 Reviewed-on: https://go-review.googlesource.com/c/go/+/643101 Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Michael Knyszek <mknyszek@google.com>
1 parent 6b60550 commit 8336dfd

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)
Please sign in to comment.