Merge pull request kubernetes-sigs#177 from chuckha/ca-cert-verify

✨ Calculate cert hashes and verify CA certificates

Author: k8s-ci-robot

certs/utils.go

@@ -19,15 +19,19 @@ package certs
 import (
 	"crypto/rand"
 	"crypto/rsa"
+	"crypto/sha256"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"encoding/hex"
 	"fmt"
 	"math"
 	"math/big"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
 	"k8s.io/client-go/tools/clientcmd/api"
+	"k8s.io/client-go/util/cert"
 )
 
 const (
@@ -172,3 +176,22 @@ func (cfg *Config) NewSignedCert(key *rsa.PrivateKey, caCert *x509.Certificate,
 
 	return x509.ParseCertificate(b)
 }
+
+// CertificateHashes hash all the certificates stored in a CA certificate.
+func CertificateHashes(certData []byte) ([]string, error) {
+	certificates, err := cert.ParseCertsPEM(certData)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to parse Cluster CA Certificate")
+	}
+	out := make([]string, 0)
+	for _, c := range certificates {
+		out = append(out, HashCert(c))
+	}
+	return out, nil
+}
+
+// HashCert will calculate the sha256 of the incoming certificate.
+func HashCert(certificate *x509.Certificate) string {
+	spkiHash := sha256.Sum256(certificate.RawSubjectPublicKeyInfo)
+	return "sha256:" + strings.ToLower(hex.EncodeToString(spkiHash[:]))
+}

controllers/kubeadmconfig_controller.go

@@ -117,8 +117,6 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 		return ctrl.Result{}, err
 	}
 
-	// Check for infrastructure ready. If it's not ready then we will requeue the machine until it is.
-	// The cluster-api machine controller set this value.
 	if !cluster.Status.InfrastructureReady {
 		log.Info("Infrastructure is not ready, waiting until ready.")
 		return ctrl.Result{}, nil
@@ -207,14 +205,13 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 
 		certificates, err := r.getClusterCertificates(ctx, cluster.GetName(), config.GetNamespace())
 		if err != nil {
-			if apierrors.IsNotFound(err) {
-				certificates, err = r.createClusterCertificates(ctx, cluster.GetName(), config)
-				if err != nil {
-					log.Error(err, "unable to create cluster certificates")
-					return ctrl.Result{}, err
-				}
-			} else {
-				log.Error(err, "unable to lookup cluster certificates")
+			log.Error(err, "unable to lookup cluster certificates")
+			return ctrl.Result{}, err
+		}
+		if certificates == nil {
+			certificates, err = r.createClusterCertificates(ctx, cluster.GetName(), config)
+			if err != nil {
+				log.Error(err, "unable to create cluster certificates")
 				return ctrl.Result{}, err
 			}
 		}
@@ -260,6 +257,21 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 		return ctrl.Result{}, errors.New("Control plane already exists for the cluster, only KubeadmConfig objects with JoinConfiguration are allowed")
 	}
 
+	// Get certificates to improve security of discovery
+	certificates, err := r.getClusterCertificates(ctx, cluster.GetName(), config.GetNamespace())
+	if err != nil {
+		log.Error(err, "unable to lookup cluster certificates")
+		return ctrl.Result{}, err
+	}
+	if certificates != nil {
+		hashes, err := certs.CertificateHashes(certificates.ClusterCA.Cert)
+		if err == nil {
+			config.Spec.JoinConfiguration.Discovery.BootstrapToken = &kubeadmv1beta1.BootstrapTokenDiscovery{
+				CACertHashes: hashes,
+			}
+		}
+	}
+
 	// ensure that joinConfiguration.Discovery is properly set for joining node on the current cluster
 	if err := r.reconcileDiscovery(cluster, config); err != nil {
 		if requeueErr, ok := errors.Cause(err).(capierrors.HasRequeueAfterError); ok {
@@ -286,6 +298,10 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 			log.Error(err, "unable to locate cluster certificates")
 			return ctrl.Result{}, err
 		}
+		if certificates == nil {
+			log.Info("Cluster CAs have not been created; requeuing to try again")
+			return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+		}
 
 		cloudJoinData, err := cloudinit.NewJoinControlPlane(&cloudinit.ControlPlaneJoinInput{
 			JoinConfiguration: joindata,
@@ -429,8 +445,8 @@ func (r *KubeadmConfigReconciler) reconcileDiscovery(cluster *clusterv1.Cluster,
 	// if BootstrapToken already contains a CACertHashes or UnsafeSkipCAVerification, respect it; otherwise set for UnsafeSkipCAVerification
 	// TODO: set CACertHashes instead of UnsafeSkipCAVerification
 	if len(config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes) == 0 && !config.Spec.JoinConfiguration.Discovery.BootstrapToken.UnsafeSkipCAVerification {
+		log.Info("No CAs were provided. Falling back to insecure discover method by skipping CA Cert validation")
 		config.Spec.JoinConfiguration.Discovery.BootstrapToken.UnsafeSkipCAVerification = true
-		log.Info("Altering JoinConfiguration.Discovery.BootstrapToken", "UnsafeSkipCAVerification", true)
 	}
 
 	return nil
@@ -482,11 +498,14 @@ func (r *KubeadmConfigReconciler) getClusterCertificates(ctx context.Context, cl
 	secret := &corev1.Secret{}
 
 	err := r.Get(ctx, types.NamespacedName{Name: ClusterCertificatesSecretName(clusterName), Namespace: namespace}, secret)
-	if err != nil {
+	switch {
+	case apierrors.IsNotFound(err):
+		return nil, nil
+	case err != nil:
 		return nil, err
+	default:
+		return certs.NewCertificatesFromMap(secret.Data), nil
 	}
-
-	return certs.NewCertificatesFromMap(secret.Data), nil
 }
 
 func (r *KubeadmConfigReconciler) createClusterCertificates(ctx context.Context, clusterName string, config *bootstrapv1.KubeadmConfig) (*certs.Certificates, error) {

controllers/kubeadmconfig_controller_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	fakeclient "k8s.io/client-go/kubernetes/fake"
 	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/klog/klogr"
 	bootstrapv1 "sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/api/v1alpha2"
 	"sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/certs"
 	kubeadmv1beta1 "sigs.k8s.io/cluster-api-bootstrap-provider-kubeadm/kubeadm/v1beta1"
@@ -838,6 +839,58 @@ func TestReconcileTopLevelObjectSettings(t *testing.T) {
 	}
 }
 
+func TestCACertHashesAndUnsafeCAVerifySkip(t *testing.T) {
+	namespace := "default" // hardcoded in the new* functions
+	clusterName := "my-cluster"
+	cluster := newCluster(clusterName)
+	cluster.Status.ControlPlaneInitialized = true
+	cluster.Status.InfrastructureReady = true
+
+	controlPlaneMachineName := "my-machine"
+	machine := newMachine(cluster, controlPlaneMachineName)
+
+	workerMachineName := "my-worker"
+	workerMachine := newMachine(cluster, workerMachineName)
+
+	controlPlaneConfigName := "my-config"
+	config := newKubeadmConfig(machine, controlPlaneConfigName)
+
+	workerConfigName := "worker-join-cfg"
+	workerConfig := newWorkerJoinKubeadmConfig(workerMachine)
+
+	certificates, _ := certs.NewCertificates()
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      ClusterCertificatesSecretName(cluster.GetName()),
+			Namespace: namespace,
+		},
+		Data: certificates.ToMap(),
+	}
+
+	myclient := fake.NewFakeClientWithScheme(setupScheme(), cluster, machine, workerMachine, config, workerConfig, secret)
+
+	reconciler := KubeadmConfigReconciler{
+		Client:               myclient,
+		SecretsClientFactory: newFakeSecretFactory(),
+		KubeadmInitLock:      &myInitLocker{},
+		Log:                  klogr.New(),
+	}
+
+	req := ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: workerConfigName, Namespace: namespace},
+	}
+	if _, err := reconciler.Reconcile(req); err != nil {
+		t.Fatalf("reconciled an error: %v", err)
+	}
+	cfg := &bootstrapv1.KubeadmConfig{}
+	if err := myclient.Get(context.Background(), req.NamespacedName, cfg); err != nil {
+		t.Fatal(err)
+	}
+	if cfg.Spec.JoinConfiguration.Discovery.BootstrapToken.UnsafeSkipCAVerification == true {
+		t.Fatal("Should not skip unsafe")
+	}
+}
+
 // test utils
 
 // newCluster return a CAPI cluster object