feat: check apid client certificate extended key usage

This is enabled via a machine config feature/version contract, as `talosconfig` certificate generated previously didn't have proper key usage set, so we need to keep backwards compatibility on upgrades. New v1.3+ clusters will include this check. This check prevents even potential mis-use of server certificates as a client certificate. Signed-off-by: Andrey Smirnov <[email protected]>
siderolabs · Sep 9, 2022 · 161a52a · 161a52a
1 parent 9dadc4a
commit 161a52a
Show file tree

Hide file tree

Showing 13 changed files with 102 additions and 4 deletions.
diff --git a/internal/app/apid/main.go b/internal/app/apid/main.go
@@ -6,10 +6,12 @@ package apid
 
 import (
 	"context"
+	"crypto/x509"
 	"flag"
 	"fmt"
 	"log"
 	"os/signal"
+	"reflect"
 	"regexp"
 	"syscall"
 	"time"
@@ -34,8 +36,6 @@ import (
 	"github.com/talos-systems/talos/pkg/startup"
 )
 
-var rbacEnabled *bool
-
 func runDebugServer(ctx context.Context) {
 	const debugAddr = ":9981"
 
@@ -55,13 +55,15 @@ func Main() {
 	}
 }
 
+//nolint:gocyclo
 func apidMain() error {
 	ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT)
 	defer cancel()
 
 	log.SetFlags(log.Lshortfile | log.Ldate | log.Lmicroseconds | log.Ltime)
 
-	rbacEnabled = flag.Bool("enable-rbac", false, "enable RBAC for Talos API")
+	rbacEnabled := flag.Bool("enable-rbac", false, "enable RBAC for Talos API")
+	extKeyUsageCheckEnabled := flag.Bool("enable-ext-key-usage-check", false, "enable check for client certificate ext key usage")
 
 	flag.Parse()
 
@@ -91,6 +93,10 @@ func apidMain() error {
 		return fmt.Errorf("failed to create OS-level TLS configuration: %w", err)
 	}
 
+	if *extKeyUsageCheckEnabled {
+		serverTLSConfig.VerifyPeerCertificate = verifyExtKeyUsage
+	}
+
 	clientTLSConfig, err := tlsConfig.ClientConfig()
 	if err != nil {
 		return fmt.Errorf("failed to create client TLS config: %w", err)
@@ -215,3 +221,23 @@ func apidMain() error {
 
 	return errGroup.Wait()
 }
+
+func verifyExtKeyUsage(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
+	if len(verifiedChains) == 0 {
+		return fmt.Errorf("no verified chains")
+	}
+
+	certs := verifiedChains[0]
+
+	for _, cert := range certs {
+		if cert.IsCA {
+			continue
+		}
+
+		if !reflect.DeepEqual(cert.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}) {
+			return fmt.Errorf("certificate %q is missing the client auth extended key usage", cert.Subject)
+		}
+	}
+
+	return nil
+}
diff --git a/internal/app/machined/pkg/system/services/apid.go b/internal/app/machined/pkg/system/services/apid.go
@@ -136,6 +136,10 @@ func (o *APID) Runner(r runtime.Runtime) (runner.Runner, error) {
 		args.ProcessArgs = append(args.ProcessArgs, "--enable-rbac")
 	}
 
+	if r.Config().Machine().Features().ApidCheckExtKeyUsageEnabled() {
+		args.ProcessArgs = append(args.ProcessArgs, "--enable-ext-key-usage-check")
+	}
+
 	// Set the mounts.
 	mounts := []specs.Mount{
 		{Type: "bind", Destination: "/etc/ssl", Source: "/etc/ssl", Options: []string{"bind", "ro"}},

diff --git a/pkg/machinery/config/contract.go b/pkg/machinery/config/contract.go
@@ -24,6 +24,7 @@ type VersionContract struct {
 // Well-known Talos version contracts.
 var (
 	TalosVersionCurrent = (*VersionContract)(nil)
+	TalosVersion1_3     = &VersionContract{1, 3}
 	TalosVersion1_2     = &VersionContract{1, 2}
 	TalosVersion1_1     = &VersionContract{1, 1}
 	TalosVersion1_0     = &VersionContract{1, 0}
@@ -136,3 +137,8 @@ func (contract *VersionContract) KubernetesAllowSchedulingOnControlPlanes() bool
 func (contract *VersionContract) KubernetesDiscoveryBackendDisabled() bool {
 	return contract.Greater(TalosVersion1_1)
 }
+
+// ApidExtKeyUsageCheckEnabled returns true if apid should check ext key usage of client certificates.
+func (contract *VersionContract) ApidExtKeyUsageCheckEnabled() bool {
+	return contract.Greater(TalosVersion1_2)
+}
diff --git a/pkg/machinery/config/contract_test.go b/pkg/machinery/config/contract_test.go
@@ -61,6 +61,27 @@ func TestContractCurrent(t *testing.T) {
 	assert.True(t, contract.KubernetesAlternateImageRegistries())
 	assert.True(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.True(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.True(t, contract.ApidExtKeyUsageCheckEnabled())
+}
+
+func TestContract1_3(t *testing.T) {
+	contract := config.TalosVersion1_3
+
+	assert.True(t, contract.SupportsAggregatorCA())
+	assert.True(t, contract.SupportsECDSAKeys())
+	assert.True(t, contract.SupportsServiceAccount())
+	assert.True(t, contract.SupportsRBACFeature())
+	assert.True(t, contract.SupportsDynamicCertSANs())
+	assert.True(t, contract.SupportsECDSASHA256())
+	assert.True(t, contract.ClusterDiscoveryEnabled())
+	assert.False(t, contract.PodSecurityPolicyEnabled())
+	assert.True(t, contract.PodSecurityAdmissionEnabled())
+	assert.True(t, contract.StableHostnameEnabled())
+	assert.True(t, contract.KubeletDefaultRuntimeSeccompProfileEnabled())
+	assert.True(t, contract.KubernetesAlternateImageRegistries())
+	assert.True(t, contract.KubernetesAllowSchedulingOnControlPlanes())
+	assert.True(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.True(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract1_2(t *testing.T) {
@@ -80,6 +101,7 @@ func TestContract1_2(t *testing.T) {
 	assert.True(t, contract.KubernetesAlternateImageRegistries())
 	assert.True(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.True(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract1_1(t *testing.T) {
@@ -99,6 +121,7 @@ func TestContract1_1(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract1_0(t *testing.T) {
@@ -118,6 +141,7 @@ func TestContract1_0(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_14(t *testing.T) {
@@ -137,6 +161,7 @@ func TestContract0_14(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_13(t *testing.T) {
@@ -156,6 +181,7 @@ func TestContract0_13(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_12(t *testing.T) {
@@ -175,6 +201,7 @@ func TestContract0_12(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_11(t *testing.T) {
@@ -194,6 +221,7 @@ func TestContract0_11(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_10(t *testing.T) {
@@ -213,6 +241,7 @@ func TestContract0_10(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_9(t *testing.T) {
@@ -232,6 +261,7 @@ func TestContract0_9(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
 
 func TestContract0_8(t *testing.T) {
@@ -251,4 +281,5 @@ func TestContract0_8(t *testing.T) {
 	assert.False(t, contract.KubernetesAlternateImageRegistries())
 	assert.False(t, contract.KubernetesAllowSchedulingOnControlPlanes())
 	assert.False(t, contract.KubernetesDiscoveryBackendDisabled())
+	assert.False(t, contract.ApidExtKeyUsageCheckEnabled())
 }
diff --git a/pkg/machinery/config/provider.go b/pkg/machinery/config/provider.go
@@ -529,6 +529,7 @@ type Features interface {
 	RBACEnabled() bool
 	StableHostnameEnabled() bool
 	KubernetesTalosAPIAccess() KubernetesTalosAPIAccess
+	ApidCheckExtKeyUsageEnabled() bool
 }
 
 // KubernetesTalosAPIAccess describes the Kubernetes Talos API access features.

diff --git a/pkg/machinery/config/types/v1alpha1/generate/generate.go b/pkg/machinery/config/types/v1alpha1/generate/generate.go
@@ -10,6 +10,7 @@ package generate
 import (
 	"bufio"
 	"crypto/rand"
+	stdx509 "crypto/x509"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -576,6 +577,8 @@ func NewAdminCertificateAndKey(currentTime time.Time, ca *x509.PEMEncodedCertifi
 		x509.Organization(roles.Strings()...),
 		x509.NotAfter(currentTime.Add(ttl)),
 		x509.NotBefore(currentTime),
+		x509.KeyUsage(stdx509.KeyUsageDigitalSignature),
+		x509.ExtKeyUsage([]stdx509.ExtKeyUsage{stdx509.ExtKeyUsageClientAuth}),
 	}
 
 	talosCA, err := x509.NewCertificateAuthorityFromCertificateAndKey(ca)

diff --git a/pkg/machinery/config/types/v1alpha1/generate/init.go b/pkg/machinery/config/types/v1alpha1/generate/init.go
@@ -65,6 +65,10 @@ func initUd(in *Input) (*v1alpha1.Config, error) {
 		machine.MachineFeatures.StableHostname = pointer.To(true)
 	}
 
+	if in.VersionContract.ApidExtKeyUsageCheckEnabled() {
+		machine.MachineFeatures.ApidCheckExtKeyUsage = pointer.To(true)
+	}
+
 	if in.VersionContract.KubeletDefaultRuntimeSeccompProfileEnabled() {
 		machine.MachineKubelet.KubeletDefaultRuntimeSeccompProfileEnabled = pointer.To(true)
 	}

diff --git a/pkg/machinery/config/types/v1alpha1/generate/worker.go b/pkg/machinery/config/types/v1alpha1/generate/worker.go
@@ -66,6 +66,10 @@ func workerUd(in *Input) (*v1alpha1.Config, error) {
 		machine.MachineFeatures.StableHostname = pointer.To(true)
 	}
 
+	if in.VersionContract.ApidExtKeyUsageCheckEnabled() {
+		machine.MachineFeatures.ApidCheckExtKeyUsage = pointer.To(true)
+	}
+
 	if in.VersionContract.KubeletDefaultRuntimeSeccompProfileEnabled() {
 		machine.MachineKubelet.KubeletDefaultRuntimeSeccompProfileEnabled = pointer.To(true)
 	}

diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_features.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_features.go
@@ -28,3 +28,8 @@ func (f *FeaturesConfig) StableHostnameEnabled() bool {
 func (f *FeaturesConfig) KubernetesTalosAPIAccess() config.KubernetesTalosAPIAccess {
 	return f.KubernetesTalosAPIAccessConfig
 }
+
+// ApidCheckExtKeyUsageEnabled implements config.Features interface.
+func (f *FeaturesConfig) ApidCheckExtKeyUsageEnabled() bool {
+	return pointer.SafeDeref(f.ApidCheckExtKeyUsage)
+}
diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_types.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_types.go
@@ -2379,6 +2379,9 @@ type FeaturesConfig struct {
 	//   examples:
 	//     - value: kubernetesTalosAPIAccessConfigExample
 	KubernetesTalosAPIAccessConfig *KubernetesTalosAPIAccessConfig `yaml:"kubernetesTalosAPIAccess,omitempty"`
+	//   description: |
+	//     Enable checks for extended key usage of client certificates in apid.
+	ApidCheckExtKeyUsage *bool `yaml:"apidCheckExtKeyUsage,omitempty"`
 }
 
 // KubernetesTalosAPIAccessConfig describes the configuration for the Talos API access from Kubernetes pods.

diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_types_doc.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_types_doc.go
diff --git a/pkg/machinery/config/types/v1alpha1/zz_generated.deepcopy.go b/pkg/machinery/config/types/v1alpha1/zz_generated.deepcopy.go
diff --git a/website/content/v1.3/reference/configuration.md b/website/content/v1.3/reference/configuration.md
@@ -2600,6 +2600,7 @@ kubernetesTalosAPIAccess:
     allowedKubernetesNamespaces:
         - kube-system
 {{< /highlight >}}</details> | |
+|`apidCheckExtKeyUsage` |bool |Enable checks for extended key usage of client certificates in apid.  | |