Prevent blank revisions from being read from the backend

api/types/installer.go

@@ -22,7 +22,7 @@ import (
 	"github.com/gravitational/trace"
 )
 
-// Installer is an installer script rseource
+// Installer is an installer script resource
 type Installer interface {
 	Resource
 

lib/auth/auth_test.go

@@ -34,6 +34,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/uuid"
 	"github.com/gravitational/license"
 	"github.com/gravitational/trace"
@@ -233,7 +234,7 @@ func TestSessions(t *testing.T) {
 	out, err := s.a.GetWebSessionInfo(ctx, user, ws.GetName())
 	require.NoError(t, err)
 	ws.SetPriv(nil)
-	require.Equal(t, ws, out)
+	require.Empty(t, cmp.Diff(ws, out, cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision")))
 
 	err = s.a.WebSessions().Delete(ctx, types.DeleteWebSessionRequest{
 		User:      user,

lib/backend/backend.go

@@ -433,6 +433,12 @@ func CreateRevision() string {
 	return uuid.NewString()
 }
 
+// BlankRevision is a placeholder revision to be used by backends when
+// the revision of the item in the backend is empty. This can happen
+// to any existing resources that were last written before support for
+// revisions was added.
+var BlankRevision = uuid.Nil.String()
+
 // NewLease creates a lease for the provided [Item].
 func NewLease(item Item) *Lease {
 	return &Lease{

lib/backend/dynamo/dynamodbbk.go

@@ -412,6 +412,9 @@ func (b *Backend) GetRange(ctx context.Context, startKey []byte, endKey []byte,
 		if r.Expires != nil {
 			values[i].Expires = time.Unix(*r.Expires, 0).UTC()
 		}
+		if values[i].Revision == "" {
+			values[i].Revision = backend.BlankRevision
+		}
 	}
 	return &backend.GetResult{Items: values}, nil
 }
@@ -503,6 +506,9 @@ func (b *Backend) Get(ctx context.Context, key []byte) (*backend.Item, error) {
 	if r.Expires != nil {
 		item.Expires = time.Unix(*r.Expires, 0)
 	}
+	if item.Revision == "" {
+		item.Revision = backend.BlankRevision
+	}
 	return item, nil
 }
 
@@ -572,6 +578,14 @@ func (b *Backend) Delete(ctx context.Context, key []byte) error {
 // ConditionalUpdate updates the matching item in Dynamo if the provided revision matches
 // the revision of the item in Dynamo.
 func (b *Backend) ConditionalUpdate(ctx context.Context, item backend.Item) (*backend.Lease, error) {
+	if item.Revision == "" {
+		return nil, trace.Wrap(backend.ErrIncorrectRevision)
+	}
+
+	if item.Revision == backend.BlankRevision {
+		item.Revision = ""
+	}
+
 	rev, err := b.create(ctx, item, modeConditionalUpdate)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -584,13 +598,20 @@ func (b *Backend) ConditionalUpdate(ctx context.Context, item backend.Item) (*ba
 // ConditionalDelete deletes item by key if the provided revision matches
 // the revision of the item in Dynamo.
 func (b *Backend) ConditionalDelete(ctx context.Context, key []byte, rev string) error {
+	if rev == "" {
+		return trace.Wrap(backend.ErrIncorrectRevision)
+	}
+
 	av, err := dynamodbattribute.MarshalMap(keyLookup{
 		HashKey:  hashKey,
 		FullPath: prependPrefix(key),
 	})
 	if err != nil {
 		return trace.Wrap(err)
 	}
+	if rev == backend.BlankRevision {
+		rev = ""
+	}
 	input := dynamodb.DeleteItemInput{Key: av, TableName: aws.String(b.TableName)}
 	input.SetConditionExpression("Revision = :rev")
 	input.SetExpressionAttributeValues(map[string]*dynamodb.AttributeValue{":rev": {S: aws.String(rev)}})

lib/backend/dynamo/shards.go

@@ -318,10 +318,11 @@ func toEvent(rec *dynamodbstreams.Record) (*backend.Event, error) {
 		return &backend.Event{
 			Type: op,
 			Item: backend.Item{
-				Key:     trimPrefix(r.FullPath),
-				Value:   r.Value,
-				Expires: expires,
-				ID:      r.ID,
+				Key:      trimPrefix(r.FullPath),
+				Value:    r.Value,
+				Expires:  expires,
+				ID:       r.ID,
+				Revision: r.Revision,
 			},
 		}, nil
 	case types.OpDelete:

lib/backend/lite/lite.go

@@ -588,6 +588,10 @@ func (l *Backend) Get(ctx context.Context, key []byte) (*backend.Item, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+
+	if item.Revision == "" {
+		item.Revision = backend.BlankRevision
+	}
 	return &item, nil
 }
 
@@ -646,6 +650,9 @@ func (l *Backend) GetRange(ctx context.Context, startKey []byte, endKey []byte,
 				return trace.Wrap(err)
 			}
 			i.Expires = expires.Time
+			if i.Revision == "" {
+				i.Revision = backend.BlankRevision
+			}
 			result.Items = append(result.Items, i)
 		}
 		return nil
@@ -791,8 +798,12 @@ func (l *Backend) DeleteRange(ctx context.Context, startKey, endKey []byte) erro
 }
 
 func (l *Backend) ConditionalUpdate(ctx context.Context, i backend.Item) (*backend.Lease, error) {
-	if i.Key == nil {
-		return nil, trace.BadParameter("missing parameter key")
+	if i.Key == nil || i.Revision == "" {
+		return nil, trace.Wrap(backend.ErrIncorrectRevision)
+	}
+
+	if i.Revision == backend.BlankRevision {
+		i.Revision = ""
 	}
 
 	rev := backend.CreateRevision()
@@ -822,7 +833,7 @@ func (l *Backend) ConditionalUpdate(ctx context.Context, i backend.Item) (*backe
 			}
 			defer stmt.Close()
 
-			if _, err := stmt.ExecContext(ctx, types.OpPut, now, string(i.Key), id(now), expires(i.Expires), i.Value, i.Revision); err != nil {
+			if _, err := stmt.ExecContext(ctx, types.OpPut, now, string(i.Key), id(now), expires(i.Expires), i.Value, rev); err != nil {
 				return trace.Wrap(err)
 			}
 		}
@@ -837,8 +848,12 @@ func (l *Backend) ConditionalUpdate(ctx context.Context, i backend.Item) (*backe
 }
 
 func (l *Backend) ConditionalDelete(ctx context.Context, key []byte, revision string) error {
-	if len(key) == 0 {
-		return trace.BadParameter("missing parameter key")
+	if len(key) == 0 || revision == "" {
+		return trace.Wrap(backend.ErrIncorrectRevision)
+	}
+
+	if revision == backend.BlankRevision {
+		revision = ""
 	}
 
 	return l.inTransaction(ctx, func(tx *sql.Tx) error {

lib/backend/memory/memory.go

@@ -386,9 +386,10 @@ func (m *Memory) CompareAndSwap(ctx context.Context, expected backend.Item, repl
 }
 
 func (m *Memory) ConditionalDelete(ctx context.Context, key []byte, rev string) error {
-	if len(key) == 0 {
-		return trace.BadParameter("missing parameter key")
+	if len(key) == 0 || (rev == "" && !m.Mirror) {
+		return trace.Wrap(backend.ErrIncorrectRevision)
 	}
+
 	m.Lock()
 	defer m.Unlock()
 	m.removeExpired()
@@ -412,9 +413,10 @@ func (m *Memory) ConditionalDelete(ctx context.Context, key []byte, rev string)
 }
 
 func (m *Memory) ConditionalUpdate(ctx context.Context, i backend.Item) (*backend.Lease, error) {
-	if len(i.Key) == 0 {
-		return nil, trace.BadParameter("missing parameter key")
+	if len(i.Key) == 0 || (i.Revision == "" && !m.Mirror) {
+		return nil, trace.Wrap(backend.ErrIncorrectRevision)
 	}
+
 	m.Lock()
 	defer m.Unlock()
 	m.removeExpired()

lib/services/discoveryconfig.go

@@ -64,6 +64,7 @@ func MarshalDiscoveryConfig(discoveryConfig *discoveryconfig.DiscoveryConfig, op
 	if !cfg.PreserveResourceID {
 		copy := *discoveryConfig
 		copy.SetResourceID(0)
+		copy.SetRevision("")
 		discoveryConfig = &copy
 	}
 	return utils.FastMarshal(discoveryConfig)
@@ -88,6 +89,9 @@ func UnmarshalDiscoveryConfig(data []byte, opts ...MarshalOption) (*discoverycon
 	if cfg.ID != 0 {
 		discoveryConfig.SetResourceID(cfg.ID)
 	}
+	if cfg.Revision != "" {
+		discoveryConfig.SetRevision(cfg.Revision)
+	}
 	if !cfg.Expires.IsZero() {
 		discoveryConfig.SetExpiry(cfg.Expires)
 	}

lib/services/local/access.go

@@ -104,16 +104,18 @@ func (s *AccessService) UpsertRole(ctx context.Context, role types.Role) error {
 		return trace.Wrap(err)
 	}
 
+	rev := role.GetRevision()
 	value, err := services.MarshalRole(role)
 	if err != nil {
 		return trace.Wrap(err)
 	}
 
 	item := backend.Item{
-		Key:     backend.Key(rolesPrefix, role.GetName(), paramsPrefix),
-		Value:   value,
-		Expires: role.Expiry(),
-		ID:      role.GetResourceID(),
+		Key:      backend.Key(rolesPrefix, role.GetName(), paramsPrefix),
+		Value:    value,
+		Expires:  role.Expiry(),
+		ID:       role.GetResourceID(),
+		Revision: rev,
 	}
 
 	_, err = s.Put(ctx, item)
@@ -203,15 +205,17 @@ func (s *AccessService) GetLocks(ctx context.Context, inForceOnly bool, targets
 
 // UpsertLock upserts a lock.
 func (s *AccessService) UpsertLock(ctx context.Context, lock types.Lock) error {
+	rev := lock.GetRevision()
 	value, err := services.MarshalLock(lock)
 	if err != nil {
 		return trace.Wrap(err)
 	}
 	item := backend.Item{
-		Key:     backend.Key(locksPrefix, lock.GetName()),
-		Value:   value,
-		Expires: lock.Expiry(),
-		ID:      lock.GetResourceID(),
+		Key:      backend.Key(locksPrefix, lock.GetName()),
+		Value:    value,
+		Expires:  lock.Expiry(),
+		ID:       lock.GetResourceID(),
+		Revision: rev,
 	}
 
 	if _, err = s.Put(ctx, item); err != nil {
@@ -260,15 +264,17 @@ func (s *AccessService) ReplaceRemoteLocks(ctx context.Context, clusterName stri
 			if !strings.HasPrefix(lock.GetName(), clusterName) {
 				lock.SetName(clusterName + "/" + lock.GetName())
 			}
+			rev := lock.GetRevision()
 			value, err := services.MarshalLock(lock)
 			if err != nil {
 				return trace.Wrap(err)
 			}
 			item := backend.Item{
-				Key:     backend.Key(locksPrefix, lock.GetName()),
-				Value:   value,
-				Expires: lock.Expiry(),
-				ID:      lock.GetResourceID(),
+				Key:      backend.Key(locksPrefix, lock.GetName()),
+				Value:    value,
+				Expires:  lock.Expiry(),
+				ID:       lock.GetResourceID(),
+				Revision: rev,
 			}
 			newRemoteLocksToStore[string(item.Key)] = item
 		}

lib/services/local/apps.go

@@ -99,15 +99,17 @@ func (s *AppService) UpdateApp(ctx context.Context, app types.Application) error
 	if err := app.CheckAndSetDefaults(); err != nil {
 		return trace.Wrap(err)
 	}
+	rev := app.GetRevision()
 	value, err := services.MarshalApp(app)
 	if err != nil {
 		return trace.Wrap(err)
 	}
 	item := backend.Item{
-		Key:     backend.Key(appPrefix, app.GetName()),
-		Value:   value,
-		Expires: app.Expiry(),
-		ID:      app.GetResourceID(),
+		Key:      backend.Key(appPrefix, app.GetName()),
+		Value:    value,
+		Expires:  app.Expiry(),
+		ID:       app.GetResourceID(),
+		Revision: rev,
 	}
 	_, err = s.Update(ctx, item)
 	if err != nil {