From c01024b5531ad1b123fb6f400b16173fc01f7307 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Tue, 1 Dec 2020 09:39:35 +0300 Subject: [PATCH] [#217] pkg/eacl: Change interface for working with keys on Target and Record In previous implementation Target provided Keys/SetKeys methods which allowed working with ECDSA keys. There was also a bug in the NewTargetFromV2 function when the binary key differed in format from the ECDSA key. New BinaryKeys/SetBinaryKeys methods work with binary keys. To work with ECDSA keys added functions TargetECDSAKeys/SetTargetECDSAKeys. Old methods are left and marked deprecated. Type Record provided an interface for adding a Target by Role and a list of ECDSA keys. New SetTargets method allows to set the list of Target's, AddTarget function allows to add a single Target. AddFormedTarget works like old AddTarget method, which is now deprecated. Signed-off-by: Leonard Lyubich --- pkg/acl/eacl/record.go | 29 ++++++++--- pkg/acl/eacl/record_test.go | 34 ++++++++++--- pkg/acl/eacl/target.go | 97 +++++++++++++++++++++++++++++-------- pkg/acl/eacl/target_test.go | 17 +++---- 4 files changed, 134 insertions(+), 43 deletions(-) diff --git a/pkg/acl/eacl/record.go b/pkg/acl/eacl/record.go index dd4abbf..fec9d50 100644 --- a/pkg/acl/eacl/record.go +++ b/pkg/acl/eacl/record.go @@ -26,6 +26,11 @@ func (r Record) Targets() []*Target { return r.targets } +// SetTargets sets list of target subjects to apply ACL rule to. +func (r *Record) SetTargets(targets ...*Target) { + r.targets = targets +} + // Filters returns list of filters to match and see if rule is applicable. func (r Record) Filters() []*Filter { return r.filters @@ -52,17 +57,25 @@ func (r *Record) SetAction(action Action) { } // AddTarget adds target subject with specified Role and key list. +// +// Deprecated: use AddFormedTarget instead. func (r *Record) AddTarget(role Role, keys ...ecdsa.PublicKey) { - t := &Target{ - role: role, - keys: make([]ecdsa.PublicKey, 0, len(keys)), - } + AddFormedTarget(r, role, keys...) +} - for i := range keys { - t.keys = append(t.keys, keys[i]) - } +// AddRecordTarget adds single Target to the Record. +func AddRecordTarget(r *Record, t *Target) { + r.SetTargets(append(r.Targets(), t)...) +} - r.targets = append(r.targets, t) +// AddFormedTarget forms Target with specified Role and list of +// ECDSA public keys and adds it to the Record. +func AddFormedTarget(r *Record, role Role, keys ...ecdsa.PublicKey) { + t := NewTarget() + t.SetRole(role) + SetTargetECDSAKeys(t, ecdsaKeysToPtrs(keys)...) + + AddRecordTarget(r, t) } func (r *Record) addFilter(from FilterHeaderType, m Match, keyTyp filterKeyType, key string, val fmt.Stringer) { diff --git a/pkg/acl/eacl/record_test.go b/pkg/acl/eacl/record_test.go index 87b5345..a22f223 100644 --- a/pkg/acl/eacl/record_test.go +++ b/pkg/acl/eacl/record_test.go @@ -15,7 +15,10 @@ func TestRecord(t *testing.T) { record.SetAction(ActionAllow) record.AddFilter(HeaderFromRequest, MatchStringEqual, "A", "B") record.AddFilter(HeaderFromRequest, MatchStringNotEqual, "C", "D") - record.AddTarget(RoleSystem) + + target := NewTarget() + target.SetRole(RoleSystem) + AddRecordTarget(record, target) v2 := record.ToV2() require.NotNil(t, v2) @@ -38,8 +41,11 @@ func TestRecord(t *testing.T) { }) } -func TestRecord_AddTarget(t *testing.T) { - targets := []*Target{ +func TestAddFormedTarget(t *testing.T) { + items := []struct { + role Role + keys []ecdsa.PublicKey + }{ { role: RoleUnknown, keys: []ecdsa.PublicKey{test.DecodeKey(1).PublicKey}, @@ -50,12 +56,26 @@ func TestRecord_AddTarget(t *testing.T) { }, } + targets := make([]*Target, 0, len(items)) + r := NewRecord() - for _, target := range targets { - r.AddTarget(target.Role(), target.Keys()...) + + for _, item := range items { + tgt := NewTarget() + tgt.SetRole(item.role) + SetTargetECDSAKeys(tgt, ecdsaKeysToPtrs(item.keys)...) + + targets = append(targets, tgt) + + AddFormedTarget(r, item.role, item.keys...) } - require.Equal(t, targets, r.Targets()) + tgts := r.Targets() + require.Len(t, tgts, len(targets)) + + for _, tgt := range targets { + require.Contains(t, tgts, tgt) + } } func TestRecord_AddFilter(t *testing.T) { @@ -77,7 +97,7 @@ func TestRecordEncoding(t *testing.T) { r.SetOperation(OperationHead) r.SetAction(ActionDeny) r.AddObjectAttributeFilter(MatchStringEqual, "key", "value") - r.AddTarget(RoleSystem, test.DecodeKey(-1).PublicKey) + AddFormedTarget(r, RoleSystem, test.DecodeKey(-1).PublicKey) t.Run("binary", func(t *testing.T) { data, err := r.Marshal() diff --git a/pkg/acl/eacl/target.go b/pkg/acl/eacl/target.go index 0d15bda..b951082 100644 --- a/pkg/acl/eacl/target.go +++ b/pkg/acl/eacl/target.go @@ -13,17 +13,88 @@ import ( // Target is compatible with v2 acl.EACLRecord.Target message. type Target struct { role Role - keys []ecdsa.PublicKey + keys [][]byte } -// SetKeys sets list of public keys to identify target subject. +func ecdsaKeysToPtrs(keys []ecdsa.PublicKey) []*ecdsa.PublicKey { + keysPtr := make([]*ecdsa.PublicKey, len(keys)) + + for i := range keys { + keysPtr[i] = &keys[i] + } + + return keysPtr +} + +// SetKeys sets list of ECDSA public keys to identify target subject. +// +// Deprecated: use SetTargetECDSAKeys instead. func (t *Target) SetKeys(keys ...ecdsa.PublicKey) { + SetTargetECDSAKeys(t, ecdsaKeysToPtrs(keys)...) +} + +// Keys returns list of ECDSA public keys to identify target subject. +// If some key has a different format, it is ignored. +// +// Deprecated: use TargetECDSAKeys instead. +func (t *Target) Keys() []ecdsa.PublicKey { + keysPtr := TargetECDSAKeys(t) + keys := make([]ecdsa.PublicKey, 0, len(keysPtr)) + + for i := range keysPtr { + if keysPtr[i] != nil { + keys = append(keys, *keysPtr[i]) + } + } + + return keys +} + +// BinaryKeys returns list of public keys to identify +// target subject in a binary format. +func (t *Target) BinaryKeys() [][]byte { + return t.keys +} + +// SetBinaryKeys sets list of binary public keys to identify +// target subject. +func (t *Target) SetBinaryKeys(keys [][]byte) { t.keys = keys } -// Keys returns list of public keys to identify target subject. -func (t Target) Keys() []ecdsa.PublicKey { - return t.keys +// SetTargetECDSAKeys converts ECDSA public keys to a binary +// format and stores them in Target. +func SetTargetECDSAKeys(t *Target, keys ...*ecdsa.PublicKey) { + binKeys := t.BinaryKeys() + ln := len(keys) + + if cap(binKeys) >= ln { + binKeys = binKeys[:0] + } else { + binKeys = make([][]byte, 0, ln) + } + + for i := 0; i < ln; i++ { + binKeys = append(binKeys, crypto.MarshalPublicKey(keys[i])) + } + + t.SetBinaryKeys(binKeys) +} + +// TargetECDSAKeys interprets binary public keys of Target +// as ECDSA public keys. If any key has a different format, +// the corresponding element will be nil. +func TargetECDSAKeys(t *Target) []*ecdsa.PublicKey { + binKeys := t.BinaryKeys() + ln := len(binKeys) + + keys := make([]*ecdsa.PublicKey, ln) + + for i := 0; i < ln; i++ { + keys[i] = crypto.UnmarshalPublicKey(binKeys[i]) + } + + return keys } // SetRole sets target subject's role class. @@ -38,16 +109,10 @@ func (t Target) Role() Role { // ToV2 converts Target to v2 acl.EACLRecord.Target message. func (t *Target) ToV2() *v2acl.Target { - keys := make([][]byte, 0, len(t.keys)) - for i := range t.keys { - key := crypto.MarshalPublicKey(&t.keys[i]) - keys = append(keys, key) - } - target := new(v2acl.Target) target.SetRole(t.role.ToV2()) - target.SetKeys(keys) + target.SetKeys(t.keys) return target } @@ -66,13 +131,7 @@ func NewTargetFromV2(target *v2acl.Target) *Target { } t.role = RoleFromV2(target.GetRole()) - v2keys := target.GetKeys() - t.keys = make([]ecdsa.PublicKey, 0, len(v2keys)) - - for i := range v2keys { - key := crypto.UnmarshalPublicKey(v2keys[i]) - t.keys = append(t.keys, *key) - } + t.keys = target.GetKeys() return t } diff --git a/pkg/acl/eacl/target_test.go b/pkg/acl/eacl/target_test.go index 712804c..5c33ec7 100644 --- a/pkg/acl/eacl/target_test.go +++ b/pkg/acl/eacl/target_test.go @@ -11,22 +11,21 @@ import ( ) func TestTarget(t *testing.T) { - keys := []ecdsa.PublicKey{ - test.DecodeKey(1).PublicKey, - test.DecodeKey(2).PublicKey, + keys := []*ecdsa.PublicKey{ + &test.DecodeKey(1).PublicKey, + &test.DecodeKey(2).PublicKey, } - target := &Target{ - role: RoleSystem, - keys: keys, - } + target := NewTarget() + target.SetRole(RoleSystem) + SetTargetECDSAKeys(target, keys...) v2 := target.ToV2() require.NotNil(t, v2) require.Equal(t, v2acl.RoleSystem, v2.GetRole()) require.Len(t, v2.GetKeys(), len(keys)) for i, key := range v2.GetKeys() { - require.Equal(t, key, crypto.MarshalPublicKey(&keys[i])) + require.Equal(t, key, crypto.MarshalPublicKey(keys[i])) } newTarget := NewTargetFromV2(v2) @@ -40,7 +39,7 @@ func TestTarget(t *testing.T) { func TestTargetEncoding(t *testing.T) { tar := NewTarget() tar.SetRole(RoleSystem) - tar.SetKeys(test.DecodeKey(-1).PublicKey) + SetTargetECDSAKeys(tar, &test.DecodeKey(-1).PublicKey) t.Run("binary", func(t *testing.T) { data, err := tar.Marshal()