Skip to content

Commit

Permalink
fix: require accepted CAs on worker nodes
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
smira committed May 23, 2024
1 parent 23c1c45 commit 2e64e9e
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 15 deletions.
2 changes: 1 addition & 1 deletion internal/app/machined/pkg/controllers/secrets/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 9 additions & 1 deletion internal/integration/api/apply-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package api

import (
"context"
"os"
"sort"
"testing"
"time"
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions pkg/grpc/gen/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -37,7 +37,7 @@ func NewRemoteGenerator(token string, endpoints []string, ca *x509.PEMEncodedCer

g = &RemoteGenerator{}

conn, err := basic.NewConnection(fmt.Sprintf("%s:///%s", resolver.RoundRobinResolverScheme, strings.Join(endpoints, ",")), basic.NewTokenCredentials(token), ca)
conn, err := basic.NewConnection(fmt.Sprintf("%s:///%s", resolver.RoundRobinResolverScheme, strings.Join(endpoints, ",")), basic.NewTokenCredentials(token), acceptedCAs)
if err != nil {
return nil, err
}
Expand Down
20 changes: 13 additions & 7 deletions pkg/grpc/middleware/auth/basic/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)),
Expand Down
4 changes: 4 additions & 0 deletions pkg/machinery/config/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -137,6 +138,9 @@ func TestValidate(t *testing.T) {
},
MachineConfig: &v1alpha1.MachineConfig{
MachineType: "worker",
MachineCA: &x509.PEMEncodedCertificateAndKey{
Crt: []byte("cert"),
},
},
}

Expand Down
19 changes: 17 additions & 2 deletions pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 2e64e9e

Please sign in to comment.