From 4a907bb0edf24522eba697e59b76776c5112d08c Mon Sep 17 00:00:00 2001 From: Sorin Dumitru Date: Fri, 20 Sep 2024 07:39:37 +0100 Subject: [PATCH] Validate registration entry at input to be able to return error codes Signed-off-by: Sorin Dumitru --- pkg/server/api/entry/v1/service.go | 104 ++++++ pkg/server/api/entry/v1/service_test.go | 328 +++++++++++++++++- pkg/server/datastore/sqlstore/sqlstore.go | 115 +----- .../datastore/sqlstore/sqlstore_test.go | 86 ----- 4 files changed, 418 insertions(+), 215 deletions(-) diff --git a/pkg/server/api/entry/v1/service.go b/pkg/server/api/entry/v1/service.go index 180d66c241..bd4b992563 100644 --- a/pkg/server/api/entry/v1/service.go +++ b/pkg/server/api/entry/v1/service.go @@ -7,6 +7,7 @@ import ( "slices" "sort" "strings" + "unicode" "github.com/sirupsen/logrus" "github.com/spiffe/go-spiffe/v2/spiffeid" @@ -22,6 +23,17 @@ import ( "google.golang.org/grpc/status" ) +var validEntryIDChars = &unicode.RangeTable{ + R16: []unicode.Range16{ + {0x002d, 0x002e, 1}, // - | . + {0x0030, 0x0039, 1}, // [0-9] + {0x0041, 0x005a, 1}, // [A-Z] + {0x005f, 0x005f, 1}, // _ + {0x0061, 0x007a, 1}, // [a-z] + }, + LatinOffset: 5, +} + const defaultEntryPageSize = 500 // Config defines the service configuration. @@ -275,6 +287,45 @@ func (s *Service) BatchCreateEntry(ctx context.Context, req *entryv1.BatchCreate }, nil } +func validateRegistrationEntry(entry *common.RegistrationEntry) error { + // In case of StoreSvid is set, all entries 'must' be the same type, + // it is done to avoid users to mix selectors from different platforms in + // entries with storable SVIDs + if entry.StoreSvid { + // Selectors must never be empty + tpe := entry.Selectors[0].Type + for _, t := range entry.Selectors { + if tpe != t.Type { + return errors.New("invalid registration entry: selector types must be the same when store SVID is enabled") + } + } + } + + if len(entry.EntryId) > 255 { + return errors.New("invalid registration entry: entry ID too long") + } + + for _, e := range entry.EntryId { + if !unicode.In(e, validEntryIDChars) { + return errors.New("invalid registration entry: entry ID contains invalid characters") + } + } + + if len(entry.SpiffeId) == 0 { + return errors.New("invalid registration entry: missing SPIFFE ID") + } + + if entry.X509SvidTtl < 0 { + return errors.New("invalid registration entry: X509SvidTtl is not set") + } + + if entry.JwtSvidTtl < 0 { + return errors.New("invalid registration entry: JwtSvidTtl is not set") + } + + return nil +} + func (s *Service) createEntry(ctx context.Context, e *types.Entry, outputMask *types.EntryMask) *entryv1.BatchCreateEntryResponse_Result { log := rpccontext.Logger(ctx) @@ -285,6 +336,12 @@ func (s *Service) createEntry(ctx context.Context, e *types.Entry, outputMask *t } } + if err := validateRegistrationEntry(cEntry); err != nil { + return &entryv1.BatchCreateEntryResponse_Result{ + Status: api.MakeStatus(log, codes.InvalidArgument, "failed to create entry", err), + } + } + log = log.WithField(telemetry.SPIFFEID, cEntry.SpiffeId) resultStatus := api.OK() @@ -630,6 +687,46 @@ func applyMask(e *types.Entry, mask *types.EntryMask) { } } +func validateRegistrationEntryForUpdate(entry *common.RegistrationEntry, mask *common.RegistrationEntryMask) error { + if entry == nil { + return errors.New("invalid request: missing registered entry") + } + + if (mask == nil || mask.Selectors) && len(entry.Selectors) == 0 { + return errors.New("invalid registration entry: missing selector list") + } + + // In case of StoreSvid is set, all entries 'must' be the same type, + // it is done to avoid users to mix selectors from different platforms in + // entries with storable SVIDs + if entry.StoreSvid { + // Selectors must never be empty + tpe := entry.Selectors[0].Type + for _, t := range entry.Selectors { + if tpe != t.Type { + return errors.New("invalid registration entry: selector types must be the same when store SVID is enabled") + } + } + } + + if (mask == nil || mask.SpiffeId) && + entry.SpiffeId == "" { + return errors.New("invalid registration entry: missing SPIFFE ID") + } + + if (mask == nil || mask.X509SvidTtl) && + (entry.X509SvidTtl < 0) { + return errors.New("invalid registration entry: X509SvidTtl is not set") + } + + if (mask == nil || mask.JwtSvidTtl) && + (entry.JwtSvidTtl < 0) { + return errors.New("invalid registration entry: JwtSvidTtl is not set") + } + + return nil +} + func (s *Service) updateEntry(ctx context.Context, e *types.Entry, inputMask *types.EntryMask, outputMask *types.EntryMask) *entryv1.BatchUpdateEntryResponse_Result { log := rpccontext.Logger(ctx) log = log.WithField(telemetry.RegistrationID, e.Id) @@ -658,6 +755,13 @@ func (s *Service) updateEntry(ctx context.Context, e *types.Entry, inputMask *ty Hint: inputMask.Hint, } } + + if err := validateRegistrationEntryForUpdate(convEntry, mask); err != nil { + return &entryv1.BatchUpdateEntryResponse_Result{ + Status: api.MakeStatus(log, codes.InvalidArgument, "failed to update entry", err), + } + } + dsEntry, err := s.ds.UpdateRegistrationEntry(ctx, convEntry, mask) if err != nil { return &entryv1.BatchUpdateEntryResponse_Result{ diff --git a/pkg/server/api/entry/v1/service_test.go b/pkg/server/api/entry/v1/service_test.go index 429c01ec60..8e75af6a97 100644 --- a/pkg/server/api/entry/v1/service_test.go +++ b/pkg/server/api/entry/v1/service_test.go @@ -2185,24 +2185,23 @@ func TestBatchCreateEntry(t *testing.T) { expectResults: []*entryv1.BatchCreateEntryResponse_Result{ { Status: &types.Status{ - Code: int32(codes.Internal), - Message: "failed to create entry: datastore-sql: invalid registration entry: entry ID contains invalid characters", + Code: int32(codes.InvalidArgument), + Message: "failed to create entry: invalid registration entry: entry ID contains invalid characters", }, }, { Status: &types.Status{ - Code: int32(codes.Internal), - Message: "failed to create entry: datastore-sql: invalid registration entry: entry ID too long", + Code: int32(codes.InvalidArgument), + Message: "failed to create entry: invalid registration entry: entry ID too long", }, }, }, expectLogs: []spiretest.LogEntry{ { Level: logrus.ErrorLevel, - Message: "Failed to create entry", + Message: "Invalid argument: failed to create entry", Data: logrus.Fields{ - logrus.ErrorKey: "datastore-sql: invalid registration entry: entry ID contains invalid characters", - telemetry.SPIFFEID: "spiffe://example.org/bar", + logrus.ErrorKey: "invalid registration entry: entry ID contains invalid characters", }, }, { @@ -2224,16 +2223,15 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Hint: "", telemetry.CreatedAt: "0", telemetry.StoreSvid: "false", - telemetry.StatusCode: "Internal", - telemetry.StatusMessage: "failed to create entry: datastore-sql: invalid registration entry: entry ID contains invalid characters", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to create entry: invalid registration entry: entry ID contains invalid characters", }, }, { Level: logrus.ErrorLevel, - Message: "Failed to create entry", + Message: "Invalid argument: failed to create entry", Data: logrus.Fields{ - logrus.ErrorKey: "datastore-sql: invalid registration entry: entry ID too long", - telemetry.SPIFFEID: "spiffe://example.org/bar", + logrus.ErrorKey: "invalid registration entry: entry ID too long", }, }, { @@ -2255,8 +2253,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Hint: "", telemetry.CreatedAt: "0", telemetry.StoreSvid: "false", - telemetry.StatusCode: "Internal", - telemetry.StatusMessage: "failed to create entry: datastore-sql: invalid registration entry: entry ID too long", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to create entry: invalid registration entry: entry ID too long", }, }, }, @@ -2389,6 +2387,214 @@ func TestBatchCreateEntry(t *testing.T) { SpiffeId: "sparfe://invalid/scheme", }}, }, + { + name: "missing selector list", + expectResults: []*entryv1.BatchCreateEntryResponse_Result{ + { + Status: &types.Status{ + Code: int32(codes.InvalidArgument), + Message: "failed to convert entry: selector list is empty", + }, + }, + }, + expectLogs: []spiretest.LogEntry{ + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to convert entry", + Data: logrus.Fields{ + logrus.ErrorKey: "selector list is empty", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.Admin: "false", + telemetry.Downstream: "false", + telemetry.RegistrationID: "missingselectorlist", + telemetry.ExpiresAt: "0", + telemetry.ParentID: "spiffe://example.org/foo", + telemetry.RevisionNumber: "0", + telemetry.SPIFFEID: "spiffe://example.org/bar", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", + telemetry.Hint: "", + telemetry.CreatedAt: "0", + telemetry.StoreSvid: "false", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to convert entry: selector list is empty", + }, + }, + }, + reqEntries: []*types.Entry{ + { + Id: "missingselectorlist", + ParentId: api.ProtoFromID(entryParentID), + SpiffeId: api.ProtoFromID(entrySpiffeID), + X509SvidTtl: 45, + JwtSvidTtl: 30, + }, + }, + noCustomCreate: true, + }, + { + name: "missing SPIFFE ID", + expectResults: []*entryv1.BatchCreateEntryResponse_Result{ + { + Status: &types.Status{ + Code: int32(codes.InvalidArgument), + Message: "failed to convert entry: invalid spiffe ID: request must specify SPIFFE ID", + }, + }, + }, + expectLogs: []spiretest.LogEntry{ + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to convert entry", + Data: logrus.Fields{ + logrus.ErrorKey: "invalid spiffe ID: request must specify SPIFFE ID", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.Admin: "false", + telemetry.Downstream: "false", + telemetry.RegistrationID: "missingspiffeid", + telemetry.ExpiresAt: "0", + telemetry.ParentID: "spiffe://example.org/foo", + telemetry.RevisionNumber: "0", + telemetry.Selectors: "type:value1", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", + telemetry.Hint: "", + telemetry.CreatedAt: "0", + telemetry.StoreSvid: "false", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to convert entry: invalid spiffe ID: request must specify SPIFFE ID", + }, + }, + }, + reqEntries: []*types.Entry{ + { + Id: "missingspiffeid", + ParentId: api.ProtoFromID(entryParentID), + X509SvidTtl: 45, + JwtSvidTtl: 30, + Selectors: []*types.Selector{ + {Type: "type", Value: "value1"}, + }, + }, + }, + noCustomCreate: true, + }, + { + name: "negative X509/JWT-SVID TTL", + expectResults: []*entryv1.BatchCreateEntryResponse_Result{ + { + Status: &types.Status{ + Code: int32(codes.InvalidArgument), + Message: "failed to create entry: invalid registration entry: JwtSvidTtl is not set", + }, + }, + { + Status: &types.Status{ + Code: int32(codes.InvalidArgument), + Message: "failed to create entry: invalid registration entry: X509SvidTtl is not set", + }, + }, + }, + expectLogs: []spiretest.LogEntry{ + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to create entry", + Data: logrus.Fields{ + logrus.ErrorKey: "invalid registration entry: JwtSvidTtl is not set", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.Admin: "false", + telemetry.Downstream: "false", + telemetry.RegistrationID: "invalidjwtsvidttl", + telemetry.ExpiresAt: "0", + telemetry.ParentID: "spiffe://example.org/foo", + telemetry.RevisionNumber: "0", + telemetry.Selectors: "type:value1", + telemetry.SPIFFEID: "spiffe://example.org/bar", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "-1", + telemetry.Hint: "", + telemetry.CreatedAt: "0", + telemetry.StoreSvid: "false", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to create entry: invalid registration entry: JwtSvidTtl is not set", + }, + }, + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to create entry", + Data: logrus.Fields{ + logrus.ErrorKey: "invalid registration entry: X509SvidTtl is not set", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.Admin: "false", + telemetry.Downstream: "false", + telemetry.RegistrationID: "invalidx509svidttl", + telemetry.ExpiresAt: "0", + telemetry.ParentID: "spiffe://example.org/foo", + telemetry.RevisionNumber: "0", + telemetry.Selectors: "type:value1", + telemetry.SPIFFEID: "spiffe://example.org/bar", + telemetry.X509SVIDTTL: "-1", + telemetry.JWTSVIDTTL: "30", + telemetry.Hint: "", + telemetry.CreatedAt: "0", + telemetry.StoreSvid: "false", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to create entry: invalid registration entry: X509SvidTtl is not set", + }, + }, + }, + reqEntries: []*types.Entry{ + { + Id: "invalidjwtsvidttl", + ParentId: api.ProtoFromID(entryParentID), + SpiffeId: api.ProtoFromID(entrySpiffeID), + X509SvidTtl: 45, + JwtSvidTtl: -1, + Selectors: []*types.Selector{ + {Type: "type", Value: "value1"}, + }, + }, + { + Id: "invalidx509svidttl", + ParentId: api.ProtoFromID(entryParentID), + SpiffeId: api.ProtoFromID(entrySpiffeID), + X509SvidTtl: -1, + JwtSvidTtl: 30, + Selectors: []*types.Selector{ + {Type: "type", Value: "value1"}, + }, + }, + }, + noCustomCreate: true, + }, } { tt := tt t.Run(tt.name, func(t *testing.T) { @@ -4130,17 +4336,17 @@ func TestBatchUpdateEntry(t *testing.T) { }, expectResults: []*entryv1.BatchUpdateEntryResponse_Result{ { - Status: &types.Status{Code: int32(codes.Internal), Message: "failed to update entry: datastore-sql: invalid registration entry: selector types must be the same when store SVID is enabled"}, + Status: &types.Status{Code: int32(codes.InvalidArgument), Message: "failed to update entry: invalid registration entry: selector types must be the same when store SVID is enabled"}, }, }, expectLogs: func(m map[string]string) []spiretest.LogEntry { return []spiretest.LogEntry{ { Level: logrus.ErrorLevel, - Message: "Failed to update entry", + Message: "Invalid argument: failed to update entry", Data: logrus.Fields{ telemetry.RegistrationID: m[entry1SpiffeID.Path], - logrus.ErrorKey: "rpc error: code = Unknown desc = datastore-sql: invalid registration entry: selector types must be the same when store SVID is enabled", + logrus.ErrorKey: "invalid registration entry: selector types must be the same when store SVID is enabled", }, }, { @@ -4148,8 +4354,8 @@ func TestBatchUpdateEntry(t *testing.T) { Message: "API accessed", Data: logrus.Fields{ telemetry.Status: "error", - telemetry.StatusCode: "Internal", - telemetry.StatusMessage: "failed to update entry: datastore-sql: invalid registration entry: selector types must be the same when store SVID is enabled", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to update entry: invalid registration entry: selector types must be the same when store SVID is enabled", telemetry.Type: "audit", telemetry.RegistrationID: m[entry1SpiffeID.Path], telemetry.Selectors: "type1:key1:value,type2:key2:value", @@ -4323,6 +4529,90 @@ func TestBatchUpdateEntry(t *testing.T) { } }, }, + { + name: "Fail Invalid X509-SVID TTL", + initialEntries: []*types.Entry{initialEntry}, + inputMask: &types.EntryMask{ + X509SvidTtl: true, + }, + updateEntries: []*types.Entry{ + { + X509SvidTtl: -1, + }, + }, + expectResults: []*entryv1.BatchUpdateEntryResponse_Result{ + { + Status: &types.Status{Code: int32(codes.InvalidArgument), + Message: "failed to update entry: invalid registration entry: X509SvidTtl is not set"}, + }, + }, + expectLogs: func(m map[string]string) []spiretest.LogEntry { + return []spiretest.LogEntry{ + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to update entry", + Data: logrus.Fields{ + telemetry.RegistrationID: m[entry1SpiffeID.Path], + logrus.ErrorKey: "invalid registration entry: X509SvidTtl is not set", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.RegistrationID: m[entry1SpiffeID.Path], + telemetry.X509SVIDTTL: "-1", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to update entry: invalid registration entry: X509SvidTtl is not set", + }, + }, + } + }, + }, + { + name: "Fail Invalid JWT-SVID TTL", + initialEntries: []*types.Entry{initialEntry}, + inputMask: &types.EntryMask{ + JwtSvidTtl: true, + }, + updateEntries: []*types.Entry{ + { + JwtSvidTtl: -1, + }, + }, + expectResults: []*entryv1.BatchUpdateEntryResponse_Result{ + { + Status: &types.Status{Code: int32(codes.InvalidArgument), + Message: "failed to update entry: invalid registration entry: JwtSvidTtl is not set"}, + }, + }, + expectLogs: func(m map[string]string) []spiretest.LogEntry { + return []spiretest.LogEntry{ + { + Level: logrus.ErrorLevel, + Message: "Invalid argument: failed to update entry", + Data: logrus.Fields{ + telemetry.RegistrationID: m[entry1SpiffeID.Path], + logrus.ErrorKey: "invalid registration entry: JwtSvidTtl is not set", + }, + }, + { + Level: logrus.InfoLevel, + Message: "API accessed", + Data: logrus.Fields{ + telemetry.Status: "error", + telemetry.Type: "audit", + telemetry.RegistrationID: m[entry1SpiffeID.Path], + telemetry.JWTSVIDTTL: "-1", + telemetry.StatusCode: "InvalidArgument", + telemetry.StatusMessage: "failed to update entry: invalid registration entry: JwtSvidTtl is not set", + }, + }, + } + }, + }, { name: "Fail Empty Selectors List", initialEntries: []*types.Entry{initialEntry}, diff --git a/pkg/server/datastore/sqlstore/sqlstore.go b/pkg/server/datastore/sqlstore/sqlstore.go index f1645e7bd4..cd1c2ec8d7 100644 --- a/pkg/server/datastore/sqlstore/sqlstore.go +++ b/pkg/server/datastore/sqlstore/sqlstore.go @@ -12,7 +12,6 @@ import ( "strings" "sync" "time" - "unicode" "github.com/gofrs/uuid/v5" "github.com/hashicorp/hcl" @@ -37,17 +36,7 @@ import ( ) var ( - sqlError = errs.Class("datastore-sql") - validEntryIDChars = &unicode.RangeTable{ - R16: []unicode.Range16{ - {0x002d, 0x002e, 1}, // - | . - {0x0030, 0x0039, 1}, // [0-9] - {0x0041, 0x005a, 1}, // [A-Z] - {0x005f, 0x005f, 1}, // _ - {0x0061, 0x007a, 1}, // [a-z] - }, - LatinOffset: 5, - } + sqlError = errs.Class("datastore-sql") ) const ( @@ -473,9 +462,8 @@ func (ds *Plugin) CreateOrReturnRegistrationEntry(ctx context.Context, func (ds *Plugin) createOrReturnRegistrationEntry(ctx context.Context, entry *common.RegistrationEntry, ) (registrationEntry *common.RegistrationEntry, existing bool, err error) { - // TODO: Validations should be done in the ProtoBuf level [https://github.com/spiffe/spire/issues/44] - if err = validateRegistrationEntry(entry); err != nil { - return nil, false, err + if entry == nil { + return nil, false, sqlError.New("invalid request: missing registered entry") } if err = ds.withWriteTx(ctx, func(tx *gorm.DB) (err error) { @@ -3879,8 +3867,8 @@ func applyPagination(p *datastore.Pagination, entryTx *gorm.DB) (*gorm.DB, error } func updateRegistrationEntry(tx *gorm.DB, e *common.RegistrationEntry, mask *common.RegistrationEntryMask) (*common.RegistrationEntry, error) { - if err := validateRegistrationEntryForUpdate(e, mask); err != nil { - return nil, err + if e == nil { + return nil, sqlError.New("invalid request: missing registered entry") } // Get the existing entry @@ -3909,11 +3897,6 @@ func updateRegistrationEntry(tx *gorm.DB, e *common.RegistrationEntry, mask *com entry.Selectors = selectors } - // Verify that final selectors contains the same 'type' when entry is used for store SVIDs - if entry.StoreSvid && !equalSelectorTypes(entry.Selectors) { - return nil, sqlError.New("invalid registration entry: selector types must be the same when store SVID is enabled") - } - if mask == nil || mask.DnsNames { // Delete existing DNSs - we will write new ones if err := tx.Exec("DELETE FROM dns_names WHERE registered_entry_id = ?", entry.ID).Error; err != nil { @@ -4396,94 +4379,6 @@ func modelToBundle(model *Bundle) (*common.Bundle, error) { return bundle, nil } -func validateRegistrationEntry(entry *common.RegistrationEntry) error { - if entry == nil { - return sqlError.New("invalid request: missing registered entry") - } - - if len(entry.Selectors) == 0 { - return sqlError.New("invalid registration entry: missing selector list") - } - - // In case of StoreSvid is set, all entries 'must' be the same type, - // it is done to avoid users to mix selectors from different platforms in - // entries with storable SVIDs - if entry.StoreSvid { - // Selectors must never be empty - tpe := entry.Selectors[0].Type - for _, t := range entry.Selectors { - if tpe != t.Type { - return sqlError.New("invalid registration entry: selector types must be the same when store SVID is enabled") - } - } - } - - if len(entry.EntryId) > 255 { - return sqlError.New("invalid registration entry: entry ID too long") - } - - for _, e := range entry.EntryId { - if !unicode.In(e, validEntryIDChars) { - return sqlError.New("invalid registration entry: entry ID contains invalid characters") - } - } - - if len(entry.SpiffeId) == 0 { - return sqlError.New("invalid registration entry: missing SPIFFE ID") - } - - if entry.X509SvidTtl < 0 { - return sqlError.New("invalid registration entry: X509SvidTtl is not set") - } - - if entry.JwtSvidTtl < 0 { - return sqlError.New("invalid registration entry: JwtSvidTtl is not set") - } - - return nil -} - -// equalSelectorTypes validates that all selectors has the same type, -func equalSelectorTypes(selectors []Selector) bool { - typ := "" - for _, t := range selectors { - switch { - case typ == "": - typ = t.Type - case typ != t.Type: - return false - } - } - return true -} - -func validateRegistrationEntryForUpdate(entry *common.RegistrationEntry, mask *common.RegistrationEntryMask) error { - if entry == nil { - return sqlError.New("invalid request: missing registered entry") - } - - if (mask == nil || mask.Selectors) && len(entry.Selectors) == 0 { - return sqlError.New("invalid registration entry: missing selector list") - } - - if (mask == nil || mask.SpiffeId) && - entry.SpiffeId == "" { - return sqlError.New("invalid registration entry: missing SPIFFE ID") - } - - if (mask == nil || mask.X509SvidTtl) && - (entry.X509SvidTtl < 0) { - return sqlError.New("invalid registration entry: X509SvidTtl is not set") - } - - if (mask == nil || mask.JwtSvidTtl) && - (entry.JwtSvidTtl < 0) { - return sqlError.New("invalid registration entry: JwtSvidTtl is not set") - } - - return nil -} - // bundleToModel converts the given Protobuf bundle message to a database model. It // performs validation, and fully parses certificates to form CACert embedded models. func bundleToModel(pb *common.Bundle) (*Bundle, error) { diff --git a/pkg/server/datastore/sqlstore/sqlstore_test.go b/pkg/server/datastore/sqlstore/sqlstore_test.go index 83c1dda4ed..67e8c2613a 100644 --- a/pkg/server/datastore/sqlstore/sqlstore_test.go +++ b/pkg/server/datastore/sqlstore/sqlstore_test.go @@ -5,7 +5,6 @@ import ( "crypto/x509" "database/sql" "encoding/json" - "errors" "fmt" "net/url" "os" @@ -1872,38 +1871,6 @@ func (s *PluginSuite) TestCreateOrReturnRegistrationEntry() { }, expectError: "datastore-sql: invalid request: missing registered entry", }, - { - name: "no selectors", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.Selectors = nil - return e - }, - expectError: "datastore-sql: invalid registration entry: missing selector list", - }, - { - name: "no SPIFFE ID", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.SpiffeId = "" - return e - }, - expectError: "datastore-sql: invalid registration entry: missing SPIFFE ID", - }, - { - name: "negative X509 ttl", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.X509SvidTtl = -1 - return e - }, - expectError: "datastore-sql: invalid registration entry: X509SvidTtl is not set", - }, - { - name: "negative JWT ttl", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.JwtSvidTtl = -1 - return e - }, - expectError: "datastore-sql: invalid registration entry: JwtSvidTtl is not set", - }, { name: "create entry successfully", modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { @@ -1962,22 +1929,6 @@ func (s *PluginSuite) TestCreateOrReturnRegistrationEntry() { }, expectSimilar: true, }, - { - name: "entry ID too long", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.EntryId = strings.Repeat("e", 256) - return e - }, - expectError: "datastore-sql: invalid registration entry: entry ID too long", - }, - { - name: "entry ID contains invalid characters", - modifyEntry: func(e *common.RegistrationEntry) *common.RegistrationEntry { - e.EntryId = "éntry😊" - return e - }, - expectError: "datastore-sql: invalid registration entry: entry ID contains invalid characters", - }, } { s.T().Run(tt.name, func(t *testing.T) { entry := &common.RegistrationEntry{ @@ -3011,16 +2962,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithStoreSvid() { fetchRegistrationEntry, err := s.ds.FetchRegistrationEntry(ctx, entry.EntryId) s.Require().NoError(err) s.RequireProtoEqual(updateRegistrationEntry, fetchRegistrationEntry) - - // Update with invalid selectors - entry.Selectors = []*common.Selector{ - {Type: "Type1", Value: "Value1"}, - {Type: "Type1", Value: "Value2"}, - {Type: "Type2", Value: "Value3"}, - } - resp, err := s.ds.UpdateRegistrationEntry(ctx, entry, nil) - s.Require().Nil(resp) - s.Require().EqualError(err, "rpc error: code = Unknown desc = datastore-sql: invalid registration entry: selector types must be the same when store SVID is enabled") } func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { @@ -3090,10 +3031,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { mask: &common.RegistrationEntryMask{SpiffeId: false}, update: func(e *common.RegistrationEntry) { e.SpiffeId = newEntry.SpiffeId }, result: func(e *common.RegistrationEntry) {}}, - {name: "Update Spiffe ID, Bad Data, Mask True", - mask: &common.RegistrationEntryMask{SpiffeId: true}, - update: func(e *common.RegistrationEntry) { e.SpiffeId = badEntry.SpiffeId }, - err: errors.New("invalid registration entry: missing SPIFFE ID")}, {name: "Update Spiffe ID, Bad Data, Mask False", mask: &common.RegistrationEntryMask{SpiffeId: false}, update: func(e *common.RegistrationEntry) { e.SpiffeId = badEntry.SpiffeId }, @@ -3116,10 +3053,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { mask: &common.RegistrationEntryMask{X509SvidTtl: false}, update: func(e *common.RegistrationEntry) { e.X509SvidTtl = badEntry.X509SvidTtl }, result: func(e *common.RegistrationEntry) {}}, - {name: "Update X509 SVID TTL, Bad Data, Mask True", - mask: &common.RegistrationEntryMask{X509SvidTtl: true}, - update: func(e *common.RegistrationEntry) { e.X509SvidTtl = badEntry.X509SvidTtl }, - err: errors.New("invalid registration entry: X509SvidTtl is not set")}, {name: "Update X509 SVID TTL, Bad Data, Mask False", mask: &common.RegistrationEntryMask{X509SvidTtl: false}, update: func(e *common.RegistrationEntry) { e.X509SvidTtl = badEntry.X509SvidTtl }, @@ -3133,10 +3066,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { mask: &common.RegistrationEntryMask{JwtSvidTtl: false}, update: func(e *common.RegistrationEntry) { e.JwtSvidTtl = badEntry.JwtSvidTtl }, result: func(e *common.RegistrationEntry) {}}, - {name: "Update JWT SVID TTL, Bad Data, Mask True", - mask: &common.RegistrationEntryMask{JwtSvidTtl: true}, - update: func(e *common.RegistrationEntry) { e.JwtSvidTtl = badEntry.JwtSvidTtl }, - err: errors.New("invalid registration entry: JwtSvidTtl is not set")}, {name: "Update JWT SVID TTL, Bad Data, Mask False", mask: &common.RegistrationEntryMask{JwtSvidTtl: false}, update: func(e *common.RegistrationEntry) { e.JwtSvidTtl = badEntry.JwtSvidTtl }, @@ -3150,10 +3079,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { mask: &common.RegistrationEntryMask{Selectors: false}, update: func(e *common.RegistrationEntry) { e.Selectors = badEntry.Selectors }, result: func(e *common.RegistrationEntry) {}}, - {name: "Update Selectors, Bad Data, Mask True", - mask: &common.RegistrationEntryMask{Selectors: true}, - update: func(e *common.RegistrationEntry) { e.Selectors = badEntry.Selectors }, - err: errors.New("invalid registration entry: missing selector list")}, {name: "Update Selectors, Bad Data, Mask False", mask: &common.RegistrationEntryMask{Selectors: false}, update: func(e *common.RegistrationEntry) { e.Selectors = badEntry.Selectors }, @@ -3186,17 +3111,6 @@ func (s *PluginSuite) TestUpdateRegistrationEntryWithMask() { mask: &common.RegistrationEntryMask{Admin: false}, update: func(e *common.RegistrationEntry) { e.StoreSvid = newEntry.StoreSvid }, result: func(e *common.RegistrationEntry) {}}, - {name: "Update StoreSvid, Invalid selectors, Mask True", - mask: &common.RegistrationEntryMask{StoreSvid: true, Selectors: true}, - update: func(e *common.RegistrationEntry) { - e.StoreSvid = newEntry.StoreSvid - e.Selectors = []*common.Selector{ - {Type: "Type1", Value: "Value1"}, - {Type: "Type2", Value: "Value2"}, - } - }, - err: sqlError.New("invalid registration entry: selector types must be the same when store SVID is enabled"), - }, // ENTRYEXPIRY FIELD -- This field isn't validated so we just check with good data {name: "Update EntryExpiry, Good Data, Mask True",