diff --git a/src/internal/auth/capi_client.go b/src/internal/auth/capi_client.go index 3c6d07a5..9c46c2f4 100644 --- a/src/internal/auth/capi_client.go +++ b/src/internal/auth/capi_client.go @@ -96,7 +96,7 @@ func (c *CAPIClient) IsAuthorized(sourceId string, clientToken string) bool { } func (c *CAPIClient) HasApp(sourceID, authToken string) bool { - req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/apps/"+sourceID, nil) + req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/apps/"+sourceID, nil) //nolint:gosec // G704: URL from operator config only if err != nil { c.log.Printf("failed to build authorize log access request: %s", err) return false @@ -111,7 +111,7 @@ func (c *CAPIClient) HasApp(sourceID, authToken string) bool { } func (c *CAPIClient) HasService(sourceID, authToken string) bool { - req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/service_instances/"+sourceID, nil) + req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/service_instances/"+sourceID, nil) //nolint:gosec // G704: URL from operator config only if err != nil { c.log.Printf("failed to build authorize log access request: %s", err) return false @@ -171,7 +171,7 @@ func (c *CAPIClient) AvailableSourceIDs(authToken string) []string { func (c *CAPIClient) sourceIDsForResourceType(resourceType, authToken string, metrics metrics.Gauge) ([]string, error) { var sourceIDs []string - req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/"+resourceType, nil) + req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/"+resourceType, nil) //nolint:gosec // G704: URL from operator config only if err != nil { c.log.Printf("failed to build authorize service instance access request: %s", err) return nil, err diff --git a/src/internal/auth/uaa_client.go b/src/internal/auth/uaa_client.go index 43dc0d9e..921dcce3 100644 --- a/src/internal/auth/uaa_client.go +++ b/src/internal/auth/uaa_client.go @@ -99,7 +99,7 @@ func (c *UAAClient) RefreshTokenKeys() error { } atomic.CompareAndSwapInt64(&c.lastQueryTime, lastQueryTime, time.Now().UnixNano()) - req, err := http.NewRequest("GET", c.uaa.String(), nil) + req, err := http.NewRequest("GET", c.uaa.String(), nil) //nolint:gosec // G704: URL from operator config only if err != nil { panic(fmt.Sprintf("failed to create request to UAA: %s", err)) } @@ -188,29 +188,64 @@ func (e UnknownTokenKeyError) Error() string { return fmt.Sprintf("using unknown token key: %s", e.Kid) } +// JWTKeyMaterialError is returned when the JWT alg header does not match the +// kind of key material UAA published for the token's kid (e.g. HS256 with an +// RSA public key). Returning a distinct type avoids spurious UAA key refresh +// and closes algorithm-confusion issues in the JOSE stack. +type JWTKeyMaterialError struct { + Alg string + Kid string +} + +func (e JWTKeyMaterialError) Error() string { + return fmt.Sprintf("token algorithm %s is incompatible with UAA key material for kid %s", e.Alg, e.Kid) +} + func (c *UAAClient) Read(token string) (Oauth2ClientContext, error) { if token == "" { return Oauth2ClientContext{}, errors.New("missing token") } payload, _, err := jose.Decode(trimBearer(token), func(headers map[string]interface{}, payload string) interface{} { - if headers["alg"] != "RS256" && headers["alg"] != "HS256" { - return AlgorithmError{Alg: headers["alg"].(string)} + alg, ok := headers["alg"].(string) + if !ok { + return AlgorithmError{Alg: fmt.Sprintf("%v", headers["alg"])} + } + if alg != "RS256" && alg != "HS256" { + return AlgorithmError{Alg: alg} } - keyId := headers["kid"].(string) + keyID, ok := headers["kid"].(string) + if !ok { + return UnknownTokenKeyError{Kid: fmt.Sprintf("%v", headers["kid"])} + } - publicKey, err := c.loadOrFetchPublicKey(keyId) + keyMaterial, err := c.loadOrFetchPublicKey(keyID) if err != nil { return err } - return publicKey + switch alg { + case "RS256": + rsaKey, ok := keyMaterial.(*rsa.PublicKey) + if !ok { + return JWTKeyMaterialError{Alg: alg, Kid: keyID} + } + return rsaKey + case "HS256": + symmetricKey, ok := keyMaterial.([]byte) + if !ok { + return JWTKeyMaterialError{Alg: alg, Kid: keyID} + } + return symmetricKey + default: + return AlgorithmError{Alg: alg} + } }) if err != nil { switch err.(type) { - case AlgorithmError, UnknownTokenKeyError: + case AlgorithmError, UnknownTokenKeyError, JWTKeyMaterialError: // no-op default: // we're specifically trying to catch "crypto/rsa: verification error", diff --git a/src/internal/auth/uaa_client_test.go b/src/internal/auth/uaa_client_test.go index a6bf0581..9f711e61 100644 --- a/src/internal/auth/uaa_client_test.go +++ b/src/internal/auth/uaa_client_test.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "encoding/base64" "errors" "fmt" "io" @@ -57,6 +58,19 @@ var _ = Describe("UAAClient", func() { Expect(err.Error()).To(Equal("failed to decode token: unsupported algorithm: none")) }) + It("rejects HS256 JWTs when UAA only published an RSA key for that kid (algorithm confusion)", func() { + kid := tc.privateKeys[0].keyId + header := fmt.Sprintf(`{"alg":"HS256","kid":%q}`, kid) + payload := `{"scope":["logs.admin"],"exp":9999999999}` + enc := base64.RawURLEncoding.EncodeToString + forged := enc([]byte(header)) + "." + enc([]byte(payload)) + "." + + _, err := tc.uaaClient.Read(withBearer(forged)) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to decode token")) + Expect(err.Error()).To(ContainSubstring("incompatible with UAA key material")) + }) + It("returns IsAdmin == true when scopes include doppler.firehose", func() { payload := tc.BuildValidPayload("doppler.firehose") token := tc.CreateSignedToken(payload) diff --git a/src/internal/cache/log_cache.go b/src/internal/cache/log_cache.go index 522327f1..48d31898 100644 --- a/src/internal/cache/log_cache.go +++ b/src/internal/cache/log_cache.go @@ -1,6 +1,7 @@ package cache import ( + "context" "log" "net" "strconv" @@ -9,7 +10,6 @@ import ( metrics "code.cloudfoundry.org/go-metric-registry" - "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" diff --git a/src/internal/gateway/gateway.go b/src/internal/gateway/gateway.go index eb04f237..4206e64a 100644 --- a/src/internal/gateway/gateway.go +++ b/src/internal/gateway/gateway.go @@ -1,6 +1,7 @@ package gateway import ( + "context" "fmt" "io" "log" @@ -10,7 +11,6 @@ import ( "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/shirou/gopsutil/v4/host" - "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/status" "google.golang.org/protobuf/encoding/protojson" diff --git a/src/internal/nozzle/nozzle.go b/src/internal/nozzle/nozzle.go index 8abc9433..e9473c72 100644 --- a/src/internal/nozzle/nozzle.go +++ b/src/internal/nozzle/nozzle.go @@ -1,6 +1,7 @@ package nozzle import ( + "context" "log" "runtime" "time" @@ -11,7 +12,6 @@ import ( diodes "code.cloudfoundry.org/go-diodes" "code.cloudfoundry.org/go-log-cache/v3/rpc/logcache_v1" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" - "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) @@ -177,12 +177,13 @@ func (n *Nozzle) envelopeWriter(ch chan []*loggregator_v2.Envelope, client logca for { envelopes := <-ch - ctx, _ := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) _, err := client.Send(ctx, &logcache_v1.SendRequest{ Envelopes: &loggregator_v2.EnvelopeBatch{ Batch: envelopes, }, }) + cancel() if err != nil { n.errCounter.Add(1) diff --git a/src/internal/nozzle/nozzle_test.go b/src/internal/nozzle/nozzle_test.go index 79aabddb..c3e18e41 100644 --- a/src/internal/nozzle/nozzle_test.go +++ b/src/internal/nozzle/nozzle_test.go @@ -1,6 +1,7 @@ package nozzle_test import ( + "context" "log" "sync" @@ -9,7 +10,6 @@ import ( "code.cloudfoundry.org/go-loggregator/v10" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" . "code.cloudfoundry.org/log-cache/internal/nozzle" - "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" diff --git a/src/internal/routing/batched_ingress_client.go b/src/internal/routing/batched_ingress_client.go index 6ee212c6..6844e670 100644 --- a/src/internal/routing/batched_ingress_client.go +++ b/src/internal/routing/batched_ingress_client.go @@ -1,6 +1,7 @@ package routing import ( + "context" "log" "time" @@ -10,7 +11,6 @@ import ( diodes "code.cloudfoundry.org/go-diodes" rpc "code.cloudfoundry.org/go-log-cache/v3/rpc/logcache_v1" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" - "golang.org/x/net/context" "google.golang.org/grpc" ) @@ -96,11 +96,12 @@ func (b *BatchedIngressClient) write(batch []interface{}) { e = append(e, i.(*loggregator_v2.Envelope)) } - ctx, _ := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) _, err := b.c.Send(ctx, &rpc.SendRequest{ LocalOnly: b.localOnly, Envelopes: &loggregator_v2.EnvelopeBatch{Batch: e}, }) + cancel() if err != nil { b.log.Printf("failed to write %d envelopes: %s", len(e), err) diff --git a/src/internal/routing/batched_ingress_client_test.go b/src/internal/routing/batched_ingress_client_test.go index f9a1d8d7..d761ec35 100644 --- a/src/internal/routing/batched_ingress_client_test.go +++ b/src/internal/routing/batched_ingress_client_test.go @@ -1,17 +1,16 @@ package routing_test import ( + "context" "io" "log" "time" "code.cloudfoundry.org/go-metric-registry/testhelpers" - + metrics "code.cloudfoundry.org/go-metric-registry" rpc "code.cloudfoundry.org/go-log-cache/v3/rpc/logcache_v1" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" - metrics "code.cloudfoundry.org/go-metric-registry" "code.cloudfoundry.org/log-cache/internal/routing" - "golang.org/x/net/context" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" diff --git a/src/internal/routing/local_store_reader.go b/src/internal/routing/local_store_reader.go index 2a00b6ac..d8862ad3 100644 --- a/src/internal/routing/local_store_reader.go +++ b/src/internal/routing/local_store_reader.go @@ -1,13 +1,13 @@ package routing import ( + "context" "fmt" "regexp" "time" "code.cloudfoundry.org/go-log-cache/v3/rpc/logcache_v1" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" - "golang.org/x/net/context" "google.golang.org/grpc" ) diff --git a/src/internal/routing/local_store_reader_test.go b/src/internal/routing/local_store_reader_test.go index 692d32b4..72cb3a42 100644 --- a/src/internal/routing/local_store_reader_test.go +++ b/src/internal/routing/local_store_reader_test.go @@ -1,13 +1,13 @@ package routing_test import ( + "context" "regexp" "time" "code.cloudfoundry.org/go-log-cache/v3/rpc/logcache_v1" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" "code.cloudfoundry.org/log-cache/internal/routing" - "golang.org/x/net/context" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" diff --git a/src/internal/routing/static_lookup.go b/src/internal/routing/static_lookup.go index 49f2fd82..e8314522 100644 --- a/src/internal/routing/static_lookup.go +++ b/src/internal/routing/static_lookup.go @@ -23,8 +23,9 @@ func NewStaticLookup(numOfRoutes int, hasher func(string) uint64) *StaticLookup // NOTE: 18446744073709551615 is 0xFFFFFFFFFFFFFFFF or the max value of a // uint64. - x := (18446744073709551615 / uint64(numOfRoutes)) - for i := uint64(0); i < uint64(numOfRoutes); i++ { + nr := uint64(numOfRoutes) //#nosec G115 + x := 18446744073709551615 / nr + for i := uint64(0); i < nr; i++ { t.Put(i*x, i) } diff --git a/src/internal/syslog/server.go b/src/internal/syslog/server.go index 5473105e..ee309bf3 100644 --- a/src/internal/syslog/server.go +++ b/src/internal/syslog/server.go @@ -1,10 +1,15 @@ package syslog import ( + "context" "crypto/tls" "errors" "fmt" "log" + "net" + "strconv" + "strings" + "sync" "time" "code.cloudfoundry.org/go-loggregator/v10" @@ -15,13 +20,6 @@ import ( "github.com/leodido/go-syslog/v4/nontransparent" "github.com/leodido/go-syslog/v4/octetcounting" "github.com/leodido/go-syslog/v4/rfc5424" - - "net" - "strconv" - "strings" - "sync" - - "golang.org/x/net/context" ) type Server struct {