coordinator: support insecure manifests behind opt-in

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: msanft

coordinator/internal/stateguard/stateguard.go

@@ -52,6 +52,18 @@ var (
 	// ErrConcurrentUpdate is returned by state-modifying operations if the input oldState is not
 	// the current state. This usually happens when a concurrent operation succeeded.
 	ErrConcurrentUpdate = errors.New("coordinator state was updated concurrently")
+
+	// ErrInsecureNotAllowed is returned when a manifest contains insecure platforms but the
+	// coordinator was not started with the allow-insecure flag.
+	ErrInsecureNotAllowed = errors.New("manifest contains insecure platforms, but the coordinator is not configured to allow them")
+
+	// ErrMixedManifestNotAllowed is returned when a manifest mixes secure and insecure platforms.
+	// Manifests must be either all-secure or all-insecure.
+	ErrMixedManifestNotAllowed = errors.New("manifest must not mix secure and insecure platforms")
+
+	// ErrInsecureToSecureTransition is returned when a Coordinator with an insecure manifest tries
+	// to transition back to a secure manifest.
+	ErrInsecureToSecureTransition = errors.New("coordinator must not transition from an insecure manifest to a secure manifest")
 )
 
 // Guard manages the manifest state of Contrast.
@@ -65,6 +77,10 @@ type Guard struct {
 	logger  *slog.Logger
 	metrics metrics
 
+	// allowInsecure controls whether manifests with insecure platforms are accepted.
+	// It defaults to false and can be enabled via MakeInsecure.
+	allowInsecure bool
+
 	clock clock.Clock
 }
 
@@ -73,6 +89,9 @@ type metrics struct {
 }
 
 // New creates a new state Guard instance.
+//
+// By default, the Guard rejects manifests that contain insecure platforms. Call MakeInsecure to
+// allow them.
 func New(hist *history.History, reg *prometheus.Registry, log *slog.Logger) *Guard {
 	manifestGeneration := promauto.With(reg).NewGauge(prometheus.GaugeOpts{
 		Subsystem: "contrast_coordinator",
@@ -91,6 +110,14 @@ func New(hist *history.History, reg *prometheus.Registry, log *slog.Logger) *Gua
 	}
 }
 
+// MakeInsecure configures the Guard to accept manifests that contain insecure platforms.
+//
+// This voids the security guarantees of the deployment and must only be used for testing or
+// benchmarking. It should be called before the Guard starts serving requests.
+func (g *Guard) MakeInsecure() {
+	g.allowInsecure = true
+}
+
 // WatchHistory monitors the history for manifest updates and sets the state stale if necessary.
 //
 // This function blocks and keeps watching until the context expires.
@@ -188,6 +215,9 @@ func (g *Guard) ResetState(ctx context.Context, oldState *State, authorizer Secr
 	if err := json.Unmarshal(manifestBytes, mnfst); err != nil {
 		return nil, fmt.Errorf("parsing manifest: %w", err)
 	}
+	if err := g.validateManifestSecurity(mnfst); err != nil {
+		return nil, err
+	}
 
 	se, meshCAKey, err := authorizer.AuthorizeByManifest(ctx, mnfst)
 	if err != nil {
@@ -271,6 +301,12 @@ func (g *Guard) UpdateState(_ context.Context, oldState *State, se *seedengine.S
 	if err := json.Unmarshal(manifestBytes, &mnfst); err != nil {
 		return nil, fmt.Errorf("unmarshaling manifest: %w", err)
 	}
+	if err := g.validateManifestSecurity(&mnfst); err != nil {
+		return nil, err
+	}
+	if oldState != nil && oldState.Manifest().HasInsecurePlatforms() && !mnfst.HasInsecurePlatforms() {
+		return nil, ErrInsecureToSecureTransition
+	}
 	policyMap := make(map[[history.HashSize]byte][]byte)
 	for _, policy := range policies {
 		policyHash, err := g.hist.SetPolicy(policy)
@@ -351,6 +387,19 @@ func (g *Guard) UpdateState(_ context.Context, oldState *State, se *seedengine.S
 	return nextState, nil
 }
 
+func (g *Guard) validateManifestSecurity(mnfst *manifest.Manifest) error {
+	if !mnfst.HasInsecurePlatforms() {
+		return nil
+	}
+	if !g.allowInsecure {
+		return ErrInsecureNotAllowed
+	}
+	if mnfst.HasSecurePlatforms() {
+		return ErrMixedManifestNotAllowed
+	}
+	return nil
+}
+
 // GetHistory returns a list of manifests, the current manifest being last, and the policies
 // referenced in at least one of the manifests.
 func (g *Guard) GetHistory(ctx context.Context) ([][]byte, map[manifest.HexString][]byte, error) {

coordinator/internal/stateguard/stateguard_test.go

@@ -192,6 +192,125 @@ func TestResetState(t *testing.T) {
 	require.ErrorIs(err, assert.AnError)
 }
 
+func TestResetStateInsecureRequiresOptIn(t *testing.T) {
+	ctx := t.Context()
+	require := require.New(t)
+
+	store := aferostore.New(&afero.Afero{Fs: afero.NewMemMapFs()})
+	hist := history.NewWithStore(slog.Default(), store)
+	se := newSeedEngine(t)
+	_, manifestBytes, policies := newInsecureManifest(t)
+
+	originalGuard := New(hist, prometheus.NewRegistry(), slog.Default())
+	originalGuard.MakeInsecure()
+	state, err := originalGuard.UpdateState(ctx, nil, se, manifestBytes, policies)
+	require.NoError(err)
+	require.NotNil(state)
+
+	restartedGuard := New(hist, prometheus.NewRegistry(), slog.Default())
+	state, err = restartedGuard.ResetState(ctx, nil, &stubAuthorizer{err: assert.AnError})
+	require.ErrorIs(err, ErrInsecureNotAllowed)
+	require.Nil(state)
+
+	restartedGuard.MakeInsecure()
+	authz := &stubAuthorizer{
+		se: se,
+		pk: testkeys.ECDSA(t),
+	}
+	state, err = restartedGuard.ResetState(ctx, nil, authz)
+	require.NoError(err)
+	require.NotNil(state)
+}
+
+func TestUpdateStateInsecure(t *testing.T) {
+	ctx := t.Context()
+
+	_, insecureManifestBytes, policies := newInsecureManifest(t)
+	se := newSeedEngine(t)
+
+	t.Run("rejected when allowInsecure is false", func(t *testing.T) {
+		require := require.New(t)
+
+		store := aferostore.New(&afero.Afero{Fs: afero.NewMemMapFs()})
+		hist := history.NewWithStore(slog.Default(), store)
+		g := New(hist, prometheus.NewRegistry(), slog.Default())
+
+		state, err := g.UpdateState(ctx, nil, se, insecureManifestBytes, policies)
+		require.ErrorIs(err, ErrInsecureNotAllowed)
+		require.Nil(state)
+	})
+
+	t.Run("accepted when allowInsecure is true", func(t *testing.T) {
+		require := require.New(t)
+
+		store := aferostore.New(&afero.Afero{Fs: afero.NewMemMapFs()})
+		hist := history.NewWithStore(slog.Default(), store)
+		g := New(hist, prometheus.NewRegistry(), slog.Default())
+		g.MakeInsecure()
+
+		state, err := g.UpdateState(ctx, nil, se, insecureManifestBytes, policies)
+		require.NoError(err)
+		require.NotNil(state)
+	})
+
+	t.Run("mixed manifest rejected even when allowInsecure is true", func(t *testing.T) {
+		require := require.New(t)
+
+		_, mixedManifestBytes, mixedPolicies := newMixedManifest(t)
+
+		store := aferostore.New(&afero.Afero{Fs: afero.NewMemMapFs()})
+		hist := history.NewWithStore(slog.Default(), store)
+		g := New(hist, prometheus.NewRegistry(), slog.Default())
+		g.MakeInsecure()
+
+		state, err := g.UpdateState(ctx, nil, se, mixedManifestBytes, mixedPolicies)
+		require.ErrorIs(err, ErrMixedManifestNotAllowed)
+		require.Nil(state)
+	})
+}
+
+func TestUpdateStateSecurityTransitions(t *testing.T) {
+	type manifestFixture struct {
+		name  string
+		build func(*testing.T) (*manifest.Manifest, []byte, [][]byte)
+	}
+	fixtures := []manifestFixture{
+		{name: "secure", build: newManifest},
+		{name: "insecure", build: newInsecureManifest},
+	}
+
+	for _, from := range fixtures {
+		for _, to := range fixtures {
+			t.Run(from.name+" to "+to.name, func(t *testing.T) {
+				require := require.New(t)
+				g, _ := newTestGuard(t)
+				g.MakeInsecure()
+				se := newSeedEngine(t)
+
+				_, fromManifestBytes, fromPolicies := from.build(t)
+				oldState, err := g.UpdateState(t.Context(), nil, se, fromManifestBytes, fromPolicies)
+				require.NoError(err)
+				require.NotNil(oldState)
+
+				_, toManifestBytes, toPolicies := to.build(t)
+				nextState, err := g.UpdateState(t.Context(), oldState, se, toManifestBytes, toPolicies)
+				if from.name == "insecure" && to.name == "secure" {
+					require.ErrorIs(err, ErrInsecureToSecureTransition)
+					require.Nil(nextState)
+
+					currentState, getErr := g.GetState(t.Context())
+					require.NoError(getErr)
+					require.Same(oldState, currentState)
+					return
+				}
+
+				require.NoError(err)
+				require.NotNil(nextState)
+			})
+		}
+	}
+}
+
 func TestConcurrentUpdateState(t *testing.T) {
 	ctx := t.Context()
 	assert := assert.New(t)
@@ -543,6 +662,51 @@ func newManifest(t *testing.T) (*manifest.Manifest, []byte, [][]byte) {
 	return mnfst, mnfstBytes, [][]byte{policy}
 }
 
+func newInsecureManifest(t *testing.T) (*manifest.Manifest, []byte, [][]byte) {
+	t.Helper()
+	policy := []byte("=== SOME REGO HERE ===")
+	policyHash := sha256.Sum256(policy)
+	policyHashHex := manifest.NewHexString(policyHash[:])
+
+	mnfst := &manifest.Manifest{}
+	mnfst.Policies = map[manifest.HexString]manifest.PolicyEntry{
+		policyHashHex: {
+			SANs:             []string{"test"},
+			WorkloadSecretID: "test2",
+			Role:             manifest.RoleCoordinator,
+		},
+	}
+	mnfst.ReferenceValues.SNP = []manifest.SNPReferenceValues{
+		{Platform: "Metal-QEMU-Insecure"},
+	}
+	mnfstBytes, err := json.Marshal(mnfst)
+	require.NoError(t, err)
+	return mnfst, mnfstBytes, [][]byte{policy}
+}
+
+func newMixedManifest(t *testing.T) (*manifest.Manifest, []byte, [][]byte) {
+	t.Helper()
+	policy := []byte("=== SOME REGO HERE ===")
+	policyHash := sha256.Sum256(policy)
+	policyHashHex := manifest.NewHexString(policyHash[:])
+
+	mnfst := &manifest.Manifest{}
+	mnfst.Policies = map[manifest.HexString]manifest.PolicyEntry{
+		policyHashHex: {
+			SANs:             []string{"test"},
+			WorkloadSecretID: "test2",
+			Role:             manifest.RoleCoordinator,
+		},
+	}
+	mnfst.ReferenceValues.SNP = []manifest.SNPReferenceValues{
+		{Platform: "Metal-QEMU-Insecure"},
+		{Platform: "Metal-QEMU-SNP"},
+	}
+	mnfstBytes, err := json.Marshal(mnfst)
+	require.NoError(t, err)
+	return mnfst, mnfstBytes, [][]byte{policy}
+}
+
 func newSeedEngine(t *testing.T) *seedengine.SeedEngine {
 	t.Helper()
 	data := make([]byte, 32)

coordinator/internal/userapi/userapi.go

@@ -151,8 +151,15 @@ func (s *Server) SetManifest(ctx context.Context, req *userapi.SetManifestReques
 	state, err := s.guard.UpdateState(ctx, oldState, se, req.GetManifest(), req.GetPolicies())
 	if err != nil {
 		code := codes.Internal
-		if errors.Is(err, stateguard.ErrConcurrentUpdate) {
+		switch {
+		case errors.Is(err, stateguard.ErrConcurrentUpdate):
 			code = codes.FailedPrecondition
+		case errors.Is(err, stateguard.ErrInsecureNotAllowed):
+			code = codes.InvalidArgument
+		case errors.Is(err, stateguard.ErrMixedManifestNotAllowed):
+			code = codes.InvalidArgument
+		case errors.Is(err, stateguard.ErrInsecureToSecureTransition):
+			code = codes.InvalidArgument
 		}
 		return nil, status.Errorf(code, "updating Coordinator state: %v", err)
 	}

coordinator/internal/userapi/userapi_test.go

@@ -232,6 +232,48 @@ func TestSetManifest(t *testing.T) {
 		require.Equal(codes.InvalidArgument, status.Code(err))
 	})
 
+	t.Run("insecure manifest rejected", func(t *testing.T) {
+		require := require.New(t)
+
+		// Default coordinator does not allow insecure manifests.
+		coordinator := newCoordinator()
+		m := newInsecureManifest(t)
+		manifestBytes, err := json.Marshal(m)
+		require.NoError(err)
+		req := &userapi.SetManifestRequest{Manifest: manifestBytes}
+		_, err = coordinator.SetManifest(t.Context(), req)
+		require.Error(err)
+		require.Equal(codes.InvalidArgument, status.Code(err))
+		require.ErrorContains(err, "insecure")
+	})
+
+	t.Run("insecure manifest accepted when allowed", func(t *testing.T) {
+		require := require.New(t)
+
+		coordinator := newCoordinatorAllowInsecure()
+		m := newInsecureManifest(t)
+		manifestBytes, err := json.Marshal(m)
+		require.NoError(err)
+		req := &userapi.SetManifestRequest{Manifest: manifestBytes}
+		resp, err := coordinator.SetManifest(t.Context(), req)
+		require.NoError(err)
+		require.NotNil(resp)
+	})
+
+	t.Run("mixed manifest rejected even when insecure allowed", func(t *testing.T) {
+		require := require.New(t)
+
+		coordinator := newCoordinatorAllowInsecure()
+		m := newMixedManifest(t)
+		manifestBytes, err := json.Marshal(m)
+		require.NoError(err)
+		req := &userapi.SetManifestRequest{Manifest: manifestBytes}
+		_, err = coordinator.SetManifest(t.Context(), req)
+		require.Error(err)
+		require.Equal(codes.InvalidArgument, status.Code(err))
+		require.ErrorContains(err, "mix")
+	})
+
 	t.Run("atomic manifest update", func(t *testing.T) {
 		require := require.New(t)
 
@@ -857,6 +899,35 @@ func newCoordinatorWithRegistry(reg *prometheus.Registry) *Server {
 	return New(logger, auth, &stubDiscovery{})
 }
 
+func newCoordinatorAllowInsecure() *Server {
+	logger := slog.Default()
+	fs := afero.NewMemMapFs()
+	store := aferostore.New(&afero.Afero{Fs: fs})
+	hist := history.NewWithStore(slog.Default(), store)
+	auth := stateguard.New(hist, prometheus.NewRegistry(), logger)
+	auth.MakeInsecure()
+	return New(logger, auth, &stubDiscovery{})
+}
+
+func newInsecureManifest(t *testing.T) *manifest.Manifest {
+	t.Helper()
+	mnfst := &manifest.Manifest{}
+	mnfst.ReferenceValues.SNP = []manifest.SNPReferenceValues{
+		{Platform: "Metal-QEMU-Insecure"},
+	}
+	return mnfst
+}
+
+func newMixedManifest(t *testing.T) *manifest.Manifest {
+	t.Helper()
+	mnfst := &manifest.Manifest{}
+	mnfst.ReferenceValues.SNP = []manifest.SNPReferenceValues{
+		{Platform: "Metal-QEMU-Insecure"},
+		{Platform: "Metal-QEMU-SNP"},
+	}
+	return mnfst
+}
+
 func newCoordinatorWithWatcher(t *testing.T, hist *history.History) *Server {
 	t.Helper()
 	logger := slog.Default()