diff --git a/carrier/carrier.go b/carrier/carrier.go index 5f4fc83484e..c0dc8bdeb90 100644 --- a/carrier/carrier.go +++ b/carrier/carrier.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "strings" + "time" "github.com/pkg/errors" "github.com/rs/zerolog" @@ -33,6 +34,10 @@ type StartOptions struct { TLSClientConfig *tls.Config AutoCloseInterstitial bool IsFedramp bool + // Timeout is the HTTP timeout used for Access login/token requests, such as + // fetching app metadata from the edge and verifying a cached token against + // the origin. If zero, callers fall back to token.DefaultAccessTimeout. + Timeout time.Duration } // Connection wraps up all the needed functions to forward over the tunnel @@ -140,7 +145,7 @@ func BuildAccessRequest(options *StartOptions, log *zerolog.Logger) (*http.Reque return nil, err } - token, err := token.FetchTokenWithRedirect(req.URL, options.AppInfo, options.AutoCloseInterstitial, options.IsFedramp, log) + token, err := token.FetchTokenWithRedirect(req.URL, options.AppInfo, options.AutoCloseInterstitial, options.IsFedramp, options.Timeout, log) if err != nil { return nil, err } diff --git a/carrier/websocket.go b/carrier/websocket.go index 36cd08e7133..19726218d52 100644 --- a/carrier/websocket.go +++ b/carrier/websocket.go @@ -74,7 +74,7 @@ func createWebsocketStream(options *StartOptions, log *zerolog.Logger) (*cfwebso return nil, err } - appInfo, err := token.GetAppInfo(originReq.URL) + appInfo, err := token.GetAppInfo(originReq.URL, token.ResolveAccessTimeout(options.Timeout)) if err != nil { return nil, err } diff --git a/cmd/cloudflared/access/carrier.go b/cmd/cloudflared/access/carrier.go index 52aacc5655f..967acb089ec 100644 --- a/cmd/cloudflared/access/carrier.go +++ b/cmd/cloudflared/access/carrier.go @@ -94,6 +94,7 @@ func ssh(c *cli.Context) error { Headers: headers, Host: url.Host, IsFedramp: c.Bool(fedrampFlag), + Timeout: c.Duration(accessTimeoutFlag), } if connectTo := c.String(sshConnectTo); connectTo != "" { diff --git a/cmd/cloudflared/access/cmd.go b/cmd/cloudflared/access/cmd.go index 636b9288e27..60b90e89b4a 100644 --- a/cmd/cloudflared/access/cmd.go +++ b/cmd/cloudflared/access/cmd.go @@ -9,7 +9,6 @@ import ( "os/exec" "strings" "text/template" - "time" "github.com/getsentry/sentry-go" "github.com/pkg/errors" @@ -52,6 +51,8 @@ Host {{.Hostname}} {{end}} ` fedrampFlag = "fedramp" + + accessTimeoutFlag = "access-timeout" ) const sentryDSN = "https://56a9c9fa5c364ab28f34b14f35ea0f1b@sentry.io/189878" @@ -80,10 +81,18 @@ func Commands() []*cli.Command { Aliases: []string{"forward"}, Category: "Access", Usage: "access ", - Flags: []cli.Flag{&cli.BoolFlag{ - Name: fedrampFlag, - Usage: "use when performing operations in fedramp account", - }}, + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: fedrampFlag, + Usage: "use when performing operations in fedramp account", + }, + &cli.DurationFlag{ + Name: accessTimeoutFlag, + Usage: "HTTP timeout for requests made while logging in to or verifying tokens for an Access application: fetching application metadata from the Cloudflare edge, and checking a cached token against the origin. Increase this if you see timeout errors on slow or high-latency connections.", + EnvVars: []string{"TUNNEL_ACCESS_TIMEOUT"}, + Value: token.DefaultAccessTimeout, + }, + }, Description: `Cloudflare Access protects internal resources by securing, authenticating and monitoring access per-user and by application. With Cloudflare Access, only authenticated users with the required permissions are able to reach sensitive resources. The commands provided here allow you to interact with Access protected @@ -257,7 +266,7 @@ func login(c *cli.Context) error { return err } - appInfo, err := token.GetAppInfo(appURL) + appInfo, err := token.GetAppInfo(appURL, c.Duration(accessTimeoutFlag)) if err != nil { return err } @@ -314,7 +323,7 @@ func curl(c *cli.Context) error { return err } - appInfo, err := token.GetAppInfo(appURL) + appInfo, err := token.GetAppInfo(appURL, c.Duration(accessTimeoutFlag)) if err != nil { return err } @@ -331,7 +340,7 @@ func curl(c *cli.Context) error { log.Info().Msg("You don't have an Access token set. Please run access token to fetch one.") return run("curl", cmdArgs...) } - tok, err = token.FetchToken(appURL, appInfo, c.Bool(cfdflags.AutoCloseInterstitial), c.Bool(fedrampFlag), log) + tok, err = token.FetchToken(appURL, appInfo, c.Bool(cfdflags.AutoCloseInterstitial), c.Bool(fedrampFlag), c.Duration(accessTimeoutFlag), log) if err != nil { log.Err(err).Msg("Failed to refresh token") return err @@ -391,7 +400,7 @@ func generateToken(c *cli.Context) error { return err } - appInfo, err := token.GetAppInfo(appURL) + appInfo, err := token.GetAppInfo(appURL, c.Duration(accessTimeoutFlag)) if err != nil { return err } @@ -447,11 +456,11 @@ func sshGen(c *cli.Context) error { fetchTokenURL := &url.URL{} *fetchTokenURL = *originURL - appInfo, err := token.GetAppInfo(fetchTokenURL) + appInfo, err := token.GetAppInfo(fetchTokenURL, c.Duration(accessTimeoutFlag)) if err != nil { return err } - cfdToken, err := token.FetchTokenWithRedirect(fetchTokenURL, appInfo, c.Bool(cfdflags.AutoCloseInterstitial), c.Bool(fedrampFlag), log) + cfdToken, err := token.FetchTokenWithRedirect(fetchTokenURL, appInfo, c.Bool(cfdflags.AutoCloseInterstitial), c.Bool(fedrampFlag), c.Duration(accessTimeoutFlag), log) if err != nil { return err } @@ -551,7 +560,7 @@ func verifyTokenAtEdge(appUrl *url.URL, appInfo *token.AppInfo, c *cli.Context, if c.IsSet(sshTokenSecretFlag) { headers.Add(cfAccessClientSecretHeader, c.String(sshTokenSecretFlag)) } - options := &carrier.StartOptions{AppInfo: appInfo, OriginURL: appUrl.String(), Headers: headers, AutoCloseInterstitial: c.Bool(cfdflags.AutoCloseInterstitial), IsFedramp: c.Bool(fedrampFlag)} + options := &carrier.StartOptions{AppInfo: appInfo, OriginURL: appUrl.String(), Headers: headers, AutoCloseInterstitial: c.Bool(cfdflags.AutoCloseInterstitial), IsFedramp: c.Bool(fedrampFlag), Timeout: c.Duration(accessTimeoutFlag)} if valid, err := isTokenValid(options, log); err != nil { return err @@ -584,12 +593,14 @@ func isTokenValid(options *carrier.StartOptions, log *zerolog.Logger) (bool, err query.Set("cloudflared_token_check", "true") req.URL.RawQuery = query.Encode() + timeout := token.ResolveAccessTimeout(options.Timeout) + // Do not follow redirects client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }, - Timeout: time.Second * 5, + Timeout: timeout, } resp, err := client.Do(req) if err != nil { diff --git a/cmd/cloudflared/access/cmd_test.go b/cmd/cloudflared/access/cmd_test.go new file mode 100644 index 00000000000..74cf62267d2 --- /dev/null +++ b/cmd/cloudflared/access/cmd_test.go @@ -0,0 +1,56 @@ +package access + +import ( + "flag" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v2" + + "github.com/cloudflare/cloudflared/token" +) + +// accessTimeoutDurationFlag pulls the real --access-timeout flag definition +// off of the "access" command, so precedence is tested against production +// wiring rather than a re-declared copy of it. +func accessTimeoutDurationFlag(t *testing.T) *cli.DurationFlag { + t.Helper() + cmds := Commands() + require.Len(t, cmds, 1) + for _, f := range cmds[0].Flags { + if df, ok := f.(*cli.DurationFlag); ok && df.Name == accessTimeoutFlag { + return df + } + } + t.Fatal("access-timeout flag not registered on the access command") + return nil +} + +func parseAccessTimeout(t *testing.T, args []string) time.Duration { + t.Helper() + set := flag.NewFlagSet("access", flag.ContinueOnError) + require.NoError(t, accessTimeoutDurationFlag(t).Apply(set)) + require.NoError(t, set.Parse(args)) + ctx := cli.NewContext(cli.NewApp(), set, nil) + return ctx.Duration(accessTimeoutFlag) +} + +func TestAccessTimeoutFlag_DefaultPreservesCurrentBehavior(t *testing.T) { + got := parseAccessTimeout(t, nil) + assert.Equal(t, token.DefaultAccessTimeout, got) + assert.Equal(t, 7*time.Second, got) +} + +func TestAccessTimeoutFlag_EnvVarOverridesDefault(t *testing.T) { + t.Setenv("TUNNEL_ACCESS_TIMEOUT", "30s") + got := parseAccessTimeout(t, nil) + assert.Equal(t, 30*time.Second, got) +} + +func TestAccessTimeoutFlag_FlagOverridesEnvVar(t *testing.T) { + t.Setenv("TUNNEL_ACCESS_TIMEOUT", "30s") + got := parseAccessTimeout(t, []string{"--access-timeout", "45s"}) + assert.Equal(t, 45*time.Second, got) +} diff --git a/token/token.go b/token/token.go index b182fa2c760..19ea2faac85 100644 --- a/token/token.go +++ b/token/token.go @@ -31,6 +31,22 @@ const ( metadataAllowedClockSkew = 5 * time.Minute ) +// DefaultAccessTimeout is the default HTTP timeout used for Access login/token +// requests (fetching app metadata from the edge, and verifying a cached token +// against the origin) when no timeout is explicitly configured. +const DefaultAccessTimeout = 7 * time.Second + +// ResolveAccessTimeout returns timeout if it is positive, and DefaultAccessTimeout +// otherwise. It is used to clamp user-configured Access timeouts (e.g. from +// --access-timeout) since a zero or negative time.Duration disables the +// timeout entirely on an http.Client. +func ResolveAccessTimeout(timeout time.Duration) time.Duration { + if timeout <= 0 { + return DefaultAccessTimeout + } + return timeout +} + var userAgent = "DEV" type AppInfo struct { @@ -269,18 +285,18 @@ func Init(version string) { // FetchTokenWithRedirect will either load a stored token or generate a new one // it appends the full url as the redirect URL to the access cli request if opening the browser -func FetchTokenWithRedirect(appURL *url.URL, appInfo *AppInfo, autoClose bool, isFedramp bool, log *zerolog.Logger) (string, error) { - return getToken(appURL, appInfo, false, autoClose, isFedramp, log) +func FetchTokenWithRedirect(appURL *url.URL, appInfo *AppInfo, autoClose bool, isFedramp bool, timeout time.Duration, log *zerolog.Logger) (string, error) { + return getToken(appURL, appInfo, false, autoClose, isFedramp, timeout, log) } // FetchToken will either load a stored token or generate a new one // it appends the host of the appURL as the redirect URL to the access cli request if opening the browser -func FetchToken(appURL *url.URL, appInfo *AppInfo, autoClose bool, isFedramp bool, log *zerolog.Logger) (string, error) { - return getToken(appURL, appInfo, true, autoClose, isFedramp, log) +func FetchToken(appURL *url.URL, appInfo *AppInfo, autoClose bool, isFedramp bool, timeout time.Duration, log *zerolog.Logger) (string, error) { + return getToken(appURL, appInfo, true, autoClose, isFedramp, timeout, log) } // getToken will either load a stored token or generate a new one -func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, autoClose bool, isFedramp bool, log *zerolog.Logger) (string, error) { +func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, autoClose bool, isFedramp bool, timeout time.Duration, log *zerolog.Logger) (string, error) { if token, err := GetAppTokenIfExists(appInfo); token != "" && err == nil { return token, nil } @@ -315,7 +331,7 @@ func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, autoClose boo orgToken, err = GetOrgTokenIfExists(appInfo.AuthDomain) } if err == nil { - if appToken, err := exchangeOrgToken(appURL, orgToken); err != nil { + if appToken, err := exchangeOrgToken(appURL, orgToken, timeout); err != nil { log.Debug().Msgf("failed to exchange org token for app token: %s", err) } else { // generate app path @@ -363,9 +379,9 @@ func getTokensFromEdge(appURL *url.URL, appAUD, appTokenPath, orgTokenPath strin // a signed metadata JWT from the Cloudflare edge. The JWT signature is verified // against the account's public keys (fetched from the auth domain's JWKS // endpoint) to prevent an attacker-controlled server from spoofing app identity. -func GetAppInfo(reqURL *url.URL) (*AppInfo, error) { +func GetAppInfo(reqURL *url.URL, timeout time.Duration) (*AppInfo, error) { // Fetch the metadata JWT from the edge (no redirects followed). - rawJWT, err := fetchMetadataJWT(reqURL.String()) + rawJWT, err := fetchMetadataJWT(reqURL.String(), timeout) if err != nil { return nil, err } @@ -419,12 +435,12 @@ func GetAppInfo(reqURL *url.URL) (*AppInfo, error) { // fetchMetadataJWT sends a HEAD request to reqURL with the metadata request // header and returns the raw JWT string from the response. No redirects are // followed. -func fetchMetadataJWT(reqURL string) (string, error) { +func fetchMetadataJWT(reqURL string, timeout time.Duration) (string, error) { client := &http.Client{ CheckRedirect: func(_ *http.Request, _ []*http.Request) error { return http.ErrUseLastResponse }, - Timeout: time.Second * 7, + Timeout: ResolveAccessTimeout(timeout), } req, err := http.NewRequest("HEAD", reqURL, nil) @@ -491,12 +507,12 @@ func handleRedirects(req *http.Request, via []*http.Request, orgToken string) er // exchangeOrgToken attaches an org token to a request to the appURL and returns an app token. This uses the Access SSO // flow to automatically generate and return an app token without the login page. -func exchangeOrgToken(appURL *url.URL, orgToken string) (string, error) { +func exchangeOrgToken(appURL *url.URL, orgToken string, timeout time.Duration) (string, error) { client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error { return handleRedirects(req, via, orgToken) }, - Timeout: time.Second * 7, + Timeout: ResolveAccessTimeout(timeout), } appTokenRequest, err := http.NewRequest("HEAD", appURL.String(), nil) diff --git a/token/token_test.go b/token/token_test.go index 5e81c54ddf3..29c9a652e9e 100644 --- a/token/token_test.go +++ b/token/token_test.go @@ -256,17 +256,72 @@ func TestParseAuthDomain(t *testing.T) { func TestFetchMetadataJWT_ReturnsAppInfoErrorOnError(t *testing.T) { t.Parallel() - _, err := fetchMetadataJWT("://invalid") + _, err := fetchMetadataJWT("://invalid", DefaultAccessTimeout) require.ErrorContains(t, err, "failed to create app info request") server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) serverURL := server.URL server.Close() - _, err = fetchMetadataJWT(serverURL) + _, err = fetchMetadataJWT(serverURL, DefaultAccessTimeout) require.ErrorContains(t, err, "failed to get app info") } +func TestFetchMetadataJWT_HonorsConfiguredTimeout(t *testing.T) { + t.Parallel() + + // Handler sleeps well past the configured timeout below, so the request + // should fail at roughly the configured timeout rather than hanging or + // falling back to the old hardcoded 7s. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + time.Sleep(500 * time.Millisecond) + w.Header().Set(accessMetadataRespHeader, "irrelevant") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + configuredTimeout := 100 * time.Millisecond + start := time.Now() + _, err := fetchMetadataJWT(server.URL, configuredTimeout) + elapsed := time.Since(start) + + require.ErrorContains(t, err, "failed to get app info") + assert.Less(t, elapsed, 400*time.Millisecond, "expected the configured timeout to fire well before the handler's 500ms sleep") +} + +func TestFetchMetadataJWT_DefaultTimeoutPreservesCurrentBehavior(t *testing.T) { + t.Parallel() + + assert.Equal(t, 7*time.Second, DefaultAccessTimeout, "default must match today's larger hardcoded value so nobody's timeout gets shorter by default") + + rawJWT := "test-jwt" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set(accessMetadataRespHeader, rawJWT) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + got, err := fetchMetadataJWT(server.URL, DefaultAccessTimeout) + require.NoError(t, err) + assert.Equal(t, rawJWT, got) +} + +func TestFetchMetadataJWT_ZeroTimeoutFallsBackToDefault(t *testing.T) { + t.Parallel() + + // A 0 timeout must not disable the client's timeout (net/http treats 0 as + // "no timeout"); it should fall back to DefaultAccessTimeout instead. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + time.Sleep(500 * time.Millisecond) + w.Header().Set(accessMetadataRespHeader, "irrelevant") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + _, err := fetchMetadataJWT(server.URL, 0) + require.NoError(t, err) +} + func TestValidateMetadataIssuedAt(t *testing.T) { t.Parallel() @@ -373,7 +428,7 @@ func TestGetAppInfo_RejectsNoMetadataHeader(t *testing.T) { reqURL, err := url.Parse(server.URL) require.NoError(t, err) - appInfo, err := GetAppInfo(reqURL) + appInfo, err := GetAppInfo(reqURL, DefaultAccessTimeout) assert.Nil(t, appInfo) require.Error(t, err) assert.Contains(t, err.Error(), "failed to find Access application") @@ -402,7 +457,7 @@ func TestGetAppInfo_RejectsNonCloudflareAuthDomain(t *testing.T) { reqURL, err := url.Parse(server.URL) require.NoError(t, err) - appInfo, err := GetAppInfo(reqURL) + appInfo, err := GetAppInfo(reqURL, DefaultAccessTimeout) assert.Nil(t, appInfo) require.Error(t, err) assert.Contains(t, err.Error(), "auth_domain validation failed") @@ -436,7 +491,7 @@ func TestGetAppInfo_RejectsHostnameMismatch(t *testing.T) { reqURL, err := url.Parse(metadataServer.URL) require.NoError(t, err) - appInfo, err := GetAppInfo(reqURL) + appInfo, err := GetAppInfo(reqURL, DefaultAccessTimeout) assert.Nil(t, appInfo) require.Error(t, err) assert.Contains(t, err.Error(), "does not match request host") @@ -475,7 +530,7 @@ func TestGetAppInfo_AcceptsValidMetadata(t *testing.T) { } rawJWT = signTestMetadataJWT(t, claims, key, kid) - appInfo, err := GetAppInfo(reqURL) + appInfo, err := GetAppInfo(reqURL, DefaultAccessTimeout) require.NoError(t, err) assert.Equal(t, authDomain, appInfo.AuthDomain) assert.Equal(t, "test-aud", appInfo.AppAUD) @@ -483,7 +538,7 @@ func TestGetAppInfo_AcceptsValidMetadata(t *testing.T) { claims.AppHostname = "" rawJWT = signTestMetadataJWT(t, claims, key, kid) - appInfo, err = GetAppInfo(reqURL) + appInfo, err = GetAppInfo(reqURL, DefaultAccessTimeout) require.NoError(t, err) assert.Equal(t, claims.Hostname, appInfo.AppHostname) tokenPath, err := GenerateAppTokenFilePathFromURL(appInfo.AppHostname, appInfo.AppAUD, keyName) @@ -547,7 +602,7 @@ func TestGetAppInfo_RejectsInvalidClaims(t *testing.T) { tc.mutate(claims) rawJWT = signTestMetadataJWT(t, claims, key, kid) - appInfo, err := GetAppInfo(reqURL) + appInfo, err := GetAppInfo(reqURL, DefaultAccessTimeout) assert.Nil(t, appInfo) assert.Error(t, err) })