From b842110de525b00984c859a70ddb6b798033a793 Mon Sep 17 00:00:00 2001 From: Adam Hughes Date: Mon, 23 Aug 2021 17:28:10 +0000 Subject: [PATCH] refactor: use struct to accumulate sign options --- pkg/integrity/sign.go | 130 +++++++++++++++++------------ pkg/integrity/sign_test.go | 166 ++----------------------------------- 2 files changed, 84 insertions(+), 212 deletions(-) diff --git a/pkg/integrity/sign.go b/pkg/integrity/sign.go index cc612a32..3cfd34ca 100644 --- a/pkg/integrity/sign.go +++ b/pkg/integrity/sign.go @@ -167,20 +167,19 @@ func (gs *groupSigner) signWithEntity(e *openpgp.Entity) (sif.DescriptorInput, e ) } -// Signer describes a SIF image signer. -type Signer struct { - f *sif.FileImage // SIF image to sign. - signers []*groupSigner // Signer for each group. - e *openpgp.Entity // Entity to use to generate signature(s). +type signOpts struct { + e *openpgp.Entity + groupIDs []uint32 + objectIDs [][]uint32 } -// SignerOpt are used to configure s. -type SignerOpt func(s *Signer) error +// SignerOpt are used to configure so. +type SignerOpt func(so *signOpts) error // OptSignWithEntity specifies e as the entity to use to generate signature(s). func OptSignWithEntity(e *openpgp.Entity) SignerOpt { - return func(s *Signer) error { - s.e = e + return func(so *signOpts) error { + so.e = e return nil } } @@ -188,12 +187,8 @@ func OptSignWithEntity(e *openpgp.Entity) SignerOpt { // OptSignGroup specifies that a signature be applied to cover all objects in the group with the // specified groupID. This may be called multiple times to add multiple group signatures. func OptSignGroup(groupID uint32) SignerOpt { - return func(s *Signer) error { - gs, err := newGroupSigner(s.f, groupID) - if err != nil { - return err - } - s.signers = append(s.signers, gs) + return func(so *signOpts) error { + so.groupIDs = append(so.groupIDs, groupID) return nil } } @@ -202,54 +197,53 @@ func OptSignGroup(groupID uint32) SignerOpt { // specified ids. One signature will be applied for each group ID associated with the object(s). // This may be called multiple times to add multiple signatures. func OptSignObjects(ids ...uint32) SignerOpt { - return func(s *Signer) error { + return func(so *signOpts) error { if len(ids) == 0 { return errNoObjectsSpecified } - idMap := make(map[uint32]bool) - groupObjectIDs := make(map[uint32][]uint32) - var groupIDs []uint32 - - for _, id := range ids { - if id == 0 { - return sif.ErrInvalidObjectID - } + so.objectIDs = append(so.objectIDs, ids) + return nil + } +} - // Ignore duplicate IDs. - if _, ok := idMap[id]; ok { - continue - } - idMap[id] = true +// withGroupedObjects splits the objects represented by ids into object groups, and calls fn once +// per object group. +func withGroupedObjects(f *sif.FileImage, ids []uint32, fn func(uint32, []uint32) error) error { + var groupIDs []uint32 + groupObjectIDs := make(map[uint32][]uint32) - // Get descriptor. - od, err := s.f.GetDescriptor(sif.WithID(id)) - if err != nil { - return err - } + for _, id := range ids { + od, err := f.GetDescriptor(sif.WithID(id)) + if err != nil { + return err + } - // Note the group ID if it hasn't been seen before, and append the object ID to the - // appropriate group in the map. - groupID := od.GroupID() - if _, ok := groupObjectIDs[groupID]; !ok { - groupIDs = append(groupIDs, groupID) - } - groupObjectIDs[groupID] = append(groupObjectIDs[groupID], id) + // Note the group ID if it hasn't been seen before, and append the object ID to the + // appropriate group in the map. + groupID := od.GroupID() + if _, ok := groupObjectIDs[groupID]; !ok { + groupIDs = append(groupIDs, groupID) } + groupObjectIDs[groupID] = append(groupObjectIDs[groupID], id) + } - sort.Slice(groupIDs, func(i, j int) bool { return groupIDs[i] < groupIDs[j] }) + sort.Slice(groupIDs, func(i, j int) bool { return groupIDs[i] < groupIDs[j] }) - // Add one groupSigner per group. - for _, groupID := range groupIDs { - gs, err := newGroupSigner(s.f, groupID, optSignGroupObjects(groupObjectIDs[groupID]...)) - if err != nil { - return err - } - s.signers = append(s.signers, gs) + for _, groupID := range groupIDs { + if err := fn(groupID, groupObjectIDs[groupID]); err != nil { + return err } - - return nil } + + return nil +} + +// Signer describes a SIF image signer. +type Signer struct { + f *sif.FileImage + signers []*groupSigner + e *openpgp.Entity } // NewSigner returns a Signer to add digital signature(s) to f, according to opts. @@ -263,11 +257,41 @@ func NewSigner(f *sif.FileImage, opts ...SignerOpt) (*Signer, error) { return nil, fmt.Errorf("integrity: %w", errNilFileImage) } - s := Signer{f: f} + so := signOpts{} // Apply options. for _, opt := range opts { - if err := opt(&s); err != nil { + if err := opt(&so); err != nil { + return nil, fmt.Errorf("integrity: %w", err) + } + } + + s := Signer{ + f: f, + e: so.e, + } + + // Add signer for each groupID. + for _, groupID := range so.groupIDs { + gs, err := newGroupSigner(f, groupID) + if err != nil { + return nil, fmt.Errorf("integrity: %w", err) + } + s.signers = append(s.signers, gs) + } + + // Add signer(s) for each list of object IDs. + for _, ids := range so.objectIDs { + err := withGroupedObjects(f, ids, func(groupID uint32, ids []uint32) error { + gs, err := newGroupSigner(f, groupID, optSignGroupObjects(ids...)) + if err != nil { + return err + } + s.signers = append(s.signers, gs) + + return nil + }) + if err != nil { return nil, fmt.Errorf("integrity: %w", err) } } diff --git a/pkg/integrity/sign_test.go b/pkg/integrity/sign_test.go index 8cae9967..8881843f 100644 --- a/pkg/integrity/sign_test.go +++ b/pkg/integrity/sign_test.go @@ -484,164 +484,6 @@ func TestGroupSigner_SignWithEntity(t *testing.T) { } } -func TestOptSignGroup(t *testing.T) { - oneGroup, err := sif.LoadContainerFromPath( - filepath.Join("testdata", "images", "one-group.sif"), - sif.OptLoadWithFlag(os.O_RDONLY), - ) - if err != nil { - t.Fatal(err) - } - defer oneGroup.UnloadContainer() // nolint:errcheck - - tests := []struct { - name string - gid uint32 - wantErr error - }{ - { - name: "InvalidGroupID", - gid: 0, - wantErr: sif.ErrInvalidGroupID, - }, - { - name: "Group1", - gid: 1, - }, - { - name: "GroupNotFound", - gid: 2, - wantErr: errGroupNotFound, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - s := Signer{f: oneGroup} - - err := OptSignGroup(tt.gid)(&s) - if got, want := err, tt.wantErr; !errors.Is(got, want) { - t.Fatalf("got error %v, want %v", got, want) - } - - if err == nil { - if got, want := len(s.signers), 1; got != want { - t.Fatalf("got %v signers, want %v", got, want) - } - - if got, want := s.signers[0].id, tt.gid; got != want { - t.Errorf("got group ID %v, want %v", got, want) - } - } - }) - } -} - -func TestOptSignObjects(t *testing.T) { - tests := []struct { - name string - inputFileName string - ids []uint32 - wantErr error - wantGroupObjects map[uint32][]uint32 - }{ - { - name: "NoObjectsSpecified", - inputFileName: "empty.sif", - ids: []uint32{}, - wantErr: errNoObjectsSpecified, - }, - { - name: "InvalidObjectID", - inputFileName: "empty.sif", - ids: []uint32{0}, - wantErr: sif.ErrInvalidObjectID, - }, - { - name: "NoObjects", - inputFileName: "empty.sif", - ids: []uint32{1}, - wantErr: sif.ErrNoObjects, - }, - { - name: "ObjectNotFound", - inputFileName: "one-group.sif", - ids: []uint32{3}, - wantErr: sif.ErrObjectNotFound, - }, - { - name: "Duplicates", - inputFileName: "one-group.sif", - ids: []uint32{1, 1}, - wantGroupObjects: map[uint32][]uint32{1: {1}}, - }, - { - name: "Object1", - inputFileName: "one-group.sif", - ids: []uint32{1}, - wantGroupObjects: map[uint32][]uint32{1: {1}}, - }, - { - name: "Object2", - inputFileName: "one-group.sif", - ids: []uint32{2}, - wantGroupObjects: map[uint32][]uint32{1: {2}}, - }, - { - name: "Object3", - inputFileName: "two-groups.sif", - ids: []uint32{3}, - wantGroupObjects: map[uint32][]uint32{2: {3}}, - }, - { - name: "AllObjects", - inputFileName: "two-groups.sif", - ids: []uint32{1, 2, 3}, - wantGroupObjects: map[uint32][]uint32{1: {1, 2}, 2: {3}}, - }, - } - - for _, tt := range tests { - tt := tt - - t.Run(tt.name, func(t *testing.T) { - f, err := sif.LoadContainerFromPath( - filepath.Join("testdata", "images", tt.inputFileName), - sif.OptLoadWithFlag(os.O_RDONLY), - ) - if err != nil { - t.Fatal(err) - } - defer f.UnloadContainer() //nolint:errcheck - - s := Signer{f: f} - - err = OptSignObjects(tt.ids...)(&s) - if got, want := err, tt.wantErr; !errors.Is(got, want) { - t.Fatalf("got error %v, want %v", got, want) - } - - if err == nil { - for _, gs := range s.signers { - if want, ok := tt.wantGroupObjects[gs.id]; !ok { - t.Fatalf("unexpected signer for group ID %v", gs.id) - } else { - var got []uint32 - for _, od := range gs.ods { - got = append(got, od.ID()) - } - - if !reflect.DeepEqual(got, want) { - t.Errorf("got objects %v, want %v", got, want) - } - } - } - } - }) - } -} - func TestNewSigner(t *testing.T) { emptyImage, err := sif.LoadContainerFromPath( filepath.Join("testdata", "images", "empty.sif"), @@ -703,8 +545,14 @@ func TestNewSigner(t *testing.T) { wantErr: errNoObjectsSpecified, }, { - name: "InvalidObjectID", + name: "NoObjects", fi: emptyImage, + opts: []SignerOpt{OptSignObjects(1)}, + wantErr: sif.ErrNoObjects, + }, + { + name: "InvalidObjectID", + fi: oneGroupImage, opts: []SignerOpt{OptSignObjects(0)}, wantErr: sif.ErrInvalidObjectID, },