From 9c82a8cf7ac23b0c34d335ffae37db2995a5ad37 Mon Sep 17 00:00:00 2001 From: cuishuang Date: Thu, 27 Feb 2025 12:42:42 +0800 Subject: [PATCH 1/6] oauth2.go: use a more straightforward return value Change-Id: I72d94836c93dabe50fe67ddb762389f674ba6490 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/653215 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Pratt Auto-Submit: Ian Lance Taylor --- oauth2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2.go b/oauth2.go index 74f052aa9..a1a44c9b4 100644 --- a/oauth2.go +++ b/oauth2.go @@ -288,7 +288,7 @@ func (tf *tokenRefresher) Token() (*Token, error) { if tf.refreshToken != tk.RefreshToken { tf.refreshToken = tk.RefreshToken } - return tk, err + return tk, nil } // reuseTokenSource is a TokenSource that holds a single token in memory From 44967abe9097d1169118ef5de80b29ef293edc15 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Fri, 31 Jan 2025 00:35:58 +0200 Subject: [PATCH 2/6] google: fix typos Change-Id: I8fc92767c1ea73e731736056c72628977017301e Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/645775 Reviewed-by: Michael Pratt Auto-Submit: Sean Liao LUCI-TryBot-Result: Go LUCI Reviewed-by: Sean Liao Reviewed-by: Junyang Shao --- google/externalaccount/basecredentials.go | 2 +- google/externalaccount/basecredentials_test.go | 6 +++--- google/externalaccount/executablecredsource_test.go | 8 ++++---- google/google_test.go | 2 +- .../externalaccountauthorizeduser_test.go | 8 ++++---- google/internal/stsexchange/sts_exchange_test.go | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/google/externalaccount/basecredentials.go b/google/externalaccount/basecredentials.go index fc106347d..aa0bba2eb 100644 --- a/google/externalaccount/basecredentials.go +++ b/google/externalaccount/basecredentials.go @@ -263,7 +263,7 @@ const ( fileTypeJSON = "json" ) -// Format contains information needed to retireve a subject token for URL or File sourced credentials. +// Format contains information needed to retrieve a subject token for URL or File sourced credentials. type Format struct { // Type should be either "text" or "json". This determines whether the file or URL sourced credentials // expect a simple text subject token or if the subject token will be contained in a JSON object. diff --git a/google/externalaccount/basecredentials_test.go b/google/externalaccount/basecredentials_test.go index 8f165cdb0..d52f6a789 100644 --- a/google/externalaccount/basecredentials_test.go +++ b/google/externalaccount/basecredentials_test.go @@ -347,12 +347,12 @@ func TestNonworkforceWithWorkforcePoolUserProject(t *testing.T) { t.Fatalf("Expected error but found none") } if got, want := err.Error(), "oauth2/google/externalaccount: Workforce pool user project should not be set for non-workforce pool credentials"; got != want { - t.Errorf("Incorrect error received.\nExpected: %s\nRecieved: %s", want, got) + t.Errorf("Incorrect error received.\nExpected: %s\nReceived: %s", want, got) } } func TestWorkforcePoolCreation(t *testing.T) { - var audienceValidatyTests = []struct { + var audienceValidityTests = []struct { audience string expectSuccess bool }{ @@ -371,7 +371,7 @@ func TestWorkforcePoolCreation(t *testing.T) { } ctx := context.Background() - for _, tt := range audienceValidatyTests { + for _, tt := range audienceValidityTests { t.Run(" "+tt.audience, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability. config := testConfig config.TokenURL = "https://sts.googleapis.com" // Setting the most basic acceptable tokenURL diff --git a/google/externalaccount/executablecredsource_test.go b/google/externalaccount/executablecredsource_test.go index 69ec21ae1..3ecc05f92 100644 --- a/google/externalaccount/executablecredsource_test.go +++ b/google/externalaccount/executablecredsource_test.go @@ -654,7 +654,7 @@ func TestRetrieveOutputFileSubjectTokenNotJSON(t *testing.T) { if _, err = base.subjectToken(); err == nil { t.Fatalf("Expected error but found none") } else if got, want := err.Error(), jsonParsingError(outputFileSource, "tokentokentoken").Error(); got != want { - t.Errorf("Incorrect error received.\nExpected: %s\nRecieved: %s", want, got) + t.Errorf("Incorrect error received.\nExpected: %s\nReceived: %s", want, got) } _, deadlineSet := te.getDeadline() @@ -801,7 +801,7 @@ func TestRetrieveOutputFileSubjectTokenFailureTests(t *testing.T) { if _, err = ecs.subjectToken(); err == nil { t.Errorf("Expected error but found none") } else if got, want := err.Error(), tt.expectedErr.Error(); got != want { - t.Errorf("Incorrect error received.\nExpected: %s\nRecieved: %s", want, got) + t.Errorf("Incorrect error received.\nExpected: %s\nReceived: %s", want, got) } if _, deadlineSet := te.getDeadline(); deadlineSet { @@ -923,7 +923,7 @@ func TestRetrieveOutputFileSubjectTokenInvalidCache(t *testing.T) { } if got, want := out, "tokentokentoken"; got != want { - t.Errorf("Incorrect token received.\nExpected: %s\nRecieved: %s", want, got) + t.Errorf("Incorrect token received.\nExpected: %s\nReceived: %s", want, got) } }) } @@ -1012,7 +1012,7 @@ func TestRetrieveOutputFileSubjectTokenJwt(t *testing.T) { if out, err := ecs.subjectToken(); err != nil { t.Errorf("retrieveSubjectToken() failed: %v", err) } else if got, want := out, "tokentokentoken"; got != want { - t.Errorf("Incorrect token received.\nExpected: %s\nRecieved: %s", want, got) + t.Errorf("Incorrect token received.\nExpected: %s\nReceived: %s", want, got) } if _, deadlineSet := te.getDeadline(); deadlineSet { diff --git a/google/google_test.go b/google/google_test.go index 7078d429f..5aa5e2845 100644 --- a/google/google_test.go +++ b/google/google_test.go @@ -72,7 +72,7 @@ func TestConfigFromJSON(t *testing.T) { t.Errorf("ClientSecret = %q; want %q", got, want) } if got, want := conf.RedirectURL, "https://www.example.com/oauth2callback"; got != want { - t.Errorf("RedictURL = %q; want %q", got, want) + t.Errorf("RedirectURL = %q; want %q", got, want) } if got, want := strings.Join(conf.Scopes, ","), "scope1,scope2"; got != want { t.Errorf("Scopes = %q; want %q", got, want) diff --git a/google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser_test.go b/google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser_test.go index 94bfee3d6..1bbbbac19 100644 --- a/google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser_test.go +++ b/google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser_test.go @@ -38,7 +38,7 @@ type testRefreshTokenServer struct { server *httptest.Server } -func TestExernalAccountAuthorizedUser_JustToken(t *testing.T) { +func TestExternalAccountAuthorizedUser_JustToken(t *testing.T) { config := &Config{ Token: "AAAAAAA", Expiry: now().Add(time.Hour), @@ -57,7 +57,7 @@ func TestExernalAccountAuthorizedUser_JustToken(t *testing.T) { } } -func TestExernalAccountAuthorizedUser_TokenRefreshWithRefreshTokenInRespondse(t *testing.T) { +func TestExternalAccountAuthorizedUser_TokenRefreshWithRefreshTokenInResponse(t *testing.T) { server := &testRefreshTokenServer{ URL: "/", Authorization: "Basic Q0xJRU5UX0lEOkNMSUVOVF9TRUNSRVQ=", @@ -99,7 +99,7 @@ func TestExernalAccountAuthorizedUser_TokenRefreshWithRefreshTokenInRespondse(t } } -func TestExernalAccountAuthorizedUser_MinimumFieldsRequiredForRefresh(t *testing.T) { +func TestExternalAccountAuthorizedUser_MinimumFieldsRequiredForRefresh(t *testing.T) { server := &testRefreshTokenServer{ URL: "/", Authorization: "Basic Q0xJRU5UX0lEOkNMSUVOVF9TRUNSRVQ=", @@ -187,7 +187,7 @@ func TestExternalAccountAuthorizedUser_MissingRefreshFields(t *testing.T) { }, }, { - name: "missing client secrect", + name: "missing client secret", config: Config{ RefreshToken: "BBBBBBBBB", TokenURL: url, diff --git a/google/internal/stsexchange/sts_exchange_test.go b/google/internal/stsexchange/sts_exchange_test.go index 895b9bcf9..ff9a9ad08 100644 --- a/google/internal/stsexchange/sts_exchange_test.go +++ b/google/internal/stsexchange/sts_exchange_test.go @@ -142,7 +142,7 @@ func TestExchangeToken_Opts(t *testing.T) { } strOpts, ok := data["options"] if !ok { - t.Errorf("Server didn't recieve an \"options\" field.") + t.Errorf("Server didn't receive an \"options\" field.") } else if len(strOpts) < 1 { t.Errorf("\"options\" field has length 0.") } From ce350bff61d844ec163c2a0c0eaf35c71948cae7 Mon Sep 17 00:00:00 2001 From: Tim Cooper Date: Mon, 28 May 2018 18:28:30 -0300 Subject: [PATCH 3/6] oauth2: remove unneeded TokenSource implementation in transport test Change-Id: I2dca3a211f7b7e30ae023635c5ac44b6237ee01c Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/114957 Reviewed-by: Michael Pratt Auto-Submit: Sean Liao Reviewed-by: Sean Liao LUCI-TryBot-Result: Go LUCI Reviewed-by: Junyang Shao --- transport_test.go | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/transport_test.go b/transport_test.go index faa87d514..a8e6ea236 100644 --- a/transport_test.go +++ b/transport_test.go @@ -9,12 +9,6 @@ import ( "time" ) -type tokenSource struct{ token *Token } - -func (t *tokenSource) Token() (*Token, error) { - return t.token, nil -} - func TestTransportNilTokenSource(t *testing.T) { tr := &Transport{} server := newMockServer(func(w http.ResponseWriter, r *http.Request) {}) @@ -88,13 +82,10 @@ func TestTransportCloseRequestBodySuccess(t *testing.T) { } func TestTransportTokenSource(t *testing.T) { - ts := &tokenSource{ - token: &Token{ - AccessToken: "abc", - }, - } tr := &Transport{ - Source: ts, + Source: StaticTokenSource(&Token{ + AccessToken: "abc", + }), } server := newMockServer(func(w http.ResponseWriter, r *http.Request) { if got, want := r.Header.Get("Authorization"), "Bearer abc"; got != want { @@ -123,14 +114,11 @@ func TestTransportTokenSourceTypes(t *testing.T) { {key: "basic", val: val, want: "Basic abc"}, } for _, tc := range tests { - ts := &tokenSource{ - token: &Token{ + tr := &Transport{ + Source: StaticTokenSource(&Token{ AccessToken: tc.val, TokenType: tc.key, - }, - } - tr := &Transport{ - Source: ts, + }), } server := newMockServer(func(w http.ResponseWriter, r *http.Request) { if got, want := r.Header.Get("Authorization"), tc.want; got != want { From 0042180b24f3cfe500f4cad3cabbc33c0a341f78 Mon Sep 17 00:00:00 2001 From: Abel Tay Date: Sat, 8 Jun 2019 14:15:54 +0800 Subject: [PATCH 4/6] oauth2: Deep copy context client in NewClient OAuth2 client creation currently doesn't faithfully reuse the client passed into the context. This causes config settings such as timeout to be set to Default and may end up to be a gotcha for anyone who sends in a context client with timeout set assuming that the timeout will be copied to the new client. Fix: https://github.com/golang/oauth2/issues/368 Change-Id: I4f5f052361ebe07f50fbd694379892833cd1056c Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/180920 Auto-Submit: Sean Liao Reviewed-by: Sean Liao Reviewed-by: Junyang Shao LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Pratt --- oauth2.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/oauth2.go b/oauth2.go index a1a44c9b4..eacdd7fd9 100644 --- a/oauth2.go +++ b/oauth2.go @@ -356,11 +356,15 @@ func NewClient(ctx context.Context, src TokenSource) *http.Client { if src == nil { return internal.ContextClient(ctx) } + cc := internal.ContextClient(ctx) return &http.Client{ Transport: &Transport{ - Base: internal.ContextClient(ctx).Transport, + Base: cc.Transport, Source: ReuseTokenSource(nil, src), }, + CheckRedirect: cc.CheckRedirect, + Jar: cc.Jar, + Timeout: cc.Timeout, } } From ce56909505b351a755ad0bc294c5ee01ed0ea050 Mon Sep 17 00:00:00 2001 From: Julien Cretel Date: Tue, 11 Mar 2025 18:26:19 +0000 Subject: [PATCH 5/6] jws: improve fix for CVE-2025-22868 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix for CVE-2025-22868 relies on strings.Count, which isn't ideal because it precludes failing fast when the token contains an unexpected number of periods. Moreover, Verify still allocates more than necessary. Eschew strings.Count in favor of strings.Cut. Some benchmark results: goos: darwin goarch: amd64 pkg: golang.org/x/oauth2/jws cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz │ old │ new │ │ sec/op │ sec/op vs base │ Verify/full_of_periods-8 24862.50n ± 1% 57.87n ± 0% -99.77% (p=0.000 n=20) Verify/two_trailing_periods-8 3.485m ± 1% 3.445m ± 1% -1.13% (p=0.003 n=20) geomean 294.3µ 14.12µ -95.20% │ old │ new │ │ B/op │ B/op vs base │ Verify/full_of_periods-8 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=20) ¹ Verify/two_trailing_periods-8 2.001Mi ± 0% 1.001Mi ± 0% -49.98% (p=0.000 n=20) geomean 5.658Ki 4.002Ki -29.27% ¹ all samples are equal │ old │ new │ │ allocs/op │ allocs/op vs base │ Verify/full_of_periods-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=20) ¹ Verify/two_trailing_periods-8 12.000 ± 0% 9.000 ± 0% -25.00% (p=0.000 n=20) geomean 3.464 3.000 -13.40% ¹ all samples are equal Also, remove all remaining calls to strings.Split. Updates golang/go#71490 Change-Id: Icac3c7a81562161ab6533d892ba19247d6d5b943 GitHub-Last-Rev: 3a82900f747798f5f36065126385880277c0fce7 GitHub-Pull-Request: golang/oauth2#774 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/655455 Commit-Queue: Neal Patel Reviewed-by: Roland Shoemaker LUCI-TryBot-Result: Go LUCI Reviewed-by: Neal Patel Auto-Submit: Neal Patel --- jws/jws.go | 34 +++++++++++++++++++++-------- jws/jws_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/jws/jws.go b/jws/jws.go index 6f03a49d3..27ab06139 100644 --- a/jws/jws.go +++ b/jws/jws.go @@ -116,12 +116,12 @@ func (h *Header) encode() (string, error) { // Decode decodes a claim set from a JWS payload. func Decode(payload string) (*ClaimSet, error) { // decode returned id token to get expiry - s := strings.Split(payload, ".") - if len(s) < 2 { + _, claims, _, ok := parseToken(payload) + if !ok { // TODO(jbd): Provide more context about the error. return nil, errors.New("jws: invalid token received") } - decoded, err := base64.RawURLEncoding.DecodeString(s[1]) + decoded, err := base64.RawURLEncoding.DecodeString(claims) if err != nil { return nil, err } @@ -165,18 +165,34 @@ func Encode(header *Header, c *ClaimSet, key *rsa.PrivateKey) (string, error) { // Verify tests whether the provided JWT token's signature was produced by the private key // associated with the supplied public key. func Verify(token string, key *rsa.PublicKey) error { - if strings.Count(token, ".") != 2 { + header, claims, sig, ok := parseToken(token) + if !ok { return errors.New("jws: invalid token received, token must have 3 parts") } - - parts := strings.SplitN(token, ".", 3) - signedContent := parts[0] + "." + parts[1] - signatureString, err := base64.RawURLEncoding.DecodeString(parts[2]) + signatureString, err := base64.RawURLEncoding.DecodeString(sig) if err != nil { return err } h := sha256.New() - h.Write([]byte(signedContent)) + h.Write([]byte(header + tokenDelim + claims)) return rsa.VerifyPKCS1v15(key, crypto.SHA256, h.Sum(nil), signatureString) } + +func parseToken(s string) (header, claims, sig string, ok bool) { + header, s, ok = strings.Cut(s, tokenDelim) + if !ok { // no period found + return "", "", "", false + } + claims, s, ok = strings.Cut(s, tokenDelim) + if !ok { // only one period found + return "", "", "", false + } + sig, _, ok = strings.Cut(s, tokenDelim) + if ok { // three periods found + return "", "", "", false + } + return header, claims, sig, true +} + +const tokenDelim = "." diff --git a/jws/jws_test.go b/jws/jws_test.go index 39a136a29..1776f56b8 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -7,6 +7,8 @@ package jws import ( "crypto/rand" "crypto/rsa" + "net/http" + "strings" "testing" ) @@ -39,8 +41,57 @@ func TestSignAndVerify(t *testing.T) { } func TestVerifyFailsOnMalformedClaim(t *testing.T) { - err := Verify("abc.def", nil) - if err == nil { - t.Error("got no errors; want improperly formed JWT not to be verified") + cases := []struct { + desc string + token string + }{ + { + desc: "no periods", + token: "aa", + }, { + desc: "only one period", + token: "a.a", + }, { + desc: "more than two periods", + token: "a.a.a.a", + }, + } + for _, tc := range cases { + f := func(t *testing.T) { + err := Verify(tc.token, nil) + if err == nil { + t.Error("got no errors; want improperly formed JWT not to be verified") + } + } + t.Run(tc.desc, f) + } +} + +func BenchmarkVerify(b *testing.B) { + cases := []struct { + desc string + token string + }{ + { + desc: "full of periods", + token: strings.Repeat(".", http.DefaultMaxHeaderBytes), + }, { + desc: "two trailing periods", + token: strings.Repeat("a", http.DefaultMaxHeaderBytes-2) + "..", + }, + } + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + b.Fatal(err) + } + for _, bc := range cases { + f := func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for range b.N { + Verify(bc.token, &privateKey.PublicKey) + } + } + b.Run(bc.desc, f) } } From 65c15a35147ccc5127e9f8cdf2e07837596e56b4 Mon Sep 17 00:00:00 2001 From: Allen Li Date: Wed, 22 May 2024 00:53:22 +0000 Subject: [PATCH 6/6] oauth2: remove extra period Change-Id: I369d3bed0e28427b1e1d416952c3766932ba3773 GitHub-Last-Rev: ca611fe5a361db93b76e758ec7d956b0768039e5 GitHub-Pull-Request: golang/oauth2#724 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/587017 Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Shin Fan Reviewed-by: Ian Lance Taylor --- token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/token.go b/token.go index 109997d77..8c31136c4 100644 --- a/token.go +++ b/token.go @@ -169,7 +169,7 @@ func tokenFromInternal(t *internal.Token) *Token { // retrieveToken takes a *Config and uses that to retrieve an *internal.Token. // This token is then mapped from *internal.Token into an *oauth2.Token which is returned along -// with an error.. +// with an error. func retrieveToken(ctx context.Context, c *Config, v url.Values) (*Token, error) { tk, err := internal.RetrieveToken(ctx, c.ClientID, c.ClientSecret, c.Endpoint.TokenURL, v, internal.AuthStyle(c.Endpoint.AuthStyle), c.authStyleCache.Get()) if err != nil {