From 8a7c2d760976183f550fd609f280bb7561c395c8 Mon Sep 17 00:00:00 2001 From: Sourav Sarangi Date: Tue, 7 May 2024 02:24:57 +0530 Subject: [PATCH] ccl,sql,util: Fix jwt auth and add sensitive error logs We are running into issues with jwt authentication and currently unable to provide support as we are not logging the error from the http client used in the authenticator. The PR looks to propagate this obtained error from `ValidateJWTLogin` http client. We are also introducing `JWTClientTimeout` and `JWTClientCustomCA` cluster settings such that these could be configured directly for the http client used in authenticator. The http client now also respects the system http proxy if set and has a cluster setting `JWTClientSystemProxyEnabled` to enable it. Validated the error details when presenting an expired token ``` ERROR: JWT authentication: invalid token SQLSTATE: 28000 DETAIL: unable to parse token: exp not satisfied Failed running "sql" ``` Validated error on setting wrong proxy params ``` ERROR: JWT authentication: unable to validate token SQLSTATE: 28000 DETAIL: unable to fetch jwks: Get "https://accounts.google.com/.well-known/openid-configuration": proxyconnect tcp: dial tcp [::1]:3129: connect: connection refused Failed running "sql" ``` Verified access logs after setting up squid proxy and passing env HTTP_PROXY and HTTPS_PROXY params ``` 1715103871.761 144 ::1 TCP_TUNNEL/200 5708 CONNECT accounts.google.com:443 - HIER_DIRECT/74.125.200.84 - 1715103871.836 73 ::1 TCP_TUNNEL/200 5964 CONNECT www.googleapis.com:443 - HIER_DIRECT/142.250.182.10 - ``` fixes https://github.com/cockroachdb/cockroach/issues/123575, Epic CRDB-38386, Release note(security update): We are adding 3 cluster settings `server.jwt_authentication.client.timeout`, `server.jwt_authentication.client.custom_ca` and `server.jwt_authentication.client.system_proxy.enabled` which can control the jwt auth behaviour for http client calls. --- pkg/ccl/jwtauthccl/authentication_jwt.go | 36 +++++++++--- pkg/ccl/jwtauthccl/authentication_jwt_test.go | 55 ++++++++++++++++++- pkg/ccl/jwtauthccl/settings.go | 25 +++++++++ pkg/sql/pgwire/auth.go | 4 +- pkg/sql/pgwire/auth_methods.go | 5 ++ pkg/util/httputil/client.go | 30 ++++++++++ 6 files changed, 145 insertions(+), 10 deletions(-) diff --git a/pkg/ccl/jwtauthccl/authentication_jwt.go b/pkg/ccl/jwtauthccl/authentication_jwt.go index 8a106b46742e..a4b6a8227df2 100644 --- a/pkg/ccl/jwtauthccl/authentication_jwt.go +++ b/pkg/ccl/jwtauthccl/authentication_jwt.go @@ -71,6 +71,8 @@ type jwtAuthenticatorConf struct { jwks jwk.Set claim string jwksAutoFetchEnabled bool + //clientTimeout time.Duration + //customCA string } // reloadConfig locks mutex and then refreshes the values in conf from the cluster settings. @@ -91,6 +93,8 @@ func (authenticator *jwtAuthenticator) reloadConfigLocked( jwks: mustParseJWKS(JWTAuthJWKS.Get(&st.SV)), claim: JWTAuthClaim.Get(&st.SV), jwksAutoFetchEnabled: JWKSAutoFetchEnabled.Get(&st.SV), + // clientTimeout: JWTClientTimeout.Get(&st.SV), + // customCA: JWTClientCustomCA.Get(&st.SV), } if !authenticator.mu.conf.enabled && conf.enabled { @@ -146,7 +150,8 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin( // The token will be parsed again later to actually verify the signature. unverifiedToken, err := jwt.Parse(tokenBytes) if err != nil { - return errors.Newf("JWT authentication: invalid token") + return errors.WithDetailf( + errors.Newf("JWT authentication: invalid token"), "token parsing failed: %v", err) } // Check for issuer match against configured issuers. @@ -168,9 +173,11 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin( var jwkSet jwk.Set // If auto-fetch is enabled, fetch the JWKS remotely from the issuer's well known jwks url. if authenticator.mu.conf.jwksAutoFetchEnabled { - jwkSet, err = remoteFetchJWKS(ctx, issuerUrl) + jwkSet, err = authenticator.remoteFetchJWKS(ctx, issuerUrl) if err != nil { - return errors.Newf("JWT authentication: unable to validate token") + return errors.WithDetailf( + errors.Newf("JWT authentication: unable to validate token"), + "unable to fetch jwks: %v", err) } } else { jwkSet = authenticator.mu.conf.jwks @@ -179,7 +186,7 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin( // Now that both the issuer and key-id are matched, parse the token again to validate the signature. parsedToken, err := jwt.Parse(tokenBytes, jwt.WithKeySet(jwkSet), jwt.WithValidate(true), jwt.InferAlgorithmFromKey(true)) if err != nil { - return errors.Newf("JWT authentication: invalid token") + return errors.WithDetailf(errors.Newf("JWT authentication: invalid token"), "unable to parse token: %v", err) } // Extract all requested principals from the token. By default, we take it from the subject unless they specify @@ -269,8 +276,10 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin( } // remoteFetchJWKS fetches the JWKS from the provided URI. -func remoteFetchJWKS(ctx context.Context, issuerUrl string) (jwk.Set, error) { - jwksUrl, err := getJWKSUrl(ctx, issuerUrl) +func (authenticator *jwtAuthenticator) remoteFetchJWKS( + ctx context.Context, issuerUrl string, +) (jwk.Set, error) { + jwksUrl, err := authenticator.getJWKSUrl(ctx, issuerUrl) if err != nil { return nil, err } @@ -286,7 +295,9 @@ func remoteFetchJWKS(ctx context.Context, issuerUrl string) (jwk.Set, error) { } // getJWKSUrl returns the JWKS URI from the OpenID configuration endpoint. -func getJWKSUrl(ctx context.Context, issuerUrl string) (string, error) { +func (authenticator *jwtAuthenticator) getJWKSUrl( + ctx context.Context, issuerUrl string, +) (string, error) { type OIDCConfigResponse struct { JWKSUri string `json:"jwks_uri"` } @@ -312,7 +323,10 @@ func getOpenIdConfigEndpoint(issuerUrl string) string { } var getHttpResponse = func(ctx context.Context, url string) ([]byte, error) { - resp, err := httputil.Get(ctx, url) + // responseTimeout := authenticator.mu.conf.clientTimeout + httpClient := httputil.NewClientWithTimeouts( + httputil.StandardHTTPTimeout, httputil.StandardHTTPTimeout) + resp, err := httpClient.Get(context.Background(), url) if err != nil { return nil, err } @@ -354,6 +368,12 @@ var ConfigureJWTAuth = func( JWKSAutoFetchEnabled.SetOnChange(&st.SV, func(ctx context.Context) { authenticator.reloadConfig(ambientCtx.AnnotateCtx(ctx), st) }) + //JWTClientTimeout.SetOnChange(&st.SV, func(ctx context.Context) { + // authenticator.reloadConfig(ambientCtx.AnnotateCtx(ctx), st) + //}) + //JWTClientCustomCA.SetOnChange(&st.SV, func(ctx context.Context) { + // authenticator.reloadConfig(ambientCtx.AnnotateCtx(ctx), st) + //}) return &authenticator } diff --git a/pkg/ccl/jwtauthccl/authentication_jwt_test.go b/pkg/ccl/jwtauthccl/authentication_jwt_test.go index ce0d97713003..04838c965545 100644 --- a/pkg/ccl/jwtauthccl/authentication_jwt_test.go +++ b/pkg/ccl/jwtauthccl/authentication_jwt_test.go @@ -15,6 +15,10 @@ import ( "crypto/rand" "crypto/rsa" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" "os" "strings" "testing" @@ -146,6 +150,7 @@ func TestJWTEnabledCheck(t *testing.T) { // Now the validate call gets past the enabled check and fails on the next check (issuer check). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid issuer") + require.EqualValues(t, "token issued by issuer1", errors.GetAllDetails(err)[0]) } func TestJWTSingleKey(t *testing.T) { @@ -174,6 +179,7 @@ func TestJWTSingleKey(t *testing.T) { JWKSAutoFetchEnabled.Override(ctx, &s.ClusterSettings().SV, true) err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap) require.ErrorContains(t, err, "JWT authentication: unable to validate token") + require.EqualValues(t, "unable to fetch jwks: Get \"issuer1/.well-known/openid-configuration\": unsupported protocol scheme \"\"", errors.GetAllDetails(err)[0]) // Set the JWKS cluster setting. JWKSAutoFetchEnabled.Override(ctx, &s.ClusterSettings().SV, false) @@ -182,6 +188,7 @@ func TestJWTSingleKey(t *testing.T) { // Now the validate call gets past the token validity check and fails on the next check (subject matching user). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) } func TestJWTSingleKeyWithoutKeyAlgorithm(t *testing.T) { @@ -220,6 +227,7 @@ func TestJWTSingleKeyWithoutKeyAlgorithm(t *testing.T) { // Now the validate call gets past the token validity check and fails on the next check (subject matching user). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) } func TestJWTMultiKey(t *testing.T) { @@ -259,6 +267,7 @@ func TestJWTMultiKey(t *testing.T) { // Now jwk2 token passes the validity check and fails on the next check (subject matching user). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) } func TestExpiredToken(t *testing.T) { @@ -283,6 +292,7 @@ func TestExpiredToken(t *testing.T) { // Validation fails with an invalid token error for tokens with an expiration date in the past. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid token") + require.EqualValues(t, "unable to parse token: exp not satisfied", errors.GetAllDetails(err)[0]) } func TestKeyIdMismatch(t *testing.T) { @@ -343,15 +353,18 @@ func TestIssuerCheck(t *testing.T) { // Validation fails with no issuer are configured. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token1, identMap) require.ErrorContains(t, err, "JWT authentication: invalid issuer") + require.EqualValues(t, "token issued by issuer1", errors.GetAllDetails(err)[0]) JWTAuthIssuers.Override(ctx, &s.ClusterSettings().SV, issuer2) // Validation fails with an issuer error when the issuer in the token is not in cluster's accepted issuers. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token1, identMap) require.ErrorContains(t, err, "JWT authentication: invalid issuer") + require.EqualValues(t, "token issued by issuer1", errors.GetAllDetails(err)[0]) // Validation succeeds when the issuer in the token is equal to the cluster's accepted issuers. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token2, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) // Set the cluster setting to accept issuer values of either "issuer" or "issuer2". JWTAuthIssuers.Override(ctx, &s.ClusterSettings().SV, "[\""+issuer1+"\", \""+issuer2+"\"]") @@ -359,11 +372,12 @@ func TestIssuerCheck(t *testing.T) { // Validation succeeds when the issuer in the token is an element of the cluster's accepted issuers. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token1, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) // Validation succeeds when the issuer in the token is an element of the cluster's accepted issuers. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token2, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") - + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) } func TestSubjectCheck(t *testing.T) { @@ -389,11 +403,13 @@ func TestSubjectCheck(t *testing.T) { // "invalid" but the token is for the user "test2". err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for invalid_user", errors.GetAllDetails(err)[0]) // Validation passes the subject check when the username matches the subject and then fails on the next // check (audience field not matching). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) } func TestClaimMissing(t *testing.T) { @@ -419,6 +435,7 @@ func TestClaimMissing(t *testing.T) { // Validation fails with missing claim err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), missingClaimToken, identMap) require.ErrorContains(t, err, "JWT authentication: missing claim") + require.EqualValues(t, "token does not contain a claim for groups", errors.GetAllDetails(err)[0]) } func TestIntegerClaimValue(t *testing.T) { @@ -445,6 +462,7 @@ func TestIntegerClaimValue(t *testing.T) { // the integer claim is implicitly cast to a string err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), intClaimToken, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) } func TestSingleClaim(t *testing.T) { @@ -476,6 +494,7 @@ func TestSingleClaim(t *testing.T) { // check (audience field not matching). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) } func TestMultipleClaim(t *testing.T) { @@ -507,8 +526,10 @@ func TestMultipleClaim(t *testing.T) { // check (audience field not matching). err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username2), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) } func TestSubjectMappingCheck(t *testing.T) { @@ -536,14 +557,17 @@ func TestSubjectMappingCheck(t *testing.T) { // but they try to log in with username1. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "token issued for [test_user1] and login was for test_user1", errors.GetAllDetails(err)[0]) // Validation fails if there is a map for the issuer but no mapping rule matches. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token2, identMap) require.ErrorContains(t, err, "JWT authentication: invalid principal") + require.EqualValues(t, "the value [test_user2] for the issuer issuer2 is invalid", errors.GetAllDetails(err)[0]) // Validation passes the subject check when the username matches the mapped subject. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username2), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) } func TestSubjectReservedUser(t *testing.T) { @@ -570,10 +594,12 @@ func TestSubjectReservedUser(t *testing.T) { // You cannot log in as root or other reserved users using token based auth when mapped to root. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString("root"), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid identity") + require.EqualValues(t, "cannot use JWT auth to login to a reserved user root", errors.GetAllDetails(err)[0]) // You cannot log in as root or other reserved users using token based auth when no map is involved. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString("root"), token2, identMap) require.ErrorContains(t, err, "JWT authentication: invalid identity") + require.EqualValues(t, "cannot use JWT auth to login to a reserved user root", errors.GetAllDetails(err)[0]) } func TestAudienceCheck(t *testing.T) { @@ -601,6 +627,7 @@ func TestAudienceCheck(t *testing.T) { // Validation fails with an audience error when the audience in the token doesn't match the cluster's audience. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) // Update the audience field to "test_cluster". JWTAuthAudience.Override(ctx, &s.ClusterSettings().SV, audience1) @@ -683,6 +710,7 @@ func Test_JWKSFetchWorksWhenEnabled(t *testing.T) { // Validation fails with an audience error when the audience in the token doesn't match the cluster's audience. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) // Update the audience field to "test_cluster". JWTAuthAudience.Override(ctx, &s.ClusterSettings().SV, audience1) @@ -738,6 +766,7 @@ func Test_JWKSFetchWorksWhenEnabledIgnoresTheStaticJWKS(t *testing.T) { // Validation fails with an audience error when the audience in the token doesn't match the cluster's audience. err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.ErrorContains(t, err, "JWT authentication: invalid audience") + require.EqualValues(t, "token issued with an audience of [test_cluster]", errors.GetAllDetails(err)[0]) // Update the audience field to "test_cluster". JWTAuthAudience.Override(ctx, &s.ClusterSettings().SV, audience1) @@ -752,3 +781,27 @@ func Test_JWKSFetchWorksWhenEnabledIgnoresTheStaticJWKS(t *testing.T) { err = verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(username1), token, identMap) require.NoError(t, err) } + +func TestJWTAuthCanUseHTTPProxy(t *testing.T) { + defer leaktest.AfterTest(t)() + ctx := context.Background() + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(fmt.Sprintf("proxied-%s", r.URL))) + })) + defer proxy.Close() + + // Normally, we would set proxy via HTTP_PROXY environment variable. + // However, if we run multiple tests in this package, and earlier tests + // happen to create an http client, then the DefaultTransport will have + // been been initialized with an empty Proxy. So, set proxy directly. + http.DefaultTransport.(*http.Transport).Proxy = func(_ *http.Request) (*url.URL, error) { + return url.Parse(proxy.URL) + } + defer func() { + http.DefaultTransport.(*http.Transport).Proxy = nil + }() + + res, err := getHttpResponse(ctx, "http://my-server/.well-known/openid-configuration") + require.NoError(t, err) + require.EqualValues(t, "proxied-http://my-server/.well-known/openid-configuration", string(res)) +} diff --git a/pkg/ccl/jwtauthccl/settings.go b/pkg/ccl/jwtauthccl/settings.go index 995415fcf663..7155349a6be0 100644 --- a/pkg/ccl/jwtauthccl/settings.go +++ b/pkg/ccl/jwtauthccl/settings.go @@ -26,6 +26,9 @@ const ( JWTAuthJWKSSettingName = baseJWTAuthSettingName + "jwks" JWTAuthClaimSettingName = baseJWTAuthSettingName + "claim" JWKSAutoFetchEnabledSettingName = baseJWTAuthSettingName + "jwks_auto_fetch.enabled" + //JWTAuthClientTimeoutSettingName = baseJWTAuthSettingName + "client.timeout" + //JWTAuthClientCustomCASettingName = baseJWTAuthSettingName + "client.custom_ca" + //JWTAuthClientSystemProxyEnabledSettingName = baseJWTAuthSettingName + "client.system_proxy.enabled" ) // JWTAuthClaim sets the JWT claim that is parsed to get the username. @@ -84,6 +87,28 @@ var JWKSAutoFetchEnabled = settings.RegisterBoolSetting( settings.WithReportable(true), ) +//// JWTClientTimeout is a cluster setting used for setting jwt http client interactions. +//var JWTClientTimeout = settings.RegisterDurationSetting( +// settings.ApplicationLevel, +// JWTAuthClientTimeoutSettingName, +// "sets the timeout for jwt http client operations", +// httputil.StandardHTTPTimeout, +// settings.WithPublic) +// +//var JWTClientCustomCA = settings.RegisterStringSetting( +// settings.ApplicationLevel, +// JWTAuthClientCustomCASettingName, +// "sets the custom root CA (appended to system's default CAs) for verifying "+ +// "certificates when interacting with jwt internal HTTPS client", +// "", +// settings.WithPublic) +//var JWTClientSystemProxyEnabled = settings.RegisterBoolSetting( +// settings.ApplicationLevel, +// JWTAuthClientSystemProxyEnabledSettingName, +// "enables jwt http client to use the system proxy set", +// true, +// settings.WithPublic) + func validateJWTAuthIssuers(values *settings.Values, s string) error { var issuers []string diff --git a/pkg/sql/pgwire/auth.go b/pkg/sql/pgwire/auth.go index 224cede13ce2..1f22ef36aea7 100644 --- a/pkg/sql/pgwire/auth.go +++ b/pkg/sql/pgwire/auth.go @@ -552,8 +552,10 @@ func (p *authPipe) LogAuthFailed( // reason is likely to be more specific than at a higher point in the stack. p.loggedFailure = true var errStr redact.RedactableString + // should we actually perform the redact here? if detailedErr != nil { - errStr = redact.Sprint(detailedErr) + // should we apply redact here? + errStr = redact.Sprint(detailedErr).Redact() } ev := &eventpb.ClientAuthenticationFailed{ CommonConnectionDetails: p.connDetails, diff --git a/pkg/sql/pgwire/auth_methods.go b/pkg/sql/pgwire/auth_methods.go index b2a298b8ea88..f9cde0d225f3 100644 --- a/pkg/sql/pgwire/auth_methods.go +++ b/pkg/sql/pgwire/auth_methods.go @@ -781,7 +781,12 @@ func authJwtToken( return security.NewErrPasswordUserAuthFailed(user) } if err = jwtVerifier.ValidateJWTLogin(ctx, execCfg.Settings, user, []byte(token), identMap); err != nil { + // We are logging unsafe part of error also. Should we change this + // behaviour? Also, should we look into obtaining more specific auth + // failure reason here? c.LogAuthFailed(ctx, eventpb.AuthFailReason_CREDENTIALS_INVALID, err) + // we are returning unsafe error to client in description but only safe + // log parts are sent to sentry. is this correct way to handle this? return err } c.LogAuthOK(ctx) diff --git a/pkg/util/httputil/client.go b/pkg/util/httputil/client.go index bbeebc3ef200..f1fa79cbc3b2 100644 --- a/pkg/util/httputil/client.go +++ b/pkg/util/httputil/client.go @@ -12,6 +12,8 @@ package httputil import ( "context" + "crypto/tls" + "crypto/x509" "io" "net" "net/http" @@ -32,6 +34,25 @@ func NewClientWithTimeout(timeout time.Duration) *Client { // NewClientWithTimeouts defines a http.Client with the given dialer and client timeouts. func NewClientWithTimeouts(dialerTimeout, clientTimeout time.Duration) *Client { + return NewClientWithTimeoutsCustomCA(dialerTimeout, clientTimeout, "") +} + +// NewClientWithTimeoutsCustomCA defines a http.Client with the given dialer and client timeouts and custom CA pem. +func NewClientWithTimeoutsCustomCA( + dialerTimeout, clientTimeout time.Duration, customCAPem string, +) *Client { + var tlsConf *tls.Config + if customCAPem != "" { + roots, err := x509.SystemCertPool() + if err != nil { + return nil + } + if !roots.AppendCertsFromPEM([]byte(customCAPem)) { + return nil + } + tlsConf = &tls.Config{RootCAs: roots} + } + t := http.DefaultTransport.(*http.Transport) return &Client{&http.Client{ Timeout: clientTimeout, Transport: &http.Transport{ @@ -39,6 +60,15 @@ func NewClientWithTimeouts(dialerTimeout, clientTimeout time.Duration) *Client { // much higher than on linux). DialContext: (&net.Dialer{Timeout: dialerTimeout}).DialContext, DisableKeepAlives: true, + + Proxy: t.Proxy, + MaxIdleConns: t.MaxIdleConns, + IdleConnTimeout: t.IdleConnTimeout, + TLSHandshakeTimeout: t.TLSHandshakeTimeout, + ExpectContinueTimeout: t.ExpectContinueTimeout, + + // Add our custom CA. + TLSClientConfig: tlsConf, }, }} }