Force rotation intermediate and Server SVIDs (spiffe#5431)

* Force rotation of intermediates signed by a compromised authority
* Force rotation of Server SVIDs signed by a compromised authority
* Force rotation of server SVIDs when not using an upstream authority

Signed-off-by: Marcos Yacob <[email protected]>

pkg/agent/agent.go

@@ -17,13 +17,13 @@ import (
 	node_attestor "github.com/spiffe/spire/pkg/agent/attestor/node"
 	workload_attestor "github.com/spiffe/spire/pkg/agent/attestor/workload"
 	"github.com/spiffe/spire/pkg/agent/catalog"
-	"github.com/spiffe/spire/pkg/agent/common/backoff"
 	"github.com/spiffe/spire/pkg/agent/endpoints"
 	"github.com/spiffe/spire/pkg/agent/manager"
 	"github.com/spiffe/spire/pkg/agent/manager/storecache"
 	"github.com/spiffe/spire/pkg/agent/plugin/nodeattestor"
 	"github.com/spiffe/spire/pkg/agent/storage"
 	"github.com/spiffe/spire/pkg/agent/svid/store"
+	"github.com/spiffe/spire/pkg/common/backoff"
 	"github.com/spiffe/spire/pkg/common/diskutil"
 	"github.com/spiffe/spire/pkg/common/health"
 	"github.com/spiffe/spire/pkg/common/nodeutil"

pkg/agent/manager/cache/lru_cache.go

@@ -11,7 +11,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spiffe/go-spiffe/v2/bundle/spiffebundle"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
-	"github.com/spiffe/spire/pkg/agent/common/backoff"
+	"github.com/spiffe/spire/pkg/common/backoff"
 	"github.com/spiffe/spire/pkg/common/telemetry"
 	agentmetrics "github.com/spiffe/spire/pkg/common/telemetry/agent"
 	"github.com/spiffe/spire/proto/spire/common"

pkg/agent/manager/manager.go

@@ -13,11 +13,11 @@ import (
 	"github.com/spiffe/go-spiffe/v2/bundle/spiffebundle"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/agent/client"
-	"github.com/spiffe/spire/pkg/agent/common/backoff"
 	"github.com/spiffe/spire/pkg/agent/manager/cache"
 	"github.com/spiffe/spire/pkg/agent/manager/storecache"
 	"github.com/spiffe/spire/pkg/agent/storage"
 	"github.com/spiffe/spire/pkg/agent/svid"
+	"github.com/spiffe/spire/pkg/common/backoff"
 	"github.com/spiffe/spire/pkg/common/nodeutil"
 	"github.com/spiffe/spire/pkg/common/rotationutil"
 	"github.com/spiffe/spire/pkg/common/telemetry"

pkg/agent/svid/rotator.go

@@ -14,8 +14,8 @@ import (
 	agentv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/agent/v1"
 	node_attestor "github.com/spiffe/spire/pkg/agent/attestor/node"
 	"github.com/spiffe/spire/pkg/agent/client"
-	"github.com/spiffe/spire/pkg/agent/common/backoff"
 	"github.com/spiffe/spire/pkg/agent/plugin/keymanager"
+	"github.com/spiffe/spire/pkg/common/backoff"
 	"github.com/spiffe/spire/pkg/common/nodeutil"
 	"github.com/spiffe/spire/pkg/common/rotationutil"
 	"github.com/spiffe/spire/pkg/common/telemetry"

pkg/agent/svid/rotator_config.go

@@ -11,10 +11,10 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/agent/client"
-	"github.com/spiffe/spire/pkg/agent/common/backoff"
 	"github.com/spiffe/spire/pkg/agent/manager/cache"
 	"github.com/spiffe/spire/pkg/agent/plugin/keymanager"
 	"github.com/spiffe/spire/pkg/agent/plugin/nodeattestor"
+	"github.com/spiffe/spire/pkg/common/backoff"
 	"github.com/spiffe/spire/pkg/common/rotationutil"
 	"github.com/spiffe/spire/pkg/common/telemetry"
 )

pkg/server/api/localauthority/v1/service.go

@@ -33,6 +33,7 @@ type CAManager interface {
 	RotateX509CA(ctx context.Context)
 
 	IsUpstreamAuthority() bool
+	NotifyTaintedX509Authority(ctx context.Context, authorityID string) error
 }
 
 // RegisterService registers the service on the gRPC server.
@@ -367,6 +368,10 @@ func (s *Service) TaintX509Authority(ctx context.Context, req *localauthorityv1.
 		AuthorityId: nextSlot.AuthorityID(),
 	}
 
+	if err := s.ca.NotifyTaintedX509Authority(ctx, nextSlot.AuthorityID()); err != nil {
+		return nil, api.MakeErr(log, codes.Internal, "failed to notify tainted authority", err)
+	}
+
 	rpccontext.AuditRPC(ctx)
 	log.Info("X.509 authority tainted successfully")
 

pkg/server/api/localauthority/v1/service_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/spiffe/spire/test/testca"
 	"github.com/spiffe/spire/test/testkey"
 	testutil "github.com/spiffe/spire/test/util"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -1324,6 +1325,7 @@ func TestTaintX509Authority(t *testing.T) {
 		keyToTaint          string
 		customRootCAs       []*common.Certificate
 		isUpstreamAuthority bool
+		notifyTaintedErr    error
 
 		expectLogs []spiretest.LogEntry
 		expectCode codes.Code
@@ -1537,6 +1539,36 @@ func TestTaintX509Authority(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:             "fail to notify tainted authority",
+			currentSlot:      createSlot(journal.Status_ACTIVE, currentAuthorityID, currentKey.Public(), notAfterCurrent),
+			nextSlot:         createSlot(journal.Status_OLD, nextAuthorityID, nextKey.Public(), notAfterNext),
+			keyToTaint:       nextAuthorityID,
+			notifyTaintedErr: errors.New("oh no"),
+			expectCode:       codes.Internal,
+			expectMsg:        "failed to notify tainted authority: oh no",
+			expectLogs: []spiretest.LogEntry{
+				{
+					Level:   logrus.ErrorLevel,
+					Message: "Failed to notify tainted authority",
+					Data: logrus.Fields{
+						telemetry.LocalAuthorityID: nextAuthorityID,
+						logrus.ErrorKey:            "oh no",
+					},
+				},
+				{
+					Level:   logrus.InfoLevel,
+					Message: "API accessed",
+					Data: logrus.Fields{
+						telemetry.Status:           "error",
+						telemetry.StatusCode:       "Internal",
+						telemetry.StatusMessage:    "failed to notify tainted authority: oh no",
+						telemetry.Type:             "audit",
+						telemetry.LocalAuthorityID: nextAuthorityID,
+					},
+				},
+			},
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			test := setupServiceTest(t)
@@ -1545,6 +1577,7 @@ func TestTaintX509Authority(t *testing.T) {
 			test.ca.currentX509CASlot = tt.currentSlot
 			test.ca.nextX509CASlot = tt.nextSlot
 			test.ca.isUpstreamAuthority = tt.isUpstreamAuthority
+			test.ca.notifyTaintedExpectErr = tt.notifyTaintedErr
 
 			rootCAs := defaultRootCAs
 			if tt.customRootCAs != nil {
@@ -1564,6 +1597,10 @@ func TestTaintX509Authority(t *testing.T) {
 			spiretest.AssertGRPCStatusHasPrefix(t, err, tt.expectCode, tt.expectMsg)
 			spiretest.AssertProtoEqual(t, tt.expectResp, resp)
 			spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectLogs)
+			// Validate notification is received on success test cases
+			if tt.expectMsg == "" {
+				assert.Equal(t, tt.keyToTaint, test.ca.notifyTaintedAuthorityID)
+			}
 		})
 	}
 }
@@ -2445,6 +2482,17 @@ type fakeCAManager struct {
 
 	prepareX509CAErr    error
 	isUpstreamAuthority bool
+
+	notifyTaintedExpectErr   error
+	notifyTaintedAuthorityID string
+}
+
+func (m *fakeCAManager) NotifyTaintedX509Authority(ctx context.Context, authorityID string) error {
+	if m.notifyTaintedExpectErr != nil {
+		return m.notifyTaintedExpectErr
+	}
+	m.notifyTaintedAuthorityID = authorityID
+	return nil
 }
 
 func (m *fakeCAManager) IsUpstreamAuthority() bool {

pkg/server/ca/ca.go

@@ -36,6 +36,7 @@ type ServerCA interface {
 	SignAgentX509SVID(ctx context.Context, params AgentX509SVIDParams) ([]*x509.Certificate, error)
 	SignWorkloadX509SVID(ctx context.Context, params WorkloadX509SVIDParams) ([]*x509.Certificate, error)
 	SignWorkloadJWTSVID(ctx context.Context, params WorkloadJWTSVIDParams) (string, error)
+	TaintedAuthorities() <-chan []*x509.Certificate
 }
 
 // DownstreamX509CAParams are parameters relevant to downstream X.509 CA creation
@@ -133,10 +134,11 @@ type Config struct {
 type CA struct {
 	c Config
 
-	mu          sync.RWMutex
-	x509CA      *X509CA
-	x509CAChain []*x509.Certificate
-	jwtKey      *JWTKey
+	mu                   sync.RWMutex
+	x509CA               *X509CA
+	x509CAChain          []*x509.Certificate
+	jwtKey               *JWTKey
+	taintedAuthoritiesCh chan []*x509.Certificate
 }
 
 func NewCA(config Config) *CA {
@@ -146,6 +148,9 @@ func NewCA(config Config) *CA {
 
 	ca := &CA{
 		c: config,
+
+		// Notify caller about any tainted authority
+		taintedAuthoritiesCh: make(chan []*x509.Certificate, 1),
 	}
 
 	_ = config.HealthChecker.AddCheck("server.ca", &caHealth{
@@ -188,6 +193,17 @@ func (ca *CA) SetJWTKey(jwtKey *JWTKey) {
 	ca.jwtKey = jwtKey
 }
 
+func (ca *CA) NotifyTaintedX509Authorities(taintedAuthorities []*x509.Certificate) {
+	select {
+	case ca.taintedAuthoritiesCh <- taintedAuthorities:
+	default:
+	}
+}
+
+func (ca *CA) TaintedAuthorities() <-chan []*x509.Certificate {
+	return ca.taintedAuthoritiesCh
+}
+
 func (ca *CA) SignDownstreamX509CA(ctx context.Context, params DownstreamX509CAParams) ([]*x509.Certificate, error) {
 	x509CA, caChain, err := ca.getX509CA()
 	if err != nil {

pkg/server/ca/ca_test.go

@@ -436,6 +436,23 @@ func (s *CATestSuite) TestNoJWTKeySet() {
 	s.Require().EqualError(err, "JWT key is not available for signing")
 }
 
+func (s *CATestSuite) TestTaintedAuthoritiesArePropagated() {
+	authorities := []*x509.Certificate{
+		{Raw: []byte("foh")},
+		{Raw: []byte("bar")},
+	}
+	s.ca.NotifyTaintedX509Authorities(authorities)
+	ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	defer cancel()
+
+	select {
+	case got := <-s.ca.TaintedAuthorities():
+		s.Require().Equal(authorities, got)
+	case <-ctx.Done():
+		s.Fail("no notification received")
+	}
+}
+
 func (s *CATestSuite) TestSignWorkloadJWTSVIDUsesDefaultTTLIfTTLUnspecified() {
 	token, err := s.ca.SignWorkloadJWTSVID(ctx, s.createJWTSVIDParams(trustDomainExample, 0))
 	s.Require().NoError(err)