From 8e340d95989c3746a1034e464028efe457aadd3e Mon Sep 17 00:00:00 2001 From: Michael Borkenstein Date: Tue, 2 Mar 2021 14:35:40 -0600 Subject: [PATCH] AUTH-3394: Creates a token per app instead of per path --- carrier/carrier.go | 5 +- carrier/websocket.go | 6 +- cmd/cloudflared/access/carrier.go | 12 +++ cmd/cloudflared/access/cmd.go | 46 +++++++---- sshgen/sshgen.go | 5 +- sshgen/sshgen_test.go | 7 +- token/path.go | 6 +- token/token.go | 125 +++++++++++++++++------------- 8 files changed, 129 insertions(+), 83 deletions(-) diff --git a/carrier/carrier.go b/carrier/carrier.go index 67f50a1e..f44f5d28 100644 --- a/carrier/carrier.go +++ b/carrier/carrier.go @@ -21,6 +21,7 @@ import ( const LogFieldOriginURL = "originURL" type StartOptions struct { + AppInfo *token.AppInfo OriginURL string Headers http.Header Host string @@ -123,7 +124,7 @@ func IsAccessResponse(resp *http.Response) bool { if err != nil || location == nil { return false } - if strings.HasPrefix(location.Path, "/cdn-cgi/access/login") { + if strings.HasPrefix(location.Path, token.AccessLoginWorkerPath) { return true } @@ -137,7 +138,7 @@ func BuildAccessRequest(options *StartOptions, log *zerolog.Logger) (*http.Reque return nil, err } - token, err := token.FetchTokenWithRedirect(req.URL, log) + token, err := token.FetchTokenWithRedirect(req.URL, options.AppInfo, log) if err != nil { return nil, err } diff --git a/carrier/websocket.go b/carrier/websocket.go index 89a8dc0c..7b7252b9 100644 --- a/carrier/websocket.go +++ b/carrier/websocket.go @@ -116,11 +116,7 @@ func createAccessAuthenticatedStream(options *StartOptions, log *zerolog.Logger) } // Access Token is invalid for some reason. Go through regen flow - originReq, err := http.NewRequest(http.MethodGet, options.OriginURL, nil) - if err != nil { - return nil, err - } - if err := token.RemoveTokenIfExists(originReq.URL); err != nil { + if err := token.RemoveTokenIfExists(options.AppInfo); err != nil { return nil, err } wsConn, resp, err = createAccessWebSocketStream(options, log) diff --git a/cmd/cloudflared/access/carrier.go b/cmd/cloudflared/access/carrier.go index fc169818..db049894 100644 --- a/cmd/cloudflared/access/carrier.go +++ b/cmd/cloudflared/access/carrier.go @@ -10,6 +10,7 @@ import ( "github.com/cloudflare/cloudflared/config" "github.com/cloudflare/cloudflared/h2mux" "github.com/cloudflare/cloudflared/logger" + "github.com/cloudflare/cloudflared/token" "github.com/cloudflare/cloudflared/validation" "github.com/pkg/errors" @@ -108,6 +109,17 @@ func ssh(c *cli.Context) error { } } + originReq, err := http.NewRequest(http.MethodGet, options.OriginURL, nil) + if err != nil { + return err + } + + appInfo, err := token.GetAppInfo(originReq.URL) + if err != nil { + return err + } + options.AppInfo = appInfo + // we could add a cmd line variable for this bool if we want the SOCK5 server to be on the client side wsConn := carrier.NewWSConnection(log) diff --git a/cmd/cloudflared/access/cmd.go b/cmd/cloudflared/access/cmd.go index 960f1a6d..f439a958 100644 --- a/cmd/cloudflared/access/cmd.go +++ b/cmd/cloudflared/access/cmd.go @@ -167,9 +167,9 @@ func Commands() []*cli.Command { Usage: "Application logging level {fatal, error, info, debug}. ", }, &cli.StringFlag{ - Name: sshConnectTo, + Name: sshConnectTo, Hidden: true, - Usage: "Connect to alternate location for testing, value is host, host:port, or sni:port:host", + Usage: "Connect to alternate location for testing, value is host, host:port, or sni:port:host", }, }, }, @@ -221,12 +221,18 @@ func login(c *cli.Context) error { log.Error().Msg("Please provide the url of the Access application") return err } - if err := verifyTokenAtEdge(appURL, c, log); err != nil { + + appInfo, err := token.GetAppInfo(appURL) + if err != nil { + return err + } + + if err := verifyTokenAtEdge(appURL, appInfo, c, log); err != nil { log.Err(err).Msg("Could not verify token") return err } - cfdToken, err := token.GetAppTokenIfExists(appURL) + cfdToken, err := token.GetAppTokenIfExists(appInfo) if err != nil { fmt.Fprintln(os.Stderr, "Unable to find token for provided application.") return err @@ -268,13 +274,17 @@ func curl(c *cli.Context) error { return err } - tok, err := token.GetAppTokenIfExists(appURL) + appInfo, err := token.GetAppInfo(appURL) + if err != nil { + return err + } + tok, err := token.GetAppTokenIfExists(appInfo) if err != nil || tok == "" { if allowRequest { 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, log) + tok, err = token.FetchToken(appURL, appInfo, log) if err != nil { log.Err(err).Msg("Failed to refresh token") return err @@ -318,7 +328,12 @@ func generateToken(c *cli.Context) error { fmt.Fprintln(os.Stderr, "Please provide a url.") return err } - tok, err := token.GetAppTokenIfExists(appURL) + + appInfo, err := token.GetAppInfo(appURL) + if err != nil { + return err + } + tok, err := token.GetAppTokenIfExists(appInfo) if err != nil || tok == "" { fmt.Fprintln(os.Stderr, "Unable to find token for provided application. Please run login command to generate token.") return err @@ -369,19 +384,24 @@ func sshGen(c *cli.Context) error { // this fetchToken function mutates the appURL param. We should refactor that fetchTokenURL := &url.URL{} *fetchTokenURL = *originURL - cfdToken, err := token.FetchTokenWithRedirect(fetchTokenURL, log) + + appInfo, err := token.GetAppInfo(fetchTokenURL) + if err != nil { + return err + } + cfdToken, err := token.FetchTokenWithRedirect(fetchTokenURL, appInfo, log) if err != nil { return err } - if err := sshgen.GenerateShortLivedCertificate(originURL, cfdToken); err != nil { + if err := sshgen.GenerateShortLivedCertificate(appInfo, cfdToken); err != nil { return err } return nil } -// getAppURL will pull the appURL needed for fetching a user's Access token +// getAppURL will pull the request URL needed for fetching a user's Access token func getAppURL(cmdArgs []string, log *zerolog.Logger) (*url.URL, error) { if len(cmdArgs) < 1 { log.Error().Msg("Please provide a valid URL as the first argument to curl.") @@ -456,7 +476,7 @@ func isFileThere(candidate string) bool { // verifyTokenAtEdge checks for a token on disk, or generates a new one. // Then makes a request to to the origin with the token to ensure it is valid. // Returns nil if token is valid. -func verifyTokenAtEdge(appUrl *url.URL, c *cli.Context, log *zerolog.Logger) error { +func verifyTokenAtEdge(appUrl *url.URL, appInfo *token.AppInfo, c *cli.Context, log *zerolog.Logger) error { headers := buildRequestHeaders(c.StringSlice(sshHeaderFlag)) if c.IsSet(sshTokenIDFlag) { headers.Add(h2mux.CFAccessClientIDHeader, c.String(sshTokenIDFlag)) @@ -464,7 +484,7 @@ func verifyTokenAtEdge(appUrl *url.URL, c *cli.Context, log *zerolog.Logger) err if c.IsSet(sshTokenSecretFlag) { headers.Add(h2mux.CFAccessClientSecretHeader, c.String(sshTokenSecretFlag)) } - options := &carrier.StartOptions{OriginURL: appUrl.String(), Headers: headers} + options := &carrier.StartOptions{AppInfo: appInfo, OriginURL: appUrl.String(), Headers: headers} if valid, err := isTokenValid(options, log); err != nil { return err @@ -472,7 +492,7 @@ func verifyTokenAtEdge(appUrl *url.URL, c *cli.Context, log *zerolog.Logger) err return nil } - if err := token.RemoveTokenIfExists(appUrl); err != nil { + if err := token.RemoveTokenIfExists(appInfo); err != nil { return err } diff --git a/sshgen/sshgen.go b/sshgen/sshgen.go index 2be87444..47cc7297 100644 --- a/sshgen/sshgen.go +++ b/sshgen/sshgen.go @@ -12,7 +12,6 @@ import ( "io" "io/ioutil" "net/http" - "net/url" "time" "github.com/coreos/go-oidc/jose" @@ -52,8 +51,8 @@ type errorResponse struct { var mockRequest func(url, contentType string, body io.Reader) (*http.Response, error) = nil // GenerateShortLivedCertificate generates and stores a keypair for short lived certs -func GenerateShortLivedCertificate(appURL *url.URL, token string) error { - fullName, err := cfpath.GenerateAppTokenFilePathFromURL(appURL, keyName) +func GenerateShortLivedCertificate(appInfo *cfpath.AppInfo, token string) error { + fullName, err := cfpath.GenerateAppTokenFilePathFromURL(appInfo.AppDomain, appInfo.AppAUD, keyName) if err != nil { return err } diff --git a/sshgen/sshgen_test.go b/sshgen/sshgen_test.go index 09f83328..eae9ba4f 100644 --- a/sshgen/sshgen_test.go +++ b/sshgen/sshgen_test.go @@ -9,7 +9,6 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "net/url" "os" "testing" "time" @@ -33,10 +32,10 @@ type signingArguments struct { } func TestCertGenSuccess(t *testing.T) { - url, _ := url.Parse("https://cf-test-access.com/testpath") + appInfo := &cfpath.AppInfo{AppAUD: "abcd1234", AppDomain: "mySite.com"} token := tokenGenerator() - fullName, err := cfpath.GenerateAppTokenFilePathFromURL(url, keyName) + fullName, err := cfpath.GenerateAppTokenFilePathFromURL(appInfo.AppDomain, appInfo.AppAUD, keyName) assert.NoError(t, err) pubKeyName := fullName + ".pub" @@ -66,7 +65,7 @@ func TestCertGenSuccess(t *testing.T) { return w.Result(), nil } - err = GenerateShortLivedCertificate(url, token) + err = GenerateShortLivedCertificate(appInfo, token) assert.NoError(t, err) exist, err := config.FileExists(fullName) diff --git a/token/path.go b/token/path.go index b3d42015..d5b60190 100644 --- a/token/path.go +++ b/token/path.go @@ -2,7 +2,6 @@ package token import ( "fmt" - "net/url" "os" "path/filepath" "strings" @@ -13,12 +12,13 @@ import ( ) // GenerateAppTokenFilePathFromURL will return a filepath for given Access org token -func GenerateAppTokenFilePathFromURL(url *url.URL, suffix string) (string, error) { +func GenerateAppTokenFilePathFromURL(appDomain, aud string, suffix string) (string, error) { configPath, err := getConfigPath() if err != nil { return "", err } - name := strings.Replace(fmt.Sprintf("%s%s-%s", url.Hostname(), url.EscapedPath(), suffix), "/", "-", -1) + name := fmt.Sprintf("%s-%s-%s", appDomain, aud, suffix) + name = strings.Replace(strings.Replace(name, "/", "-", -1), "*", "-", -1) return filepath.Join(configPath, name), nil } diff --git a/token/token.go b/token/token.go index cbcd9f97..29af6559 100644 --- a/token/token.go +++ b/token/token.go @@ -22,10 +22,18 @@ import ( ) const ( - keyName = "token" - tokenHeader = "CF_Authorization" + keyName = "token" + tokenCookie = "CF_Authorization" + appDomainHeader = "CF-Access-Domain" + AccessLoginWorkerPath = "/cdn-cgi/access/login" ) +type AppInfo struct { + AuthDomain string + AppAUD string + AppDomain string +} + type lock struct { lockFilePath string backoff *origin.BackoffHandler @@ -142,23 +150,23 @@ func isTokenLocked(lockFilePath string) bool { // 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, log *zerolog.Logger) (string, error) { - return getToken(appURL, false, log) +func FetchTokenWithRedirect(appURL *url.URL, appInfo *AppInfo, log *zerolog.Logger) (string, error) { + return getToken(appURL, appInfo, false, 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, log *zerolog.Logger) (string, error) { - return getToken(appURL, true, log) +func FetchToken(appURL *url.URL, appInfo *AppInfo, log *zerolog.Logger) (string, error) { + return getToken(appURL, appInfo, true, log) } // getToken will either load a stored token or generate a new one -func getToken(appURL *url.URL, useHostOnly bool, log *zerolog.Logger) (string, error) { - if token, err := GetAppTokenIfExists(appURL); token != "" && err == nil { +func getToken(appURL *url.URL, appInfo *AppInfo, useHostOnly bool, log *zerolog.Logger) (string, error) { + if token, err := GetAppTokenIfExists(appInfo); token != "" && err == nil { return token, nil } - appTokenPath, err := GenerateAppTokenFilePathFromURL(appURL, keyName) + appTokenPath, err := GenerateAppTokenFilePathFromURL(appInfo.AppDomain, appInfo.AppAUD, keyName) if err != nil { return "", errors.Wrap(err, "failed to generate app token file path") } @@ -170,40 +178,36 @@ func getToken(appURL *url.URL, useHostOnly bool, log *zerolog.Logger) (string, e defer fileLockAppToken.Release() // check to see if another process has gotten a token while we waited for the lock - if token, err := GetAppTokenIfExists(appURL); token != "" && err == nil { + if token, err := GetAppTokenIfExists(appInfo); token != "" && err == nil { return token, nil } // If an app token couldnt be found on disk, check for an org token and attempt to exchange it for an app token. var orgTokenPath string - // Get auth domain to format into org token file path - if authDomain, err := getAuthDomain(appURL); err != nil { - log.Error().Msgf("failed to get auth domain: %s", err) - } else { - orgToken, err := GetOrgTokenIfExists(authDomain) + orgToken, err := GetOrgTokenIfExists(appInfo.AuthDomain) + if err != nil { + orgTokenPath, err = generateOrgTokenFilePathFromURL(appInfo.AuthDomain) if err != nil { - orgTokenPath, err = generateOrgTokenFilePathFromURL(authDomain) - if err != nil { - return "", errors.Wrap(err, "failed to generate org token file path") - } - - fileLockOrgToken := newLock(orgTokenPath) - if err = fileLockOrgToken.Acquire(); err != nil { - return "", errors.Wrap(err, "failed to acquire org token lock") - } - defer fileLockOrgToken.Release() - // check if an org token has been created since the lock was acquired - orgToken, err = GetOrgTokenIfExists(authDomain) + return "", errors.Wrap(err, "failed to generate org token file path") } - if err == nil { - if appToken, err := exchangeOrgToken(appURL, orgToken); err != nil { - log.Debug().Msgf("failed to exchange org token for app token: %s", err) - } else { - if err := ioutil.WriteFile(appTokenPath, []byte(appToken), 0600); err != nil { - return "", errors.Wrap(err, "failed to write app token to disk") - } - return appToken, nil + + fileLockOrgToken := newLock(orgTokenPath) + if err = fileLockOrgToken.Acquire(); err != nil { + return "", errors.Wrap(err, "failed to acquire org token lock") + } + defer fileLockOrgToken.Release() + // check if an org token has been created since the lock was acquired + orgToken, err = GetOrgTokenIfExists(appInfo.AuthDomain) + } + if err == nil { + if appToken, err := exchangeOrgToken(appURL, orgToken); err != nil { + log.Debug().Msgf("failed to exchange org token for app token: %s", err) + } else { + // generate app path + if err := ioutil.WriteFile(appTokenPath, []byte(appToken), 0600); err != nil { + return "", errors.Wrap(err, "failed to write app token to disk") } + return appToken, nil } } return getTokensFromEdge(appURL, appTokenPath, orgTokenPath, useHostOnly, log) @@ -242,31 +246,46 @@ func getTokensFromEdge(appURL *url.URL, appTokenPath, orgTokenPath string, useHo } -// getAuthDomain makes a request to the appURL and stops at the first redirect. The 302 location header will contain the +// GetAppInfo makes a request to the appURL and stops at the first redirect. The 302 location header will contain the // auth domain -func getAuthDomain(appURL *url.URL) (string, error) { +func GetAppInfo(reqURL *url.URL) (*AppInfo, error) { client := &http.Client{ // do not follow redirects CheckRedirect: func(req *http.Request, via []*http.Request) error { - return http.ErrUseLastResponse + // stop after hitting login endpoint since it will contain app path + if strings.Contains(via[len(via)-1].URL.Path, AccessLoginWorkerPath) { + return http.ErrUseLastResponse + } + return nil }, Timeout: time.Second * 7, } - authDomainReq, err := http.NewRequest("HEAD", appURL.String(), nil) + appInfoReq, err := http.NewRequest("HEAD", reqURL.String(), nil) if err != nil { - return "", errors.Wrap(err, "failed to create auth domain request") + return nil, errors.Wrap(err, "failed to create app info request") } - resp, err := client.Do(authDomainReq) + resp, err := client.Do(appInfoReq) if err != nil { - return "", errors.Wrap(err, "failed to get auth domain") + return nil, errors.Wrap(err, "failed to get app info") } resp.Body.Close() - location, err := resp.Location() - if err != nil { - return "", fmt.Errorf("failed to get auth domain. Received status code %d from %s", resp.StatusCode, appURL.String()) + location := resp.Request.URL + if !strings.Contains(location.Path, AccessLoginWorkerPath) { + return nil, fmt.Errorf("failed to get Access app info for %s", reqURL.String()) } - return location.Hostname(), nil + + aud := resp.Request.URL.Query().Get("kid") + if aud == "" { + return nil, errors.New("Empty app aud") + } + + domain := resp.Header.Get(appDomainHeader) + if domain == "" { + return nil, errors.New("Empty app domain") + } + + return &AppInfo{location.Hostname(), aud, domain}, nil } @@ -276,8 +295,8 @@ func exchangeOrgToken(appURL *url.URL, orgToken string) (string, error) { client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error { // attach org token to login request - if strings.Contains(req.URL.Path, "cdn-cgi/access/login") { - req.AddCookie(&http.Cookie{Name: tokenHeader, Value: orgToken}) + if strings.Contains(req.URL.Path, AccessLoginWorkerPath) { + req.AddCookie(&http.Cookie{Name: tokenCookie, Value: orgToken}) } // stop after hitting authorized endpoint since it will contain the app token if strings.Contains(via[len(via)-1].URL.Path, "cdn-cgi/access/authorized") { @@ -300,7 +319,7 @@ func exchangeOrgToken(appURL *url.URL, orgToken string) (string, error) { var appToken string for _, c := range resp.Cookies() { //if Org token revoked on exchange, getTokensFromEdge instead - validAppToken := c.Name == tokenHeader && time.Now().Before(c.Expires) + validAppToken := c.Name == tokenCookie && time.Now().Before(c.Expires) if validAppToken { appToken = c.Value break @@ -335,8 +354,8 @@ func GetOrgTokenIfExists(authDomain string) (string, error) { return token.Encode(), nil } -func GetAppTokenIfExists(url *url.URL) (string, error) { - path, err := GenerateAppTokenFilePathFromURL(url, keyName) +func GetAppTokenIfExists(appInfo *AppInfo) (string, error) { + path, err := GenerateAppTokenFilePathFromURL(appInfo.AppDomain, appInfo.AppAUD, keyName) if err != nil { return "", err } @@ -373,8 +392,8 @@ func getTokenIfExists(path string) (*jose.JWT, error) { } // RemoveTokenIfExists removes the a token from local storage if it exists -func RemoveTokenIfExists(url *url.URL) error { - path, err := GenerateAppTokenFilePathFromURL(url, keyName) +func RemoveTokenIfExists(appInfo *AppInfo) error { + path, err := GenerateAppTokenFilePathFromURL(appInfo.AppDomain, appInfo.AppAUD, keyName) if err != nil { return err }