Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: enhanced TLS validation for token-based discovery in kubeadm join #49520

Merged
merged 3 commits into from
Aug 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,21 @@ type NodeConfiguration struct {
NodeName string
TLSBootstrapToken string
Token string

// DiscoveryTokenCACertHashes specifies a set of public key pins to verify
// when token-based discovery is used. The root CA found during discovery
// must match one of these values. Specifying an empty set disables root CA
// pinning, which can be unsafe. Each hash is specified as "<type>:<value>",
// where the only currently supported type is "sha256". This is a hex-encoded
// SHA-256 hash of the Subject Public Key Info (SPKI) object in DER-encoded
// ASN.1. These hashes can be calculated using, for example, OpenSSL:
// openssl x509 -pubkey -in ca.crt openssl rsa -pubin -outform der 2>&/dev/null | openssl dgst -sha256 -hex
DiscoveryTokenCACertHashes []string

// DiscoveryTokenUnsafeSkipCAVerification allows token-based discovery
// without CA verification via DiscoveryTokenCACertHashes. This can weaken
// the security of kubeadm since other nodes can impersonate the master.
DiscoveryTokenUnsafeSkipCAVerification bool
}

func (cfg *MasterConfiguration) GetMasterEndpoint() string {
Expand Down
15 changes: 15 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,19 @@ type NodeConfiguration struct {
NodeName string `json:"nodeName"`
TLSBootstrapToken string `json:"tlsBootstrapToken"`
Token string `json:"token"`

// DiscoveryTokenCACertHashes specifies a set of public key pins to verify
// when token-based discovery is used. The root CA found during discovery
// must match one of these values. Specifying an empty set disables root CA
// pinning, which can be unsafe. Each hash is specified as "<type>:<value>",
// where the only currently supported type is "sha256". This is a hex-encoded
// SHA-256 hash of the Subject Public Key Info (SPKI) object in DER-encoded
// ASN.1. These hashes can be calculated using, for example, OpenSSL:
// openssl x509 -pubkey -in ca.crt openssl rsa -pubin -outform der 2>&/dev/null | openssl dgst -sha256 -hex
DiscoveryTokenCACertHashes []string `json:"discoveryTokenCACertHashes"`

// DiscoveryTokenUnsafeSkipCAVerification allows token-based discovery
// without CA verification via DiscoveryTokenCACertHashes. This can weaken
// the security of kubeadm since other nodes can impersonate the master.
DiscoveryTokenUnsafeSkipCAVerification bool `json:"discoveryTokenUnsafeSkipCAVerification"`
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ func (in *NodeConfiguration) DeepCopyInto(out *NodeConfiguration) {
*out = make([]string, len(*in))
copy(*out, *in)
}
if in.DiscoveryTokenCACertHashes != nil {
in, out := &in.DiscoveryTokenCACertHashes, &out.DiscoveryTokenCACertHashes
*out = make([]string, len(*in))
copy(*out, *in)
}
return
}

Expand Down
11 changes: 11 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ func ValidateArgSelection(cfg *kubeadm.NodeConfiguration, fldPath *field.Path) f
if len(cfg.DiscoveryTokenAPIServers) < 1 && len(cfg.DiscoveryToken) != 0 {
allErrs = append(allErrs, field.Required(fldPath, "DiscoveryTokenAPIServers not set"))
}

if len(cfg.DiscoveryFile) != 0 && len(cfg.DiscoveryTokenCACertHashes) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath, "", "DiscoveryTokenCACertHashes cannot be used with DiscoveryFile"))
}

// TODO: convert this warning to an error after v1.8
if len(cfg.DiscoveryFile) == 0 && len(cfg.DiscoveryTokenCACertHashes) == 0 && !cfg.DiscoveryTokenUnsafeSkipCAVerification {
fmt.Println("[validation] WARNING: using token-based discovery without DiscoveryTokenCACertHashes can be unsafe (see https://kubernetes.io/docs/admin/kubeadm/#kubeadm-join).")
fmt.Println("[validation] WARNING: Pass --discovery-token-unsafe-skip-ca-verification to disable this warning. This warning will become an error in Kubernetes 1.9.")
}

// TODO remove once we support multiple api servers
if len(cfg.DiscoveryTokenAPIServers) > 1 {
fmt.Println("[validation] WARNING: kubeadm doesn't fully support multiple API Servers yet")
Expand Down
5 changes: 5 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/zz_generated.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ func (in *NodeConfiguration) DeepCopyInto(out *NodeConfiguration) {
*out = make([]string, len(*in))
copy(*out, *in)
}
if in.DiscoveryTokenCACertHashes != nil {
in, out := &in.DiscoveryTokenCACertHashes, &out.DiscoveryTokenCACertHashes
*out = make([]string, len(*in))
copy(*out, *in)
}
return
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/kubeadm/app/cmd/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ go_library(
"//cmd/kubeadm/app/phases/apiconfig:go_default_library",
"//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library",
"//cmd/kubeadm/app/phases/bootstraptoken/node:go_default_library",
"//cmd/kubeadm/app/phases/certs/pkiutil:go_default_library",
"//cmd/kubeadm/app/phases/controlplane:go_default_library",
"//cmd/kubeadm/app/phases/kubeconfig:go_default_library",
"//cmd/kubeadm/app/phases/markmaster:go_default_library",
Expand All @@ -41,6 +42,7 @@ go_library(
"//cmd/kubeadm/app/util:go_default_library",
"//cmd/kubeadm/app/util/config:go_default_library",
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
"//cmd/kubeadm/app/util/pubkeypin:go_default_library",
"//cmd/kubeadm/app/util/token:go_default_library",
"//pkg/api:go_default_library",
"//pkg/bootstrap/api:go_default_library",
Expand Down
11 changes: 10 additions & 1 deletion cmd/kubeadm/app/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
apiconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/apiconfig"
clusterinfophase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo"
nodebootstraptokenphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil"
controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane"
kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig"
markmasterphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/markmaster"
Expand All @@ -46,6 +47,7 @@ import (
"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
"k8s.io/kubernetes/cmd/kubeadm/app/util/pubkeypin"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/util/version"
)
Expand All @@ -67,7 +69,7 @@ var (
You can now join any number of machines by running the following on each node
as root:

kubeadm join --token {{.Token}} {{.MasterIP}}:{{.MasterPort}}
kubeadm join --token {{.Token}} {{.MasterIP}}:{{.MasterPort}} --discovery-token-ca-cert-hash {{.CAPubKeyPin}}

`)))
)
Expand Down Expand Up @@ -310,10 +312,17 @@ func (i *Init) Run(out io.Writer) error {
}
}

// Load the CA certificate from so we can pin its public key
caCert, err := pkiutil.TryLoadCertFromDisk(i.cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName)
if err != nil {
return err
}

ctx := map[string]string{
"KubeConfigPath": filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.AdminKubeConfigFileName),
"KubeConfigName": kubeadmconstants.AdminKubeConfigFileName,
"Token": i.cfg.Token,
"CAPubKeyPin": pubkeypin.Hash(caCert),
"MasterIP": i.cfg.API.AdvertiseAddress,
"MasterPort": strconv.Itoa(int(i.cfg.API.BindPort)),
}
Expand Down
22 changes: 22 additions & 0 deletions cmd/kubeadm/app/cmd/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
the discovery information is loaded from a URL, HTTPS must be used and
the host installed CA bundle is used to verify the connection.

If you use a shared token for discovery, you should also pass the
--discovery-token-ca-cert-hash flag to validate the public key of the
root certificate authority (CA) presented by the Kubernetes Master. The
value of this flag is specified as "<hash-type>:<hex-encoded-value>",
where the supported hash type is "sha256". The hash is calculated over
the bytes of the Subject Public Key Info (SPKI) object (as in RFC7469).
This value is available in the output of "kubeadm init" or can be
calcuated using standard tools. The --discovery-token-ca-cert-hash flag
may be repeated multiple times to allow more than one public key.

If you cannot know the CA public key hash ahead of time, you can pass
the --discovery-token-unsafe-skip-ca-verification flag to disable this
verification. This weakens the kubeadm security model since other nodes
can potentially impersonate the Kubernetes Master.

The TLS bootstrap mechanism is also driven via a shared token. This is
used to temporarily authenticate with the Kubernetes Master to submit a
certificate signing request (CSR) for a locally created key pair. By
Expand Down Expand Up @@ -117,6 +132,13 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
cmd.PersistentFlags().StringVar(
&cfg.TLSBootstrapToken, "tls-bootstrap-token", "",
"A token used for TLS bootstrapping")
cmd.PersistentFlags().StringSliceVar(
&cfg.DiscoveryTokenCACertHashes, "discovery-token-ca-cert-hash", []string{},
"For token-based discovery, validate that the root CA public key matches this hash (format: \"<type>:<value>\").")
cmd.PersistentFlags().BoolVar(
&cfg.DiscoveryTokenUnsafeSkipCAVerification, "discovery-token-unsafe-skip-ca-verification", false,
"For token-based discovery, allow joining without --discovery-token-ca-cert-hash pinning.")

cmd.PersistentFlags().StringVar(
&cfg.Token, "token", "",
"Use this token for both discovery-token and tls-bootstrap-token")
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func GetValidatedClusterInfoObject(cfg *kubeadmapi.NodeConfiguration) (*clientcm
}
return file.RetrieveValidatedClusterInfo(cfg.DiscoveryFile)
case len(cfg.DiscoveryToken) != 0:
return token.RetrieveValidatedClusterInfo(cfg.DiscoveryToken, cfg.DiscoveryTokenAPIServers)
return token.RetrieveValidatedClusterInfo(cfg.DiscoveryToken, cfg.DiscoveryTokenAPIServers, cfg.DiscoveryTokenCACertHashes)
default:
return nil, fmt.Errorf("couldn't find a valid discovery configuration.")
}
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/discovery/token/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
deps = [
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
"//cmd/kubeadm/app/util/pubkeypin:go_default_library",
"//cmd/kubeadm/app/util/token:go_default_library",
"//pkg/bootstrap/api:go_default_library",
"//pkg/controller/bootstrap:go_default_library",
Expand Down
120 changes: 104 additions & 16 deletions cmd/kubeadm/app/discovery/token/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ limitations under the License.
package token

import (
"bytes"
"crypto/x509"
"encoding/pem"
"fmt"
"sync"

Expand All @@ -27,6 +30,7 @@ import (
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
"k8s.io/kubernetes/cmd/kubeadm/app/util/pubkeypin"
tokenutil "k8s.io/kubernetes/cmd/kubeadm/app/util/token"
bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
"k8s.io/kubernetes/pkg/controller/bootstrap"
Expand All @@ -35,58 +39,123 @@ import (
const BootstrapUser = "token-bootstrap-client"

// RetrieveValidatedClusterInfo connects to the API Server and tries to fetch the cluster-info ConfigMap
// It then makes sure it can trust the API Server by looking at the JWS-signed tokens
func RetrieveValidatedClusterInfo(discoveryToken string, tokenAPIServers []string) (*clientcmdapi.Cluster, error) {

// It then makes sure it can trust the API Server by looking at the JWS-signed tokens and (if rootCAPubKeys is not empty)
// validating the cluster CA against a set of pinned public keys
func RetrieveValidatedClusterInfo(discoveryToken string, tokenAPIServers, rootCAPubKeys []string) (*clientcmdapi.Cluster, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only high-level comment here is that this function becomes a little bit too long in my opinion, but that's nothing we should hold up this PR for

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. It's also lacking test coverage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, can't be unit tested currently.
However, I'm fine with deferring that to a follow-up, this PR is already quite big

tokenId, tokenSecret, err := tokenutil.ParseToken(discoveryToken)
if err != nil {
return nil, err
}

// Load the cfg.DiscoveryTokenCACertHashes into a pubkeypin.Set
pubKeyPins := pubkeypin.NewSet()
err = pubKeyPins.Allow(rootCAPubKeys...)
if err != nil {
return nil, err
}

// The function below runs for every endpoint, and all endpoints races with each other.
// The endpoint that wins the race and completes the task first gets its kubeconfig returned below
baseKubeConfig := runForEndpointsAndReturnFirst(tokenAPIServers, func(endpoint string) (*clientcmdapi.Config, error) {

bootstrapConfig := buildInsecureBootstrapKubeConfig(endpoint)
clusterName := bootstrapConfig.Contexts[bootstrapConfig.CurrentContext].Cluster
insecureBootstrapConfig := buildInsecureBootstrapKubeConfig(endpoint)
clusterName := insecureBootstrapConfig.Contexts[insecureBootstrapConfig.CurrentContext].Cluster

client, err := kubeconfigutil.KubeConfigToClientSet(bootstrapConfig)
insecureClient, err := kubeconfigutil.KubeConfigToClientSet(insecureBootstrapConfig)
if err != nil {
return nil, err
}

fmt.Printf("[discovery] Created cluster-info discovery client, requesting info from %q\n", bootstrapConfig.Clusters[clusterName].Server)
fmt.Printf("[discovery] Created cluster-info discovery client, requesting info from %q\n", insecureBootstrapConfig.Clusters[clusterName].Server)

var clusterinfo *v1.ConfigMap
// Make an initial insecure connection to get the cluster-info ConfigMap
var insecureClusterInfo *v1.ConfigMap
wait.PollImmediateInfinite(constants.DiscoveryRetryInterval, func() (bool, error) {
var err error
clusterinfo, err = client.CoreV1().ConfigMaps(metav1.NamespacePublic).Get(bootstrapapi.ConfigMapClusterInfo, metav1.GetOptions{})
insecureClusterInfo, err = insecureClient.CoreV1().ConfigMaps(metav1.NamespacePublic).Get(bootstrapapi.ConfigMapClusterInfo, metav1.GetOptions{})
if err != nil {
fmt.Printf("[discovery] Failed to request cluster info, will try again: [%s]\n", err)
return false, nil
}
return true, nil
})

kubeConfigString, ok := clusterinfo.Data[bootstrapapi.KubeConfigKey]
if !ok || len(kubeConfigString) == 0 {
// Validate the MAC on the kubeconfig from the ConfigMap and load it
insecureKubeconfigString, ok := insecureClusterInfo.Data[bootstrapapi.KubeConfigKey]
if !ok || len(insecureKubeconfigString) == 0 {
return nil, fmt.Errorf("there is no %s key in the %s ConfigMap. This API Server isn't set up for token bootstrapping, can't connect", bootstrapapi.KubeConfigKey, bootstrapapi.ConfigMapClusterInfo)
}
detachedJWSToken, ok := clusterinfo.Data[bootstrapapi.JWSSignatureKeyPrefix+tokenId]
detachedJWSToken, ok := insecureClusterInfo.Data[bootstrapapi.JWSSignatureKeyPrefix+tokenId]
if !ok || len(detachedJWSToken) == 0 {
return nil, fmt.Errorf("there is no JWS signed token in the %s ConfigMap. This token id %q is invalid for this cluster, can't connect", bootstrapapi.ConfigMapClusterInfo, tokenId)
}
if !bootstrap.DetachedTokenIsValid(detachedJWSToken, kubeConfigString, tokenId, tokenSecret) {
if !bootstrap.DetachedTokenIsValid(detachedJWSToken, insecureKubeconfigString, tokenId, tokenSecret) {
return nil, fmt.Errorf("failed to verify JWS signature of received cluster info object, can't trust this API Server")
}
insecureKubeconfigBytes := []byte(insecureKubeconfigString)
insecureConfig, err := clientcmd.Load(insecureKubeconfigBytes)
if err != nil {
return nil, fmt.Errorf("couldn't parse the kubeconfig file in the %s configmap: %v", bootstrapapi.ConfigMapClusterInfo, err)
}

// If no TLS root CA pinning was specified, we're done
if pubKeyPins.Empty() {
fmt.Printf("[discovery] Cluster info signature and contents are valid and no TLS pinning was specified, will use API Server %q\n", endpoint)
return insecureConfig, nil
}

// Load the cluster CA from the Config
if len(insecureConfig.Clusters) != 1 {
return nil, fmt.Errorf("expected the kubeconfig file in the %s configmap to have a single cluster, but it had %d", bootstrapapi.ConfigMapClusterInfo, len(insecureConfig.Clusters))
}
var clusterCABytes []byte
for _, cluster := range insecureConfig.Clusters {
clusterCABytes = cluster.CertificateAuthorityData
}
clusterCA, err := parsePEMCert(clusterCABytes)
if err != nil {
return nil, fmt.Errorf("failed to parse cluster CA from the %s configmap: %v", bootstrapapi.ConfigMapClusterInfo, err)

}

// Validate the cluster CA public key against the pinned set
err = pubKeyPins.Check(clusterCA)
if err != nil {
return nil, fmt.Errorf("cluster CA found in %s configmap is invalid: %v", bootstrapapi.ConfigMapClusterInfo, err)
}

// Now that we know the proported cluster CA, connect back a second time validating with that CA
secureBootstrapConfig := buildSecureBootstrapKubeConfig(endpoint, clusterCABytes)
secureClient, err := kubeconfigutil.KubeConfigToClientSet(secureBootstrapConfig)
if err != nil {
return nil, err
}

fmt.Printf("[discovery] Requesting info from %q again to validate TLS against the pinned public key\n", insecureBootstrapConfig.Clusters[clusterName].Server)
var secureClusterInfo *v1.ConfigMap
wait.PollImmediateInfinite(constants.DiscoveryRetryInterval, func() (bool, error) {
var err error
secureClusterInfo, err = secureClient.CoreV1().ConfigMaps(metav1.NamespacePublic).Get(bootstrapapi.ConfigMapClusterInfo, metav1.GetOptions{})
if err != nil {
fmt.Printf("[discovery] Failed to request cluster info, will try again: [%s]\n", err)
return false, nil
}
return true, nil
})

finalConfig, err := clientcmd.Load([]byte(kubeConfigString))
// Pull the kubeconfig from the securely-obtained ConfigMap and validate that it's the same as what we found the first time
secureKubeconfigBytes := []byte(secureClusterInfo.Data[bootstrapapi.KubeConfigKey])
if !bytes.Equal(secureKubeconfigBytes, insecureKubeconfigBytes) {
return nil, fmt.Errorf("the second kubeconfig from the %s configmap (using validated TLS) was different from the first", bootstrapapi.ConfigMapClusterInfo)
}

secureKubeconfig, err := clientcmd.Load(secureKubeconfigBytes)
if err != nil {
return nil, fmt.Errorf("couldn't parse the kubeconfig file in the %s configmap: %v", bootstrapapi.ConfigMapClusterInfo, err)
}

fmt.Printf("[discovery] Cluster info signature and contents are valid, will use API Server %q\n", bootstrapConfig.Clusters[clusterName].Server)
return finalConfig, nil
fmt.Printf("[discovery] Cluster info signature and contents are valid and TLS certificate validates against pinned roots, will use API Server %q\n", endpoint)
return secureKubeconfig, nil
})

return kubeconfigutil.GetClusterFromKubeConfig(baseKubeConfig), nil
Expand All @@ -101,6 +170,13 @@ func buildInsecureBootstrapKubeConfig(endpoint string) *clientcmdapi.Config {
return bootstrapConfig
}

// buildSecureBootstrapKubeConfig makes a KubeConfig object that connects securely to the API Server for bootstrapping purposes (validating with the specified CA)
func buildSecureBootstrapKubeConfig(endpoint string, caCert []byte) *clientcmdapi.Config {
masterEndpoint := fmt.Sprintf("https://%s", endpoint)
bootstrapConfig := kubeconfigutil.CreateBasic(masterEndpoint, "kubernetes", BootstrapUser, caCert)
return bootstrapConfig
}

// runForEndpointsAndReturnFirst loops the endpoints slice and let's the endpoints race for connecting to the master
func runForEndpointsAndReturnFirst(endpoints []string, fetchKubeConfigFunc func(string) (*clientcmdapi.Config, error)) *clientcmdapi.Config {
stopChan := make(chan struct{})
Expand Down Expand Up @@ -131,3 +207,15 @@ func runForEndpointsAndReturnFirst(endpoints []string, fetchKubeConfigFunc func(
wg.Wait()
return resultingKubeConfig
}

// parsePEMCert decodes a PEM-formatted certificate and returns it as an x509.Certificate
func parsePEMCert(certData []byte) (*x509.Certificate, error) {
pemBlock, trailingData := pem.Decode(certData)
if pemBlock == nil {
return nil, fmt.Errorf("invalid PEM data")
}
if len(trailingData) != 0 {
return nil, fmt.Errorf("trailing data after first PEM block")
}
return x509.ParseCertificate(pemBlock.Bytes)
}
Loading