From 45dd4afe504d6ae2887e2fc743e8974d3865527d Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Wed, 23 Sep 2020 23:47:17 -0700 Subject: [PATCH] Simplify token parsing Improves readability, reduces round-trips to the join server to validate certs. Signed-off-by: Brad Davidson --- pkg/agent/config/config.go | 5 +- pkg/agent/run.go | 4 +- pkg/clientaccess/clientaccess.go | 186 ++++++++++++++++--------------- pkg/cluster/bootstrap.go | 9 +- 4 files changed, 106 insertions(+), 98 deletions(-) diff --git a/pkg/agent/config/config.go b/pkg/agent/config/config.go index 2ab43ccfaf..dfbc1bcdf6 100644 --- a/pkg/agent/config/config.go +++ b/pkg/agent/config/config.go @@ -54,13 +54,12 @@ func Get(ctx context.Context, agent cmds.Agent, proxy proxy.Proxy) *config.Node type HTTPRequester func(u string, client *http.Client, username, password string) ([]byte, error) func Request(path string, info *clientaccess.Info, requester HTTPRequester) ([]byte, error) { - u, err := url.Parse(info.URL) + u, err := url.Parse(info.BaseURL) if err != nil { return nil, err } u.Path = path - username, password, _ := clientaccess.ParseUsernamePassword(info.Token) - return requester(u.String(), clientaccess.GetHTTPClient(info.CACerts), username, password) + return requester(u.String(), clientaccess.GetHTTPClient(info.CACerts), info.Username, info.Password) } func getNodeNamedCrt(nodeName, nodeIP, nodePasswordFile string) HTTPRequester { diff --git a/pkg/agent/run.go b/pkg/agent/run.go index c02fee8fda..b028df6f19 100644 --- a/pkg/agent/run.go +++ b/pkg/agent/run.go @@ -152,7 +152,7 @@ func Run(ctx context.Context, cfg cmds.Agent) error { } for { - newToken, err := clientaccess.NormalizeAndValidateTokenForUser(proxy.SupervisorURL(), cfg.Token, "node") + newToken, err := clientaccess.ParseAndValidateTokenForUser(proxy.SupervisorURL(), cfg.Token, "node") if err != nil { logrus.Error(err) select { @@ -162,7 +162,7 @@ func Run(ctx context.Context, cfg cmds.Agent) error { } continue } - cfg.Token = newToken + cfg.Token = newToken.String() break } diff --git a/pkg/clientaccess/clientaccess.go b/pkg/clientaccess/clientaccess.go index 7c5239dae6..bb981d8b6a 100644 --- a/pkg/clientaccess/clientaccess.go +++ b/pkg/clientaccess/clientaccess.go @@ -26,15 +26,15 @@ var ( } ) +const ( + tokenPrefix = "K10" + tokenFormat = "%s%s::%s:%s" +) + type OverrideURLCallback func(config []byte) (*url.URL, error) -type clientToken struct { - caHash string - username string - password string -} - -func WriteClientKubeConfig(destFile, url, serverCAFile, clientCertFile, clientKeyFile string) error { +// WriteClientKubeConfig generates a kubeconfig at destFile that can be used to connect to a server at url with the given certs and keys +func WriteClientKubeConfig(destFile string, url string, serverCAFile string, clientCertFile string, clientKeyFile string) error { serverCA, err := ioutil.ReadFile(serverCAFile) if err != nil { return errors.Wrapf(err, "failed to read %s", serverCAFile) @@ -73,96 +73,71 @@ func WriteClientKubeConfig(destFile, url, serverCAFile, clientCertFile, clientKe } type Info struct { - URL string `json:"url,omitempty"` CACerts []byte `json:"cacerts,omitempty"` - username string - password string - Token string `json:"token,omitempty"` + BaseURL string `json:"baseurl,omitempty"` + Username string `json:"username,omitempty"` + Password string `json:"password,omitempty"` + caHash string } -func (i *Info) ToToken() string { - return fmt.Sprintf("K10%s::%s:%s", hashCA(i.CACerts), i.username, i.password) +// String returns the token data, templated according to the token format +func (info *Info) String() string { + return fmt.Sprintf(tokenFormat, tokenPrefix, hashCA(info.CACerts), info.Username, info.Password) } -func NormalizeAndValidateTokenForUser(server, token, user string) (string, error) { - if !strings.HasPrefix(token, "K10") { - token = "K10::" + user + ":" + token - } - info, err := ParseAndValidateToken(server, token) - if err != nil { - return "", err - } - - if info.username != user { - info.username = user - } - - return info.ToToken(), nil -} - -func ParseAndValidateToken(server, token string) (*Info, error) { - url, err := url.Parse(server) - if err != nil { - return nil, errors.Wrapf(err, "Invalid url, failed to parse %s", server) - } - - if url.Scheme != "https" { - return nil, fmt.Errorf("only https:// URLs are supported, invalid scheme: %s", server) - } - - for strings.HasSuffix(url.Path, "/") { - url.Path = url.Path[:len(url.Path)-1] - } - - parsedToken, err := parseToken(token) +// ParseAndValidateToken parses a token, downloads and validates the server's CA bundle, +// and validates it according to the caHash from the token if set. +func ParseAndValidateToken(server string, token string) (*Info, error) { + info, err := parseToken(token) if err != nil { return nil, err } - cacerts, err := GetCACerts(*url) - if err != nil { + if err := info.setServer(server); err != nil { return nil, err } - if len(cacerts) > 0 && len(parsedToken.caHash) > 0 { - if ok, hash, newHash := validateCACerts(cacerts, parsedToken.caHash); !ok { - return nil, fmt.Errorf("token does not match the server %s != %s", hash, newHash) + if info.caHash != "" { + if err := info.validateCAHash(); err != nil { + return nil, err } } - if err := validateToken(*url, cacerts, parsedToken.username, parsedToken.password); err != nil { + return info, nil +} + +// ParseAndValidateToken parses a token with user override, downloads and +// validates the server's CA bundle, and validates it according to the caHash from the token if set. +func ParseAndValidateTokenForUser(server string, token string, username string) (*Info, error) { + info, err := parseToken(token) + if err != nil { return nil, err } - i := &Info{ - URL: url.String(), - CACerts: cacerts, - username: parsedToken.username, - password: parsedToken.password, - Token: token, + info.Username = username + + if err := info.setServer(server); err != nil { + return nil, err } - // normalize token - i.Token = i.ToToken() - return i, nil -} - -func validateToken(u url.URL, cacerts []byte, username, password string) error { - u.Path = "/cacerts" - _, err := get(u.String(), GetHTTPClient(cacerts), username, password) - if err != nil { - return errors.Wrap(err, "token is not valid") + if info.caHash != "" { + if err := info.validateCAHash(); err != nil { + return nil, err + } } - return nil + + return info, nil } -func validateCACerts(cacerts []byte, hash string) (bool, string, string) { +// validateCACerts returns a boolean indicating whether or not a CA bundle matches the provided hash, +// and a string containing the hash of the CA bundle. +func validateCACerts(cacerts []byte, hash string) (bool, string) { if len(cacerts) == 0 && hash == "" { - return true, "", "" + return true, "" } newHash := hashCA(cacerts) - return hash == newHash, hash, newHash + return hash == newHash, newHash } // hashCA returns the hex-encoded SHA256 digest of a byte array. @@ -174,38 +149,40 @@ func hashCA(cacerts []byte) string { // ParseUsernamePassword returns the username and password portion of a token string, // along with a bool indicating if the token was successfully parsed. func ParseUsernamePassword(token string) (string, string, bool) { - parsed, err := parseToken(token) + info, err := parseToken(token) if err != nil { return "", "", false } - return parsed.username, parsed.password, true + return info.Username, info.Password, true } -func parseToken(token string) (clientToken, error) { - var result clientToken +// parseToken parses a token into an Info struct +func parseToken(token string) (*Info, error) { + var info = &Info{} - if !strings.HasPrefix(token, "K10") { - return result, fmt.Errorf("token is not a valid token format") + if !strings.HasPrefix(token, tokenPrefix) { + token = fmt.Sprintf(tokenFormat, tokenPrefix, "", "", token) } - token = token[3:] + // Strip off the prefix + token = token[len(tokenPrefix):] parts := strings.SplitN(token, "::", 2) token = parts[0] if len(parts) > 1 { - result.caHash = parts[0] + info.caHash = parts[0] token = parts[1] } parts = strings.SplitN(token, ":", 2) if len(parts) != 2 { - return result, fmt.Errorf("token credentials are the wrong format") + return nil, fmt.Errorf("invalid token format") } - result.username = parts[0] - result.password = parts[1] + info.Username = parts[0] + info.Password = parts[1] - return result, nil + return info, nil } // GetHTTPClient returns a http client that validates TLS server certificates using the provided CA bundle. @@ -232,18 +209,53 @@ func GetHTTPClient(cacerts []byte) *http.Client { // Get makes a request to a subpath of info's BaseURL func Get(path string, info *Info) ([]byte, error) { - u, err := url.Parse(info.URL) + u, err := url.Parse(info.BaseURL) if err != nil { return nil, err } u.Path = path - return get(u.String(), GetHTTPClient(info.CACerts), info.username, info.password) + return get(u.String(), GetHTTPClient(info.CACerts), info.Username, info.Password) +} + +// setServer sets the BaseURL and CACerts fields of the Info by connecting to the server +// and storing the CA bundle. +func (info *Info) setServer(server string) error { + url, err := url.Parse(server) + if err != nil { + return errors.Wrapf(err, "Invalid server url, failed to parse: %s", server) + } + + if url.Scheme != "https" { + return fmt.Errorf("only https:// URLs are supported, invalid scheme: %s", server) + } + + for strings.HasSuffix(url.Path, "/") { + url.Path = url.Path[:len(url.Path)-1] + } + + cacerts, err := getCACerts(*url) + if err != nil { + return err + } + + info.BaseURL = url.String() + info.CACerts = cacerts + return nil +} + +// ValidateCAHash validates that info's caHash matches the CACerts hash. +func (info *Info) validateCAHash() error { + if ok, serverHash := validateCACerts(info.CACerts, info.caHash); !ok { + return fmt.Errorf("token CA hash does not match the server CA hash: %s != %s", info.caHash, serverHash) + } + + return nil } // getCACerts retrieves the CA bundle from a server. // An error is raised if the CA bundle cannot be retrieved, // or if the server's cert is not signed by the returned bundle. -func GetCACerts(u url.URL) ([]byte, error) { +func getCACerts(u url.URL) ([]byte, error) { u.Path = "/cacerts" url := u.String() @@ -258,7 +270,7 @@ func GetCACerts(u url.URL) ([]byte, error) { // Download the CA bundle using a client that does not validate certs. cacerts, err := get(url, insecureClient, "", "") if err != nil { - return nil, errors.Wrapf(err, "failed to get CA certs at %s", url) + return nil, errors.Wrap(err, "failed to get CA certs") } // Request the CA bundle again, validating that the CA bundle can be loaded @@ -266,7 +278,7 @@ func GetCACerts(u url.URL) ([]byte, error) { // get an empty CA bundle. or if the dynamiclistener cert is incorrectly signed. _, err = get(url, GetHTTPClient(cacerts), "", "") if err != nil { - return nil, errors.Wrapf(err, "server %s is not trusted", url) + return nil, errors.Wrap(err, "CA cert validation failed") } return cacerts, nil diff --git a/pkg/cluster/bootstrap.go b/pkg/cluster/bootstrap.go index dd47fa8bd7..dbe7ca1c93 100644 --- a/pkg/cluster/bootstrap.go +++ b/pkg/cluster/bootstrap.go @@ -64,12 +64,9 @@ func (c *Cluster) shouldBootstrapLoad(ctx context.Context) (bool, error) { return false, errors.New(version.ProgramUpper + "_TOKEN is required to join a cluster") } - token, err := clientaccess.NormalizeAndValidateTokenForUser(c.config.JoinURL, c.config.Token, "server") - if err != nil { - return false, err - } - - info, err := clientaccess.ParseAndValidateToken(c.config.JoinURL, token) + // Fail if the token isn't syntactically valid, or if the CA hash on the remote server doesn't match + // the hash in the token. The password isn't actually checked until later when actually bootstrapping. + info, err := clientaccess.ParseAndValidateTokenForUser(c.config.JoinURL, c.config.Token, "server") if err != nil { return false, err }