fix: require accepted CAs on worker nodes

Note: this issue never happens with default Talos worker configuration (generated by Omni, `talosctl gen config` or CABPT). Before change #4294 3 years ago, worker nodes connected to trustd in "insecure" mode (without validating the trustd server certificate). The change kept backwards compatibility, so it still allowed insecure mode on upgrades. Now it's time to break this compatibility promise, and require accepted CAs to be always present. Adds validation for machine configuration, so if upgrade is attempeted, it would not validate the machine config without accepted CAs. Now lack of accepted CAs would lead to failure to connect to trustd. Signed-off-by: Andrey Smirnov <[email protected]>
siderolabs · May 23, 2024 · 2e64e9e · 2e64e9e
1 parent 23c1c45
commit 2e64e9e
Show file tree

Hide file tree

Showing 7 changed files with 297 additions and 15 deletions.
diff --git a/internal/app/machined/pkg/controllers/secrets/api.go b/internal/app/machined/pkg/controllers/secrets/api.go
@@ -337,7 +337,7 @@ func (ctrl *APIController) generateControlPlane(ctx context.Context, r controlle
 func (ctrl *APIController) generateWorker(ctx context.Context, r controller.Runtime, logger *zap.Logger,
 	rootSpec *secrets.OSRootSpec, endpointsStr []string, certSANs *secrets.CertSANSpec,
 ) error {
-	remoteGen, err := gen.NewRemoteGenerator(rootSpec.Token, endpointsStr, rootSpec.IssuingCA)
+	remoteGen, err := gen.NewRemoteGenerator(rootSpec.Token, endpointsStr, rootSpec.AcceptedCAs)
 	if err != nil {
 		return fmt.Errorf("failed creating trustd client: %w", err)
 	}

diff --git a/internal/integration/api/apply-config.go b/internal/integration/api/apply-config.go
@@ -8,6 +8,7 @@ package api
 
 import (
 	"context"
+	"os"
 	"sort"
 	"testing"
 	"time"
@@ -397,7 +398,14 @@ func (suite *ApplyConfigSuite) TestApplyDryRun() {
 
 	cfgDataOut := suite.PatchV1Alpha1Config(provider, func(cfg *v1alpha1.Config) {
 		// this won't be possible without a reboot
-		cfg.MachineConfig.MachineType = "controlplane"
+		cfg.MachineConfig.MachineFiles = append(cfg.MachineConfig.MachineFiles,
+			&v1alpha1.MachineFile{
+				FileContent:     "test",
+				FilePermissions: v1alpha1.FileMode(os.ModePerm),
+				FilePath:        "/var/lib/test",
+				FileOp:          "create",
+			},
+		)
 	})
 
 	reply, err := suite.Client.ApplyConfiguration(

diff --git a/pkg/grpc/gen/remote.go b/pkg/grpc/gen/remote.go
@@ -28,7 +28,7 @@ type RemoteGenerator struct {
 }
 
 // NewRemoteGenerator initializes a RemoteGenerator with a preconfigured grpc.ClientConn.
-func NewRemoteGenerator(token string, endpoints []string, ca *x509.PEMEncodedCertificateAndKey) (g *RemoteGenerator, err error) {
+func NewRemoteGenerator(token string, endpoints []string, acceptedCAs []*x509.PEMEncodedCertificate) (g *RemoteGenerator, err error) {
 	if len(endpoints) == 0 {
 		return nil, errors.New("at least one root of trust endpoint is required")
 	}
@@ -37,7 +37,7 @@ func NewRemoteGenerator(token string, endpoints []string, ca *x509.PEMEncodedCer
 
 	g = &RemoteGenerator{}
 
-	conn, err := basic.NewConnection(fmt.Sprintf("%s:https:///%s", resolver.RoundRobinResolverScheme, strings.Join(endpoints, ",")), basic.NewTokenCredentials(token), ca)
+	conn, err := basic.NewConnection(fmt.Sprintf("%s:https:///%s", resolver.RoundRobinResolverScheme, strings.Join(endpoints, ",")), basic.NewTokenCredentials(token), acceptedCAs)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/grpc/middleware/auth/basic/basic.go b/pkg/grpc/middleware/auth/basic/basic.go
@@ -5,10 +5,12 @@
 package basic
 
 import (
+	"bytes"
 	"crypto/tls"
 	stdx509 "crypto/x509"
 
 	"github.com/siderolabs/crypto/x509"
+	"github.com/siderolabs/gen/xslices"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 )
@@ -22,15 +24,19 @@ type Credentials interface {
 
 // NewConnection initializes a grpc.ClientConn configured for basic
 // authentication.
-func NewConnection(address string, creds credentials.PerRPCCredentials, ca *x509.PEMEncodedCertificateAndKey) (conn *grpc.ClientConn, err error) {
+func NewConnection(address string, creds credentials.PerRPCCredentials, acceptedCAs []*x509.PEMEncodedCertificate) (conn *grpc.ClientConn, err error) {
 	tlsConfig := &tls.Config{}
 
-	if ca == nil {
-		tlsConfig.InsecureSkipVerify = true
-	} else {
-		tlsConfig.RootCAs = stdx509.NewCertPool()
-		tlsConfig.RootCAs.AppendCertsFromPEM(ca.Crt)
-	}
+	tlsConfig.RootCAs = stdx509.NewCertPool()
+	tlsConfig.RootCAs.AppendCertsFromPEM(bytes.Join(
+		xslices.Map(
+			acceptedCAs,
+			func(cert *x509.PEMEncodedCertificate) []byte {
+				return cert.Crt
+			},
+		),
+		nil,
+	))
 
 	grpcOpts := []grpc.DialOption{
 		grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)),

diff --git a/pkg/machinery/config/container/container_test.go b/pkg/machinery/config/container/container_test.go
@@ -8,6 +8,7 @@ import (
 	"net/url"
 	"testing"
 
+	"github.com/siderolabs/crypto/x509"
 	"github.com/siderolabs/gen/xtesting/must"
 	"github.com/siderolabs/go-pointer"
 	"github.com/stretchr/testify/assert"
@@ -137,6 +138,9 @@ func TestValidate(t *testing.T) {
 		},
 		MachineConfig: &v1alpha1.MachineConfig{
 			MachineType: "worker",
+			MachineCA: &x509.PEMEncodedCertificateAndKey{
+				Crt: []byte("cert"),
+			},
 		},
 	}
 

diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go
@@ -131,12 +131,21 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt
 		warnings = append(warnings, fmt.Sprintf("use %q instead of %q for machine type", t.String(), c.MachineConfig.MachineType))
 	}
 
+	if c.Machine().Security().IssuingCA() == nil && len(c.Machine().Security().AcceptedCAs()) == 0 {
+		result = multierror.Append(result, errors.New("issuing CA or some accepted CAs are required (.machine.ca, machine.acceptedCAs)"))
+	}
+
 	switch c.Machine().Type() {
 	case machine.TypeInit, machine.TypeControlPlane:
 		warn, err := ValidateCNI(c.Cluster().Network().CNI())
 		warnings = append(warnings, warn...)
 		result = multierror.Append(result, err)
 
+		if c.Machine().Security().IssuingCA() == nil {
+			result = multierror.Append(result, errors.New("issuing CA is required (.machine.ca)"))
+		} else if len(c.Machine().Security().IssuingCA().Key) == 0 {
+			result = multierror.Append(result, errors.New("issuing CA key is required for controlplane nodes (.machine.ca.key)"))
+		}
 	case machine.TypeWorker:
 		for _, d := range c.Machine().Network().Devices() {
 			if d.VIPConfig() != nil {
@@ -150,8 +159,14 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt
 			}
 		}
 
-		if c.Machine().Security().IssuingCA() != nil && len(c.Machine().Security().IssuingCA().Key) > 0 {
-			result = multierror.Append(result, errors.New("issuing Talos API CA key is not allowed on non-controlplane nodes (.machine.ca)"))
+		if c.Machine().Security().IssuingCA() != nil {
+			if len(c.Machine().Security().IssuingCA().Key) > 0 {
+				result = multierror.Append(result, errors.New("issuing Talos API CA key is not allowed on non-controlplane nodes (.machine.ca)"))
+			}
+
+			if len(c.Machine().Security().IssuingCA().Crt) == 0 && len(c.Machine().Security().AcceptedCAs()) == 0 {
+				result = multierror.Append(result, errors.New("trusted CA certificates are required on non-controlplane nodes (.machine.ca.crt, .machine.acceptedCAs)"))
+			}
 		}
 
 		if c.Cluster().IssuingCA() != nil && len(c.Cluster().IssuingCA().Key) > 0 {