From 2063ae3f0345137bc38081aac090535eca3493ce Mon Sep 17 00:00:00 2001 From: Adam Hughes <9903835+tri-adam@users.noreply.github.com> Date: Fri, 8 Nov 2024 17:29:09 +0000 Subject: [PATCH] fix: corpus image generation Add OptSignWithoutPGPSignatureSalt, which disables randomization of signature generation, and use that in the corpus to generate images deterministically. Update corpus images and related golden files to reflect the signatures generated by the new version of go-crypto. --- .../TestApp_Info/DataSignature.golden | 2 +- .../TestApp_List/OneGroupSignedPGP.golden | 2 +- .../TestApp_List/TwoGroupsSignedPGP.golden | 4 +- pkg/integrity/clearsign.go | 15 +++----- pkg/integrity/clearsign_test.go | 4 +- pkg/integrity/sign.go | 35 ++++++++++++------ pkg/integrity/sign_test.go | 7 ++-- .../Test_command_getInfo/Three/out.golden | 2 +- .../OneGroupSignedPGP/out.golden | 2 +- .../TwoGroupsSignedPGP/out.golden | 4 +- test/images/gen_sifs.go | 3 +- test/images/one-group-signed-pgp.sif | Bin 42014 -> 42008 bytes test/images/two-groups-signed-pgp.sif | Bin 305013 -> 305001 bytes 13 files changed, 45 insertions(+), 35 deletions(-) diff --git a/internal/app/siftool/testdata/TestApp_Info/DataSignature.golden b/internal/app/siftool/testdata/TestApp_Info/DataSignature.golden index cb2859f7..6aa68e5b 100644 --- a/internal/app/siftool/testdata/TestApp_Info/DataSignature.golden +++ b/internal/app/siftool/testdata/TestApp_Info/DataSignature.golden @@ -3,6 +3,6 @@ Group ID: NONE Linked ID: 1 (G) Offset: 303104 - Size: 1054 + Size: 1048 Hash Type: SHA-256 Entity: 12045C8C0B1004D058DE4BEDA20C27EE7FF7BA84 diff --git a/internal/app/siftool/testdata/TestApp_List/OneGroupSignedPGP.golden b/internal/app/siftool/testdata/TestApp_List/OneGroupSignedPGP.golden index 5b663d3e..b03df369 100644 --- a/internal/app/siftool/testdata/TestApp_List/OneGroupSignedPGP.golden +++ b/internal/app/siftool/testdata/TestApp_List/OneGroupSignedPGP.golden @@ -3,4 +3,4 @@ ID |GROUP |LINK |SIF POSITION (start-end) |TYPE ------------------------------------------------------------------------------ 1 |1 |NONE |32768-32772 |FS (Raw/System/386) 2 |1 |NONE |36864-40960 |FS (Squashfs/*System/386) -3 |NONE |1 (G) |40960-42014 |Signature (SHA-256) +3 |NONE |1 (G) |40960-42008 |Signature (SHA-256) diff --git a/internal/app/siftool/testdata/TestApp_List/TwoGroupsSignedPGP.golden b/internal/app/siftool/testdata/TestApp_List/TwoGroupsSignedPGP.golden index 17240ba5..31875e98 100644 --- a/internal/app/siftool/testdata/TestApp_List/TwoGroupsSignedPGP.golden +++ b/internal/app/siftool/testdata/TestApp_List/TwoGroupsSignedPGP.golden @@ -4,5 +4,5 @@ ID |GROUP |LINK |SIF POSITION (start-end) |TYPE 1 |1 |NONE |32768-32772 |FS (Raw/System/386) 2 |1 |NONE |36864-40960 |FS (Squashfs/*System/386) 3 |2 |NONE |40960-303104 |FS (Ext3/System/amd64) -4 |NONE |1 (G) |303104-304158 |Signature (SHA-256) -5 |NONE |2 (G) |304158-305013 |Signature (SHA-256) +4 |NONE |1 (G) |303104-304152 |Signature (SHA-256) +5 |NONE |2 (G) |304152-305001 |Signature (SHA-256) diff --git a/pkg/integrity/clearsign.go b/pkg/integrity/clearsign.go index 4c60c440..a8aa7aa8 100644 --- a/pkg/integrity/clearsign.go +++ b/pkg/integrity/clearsign.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2020-2024, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the LICENSE.md file // distributed with the sources of this project regarding your rights to use or distribute this // software. @@ -11,7 +11,6 @@ import ( "crypto" "errors" "io" - "time" "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/clearsign" @@ -25,14 +24,12 @@ type clearsignEncoder struct { config *packet.Config } -// newClearsignEncoder returns an encoder that signs messages in clear-sign format using entity e. -// If timeFunc is not nil, it is used to generate signature timestamps. -func newClearsignEncoder(e *openpgp.Entity, timeFunc func() time.Time) *clearsignEncoder { +// newClearsignEncoder returns an encoder that signs messages in clear-sign format using entity e, +// according to config. +func newClearsignEncoder(e *openpgp.Entity, config *packet.Config) *clearsignEncoder { return &clearsignEncoder{ - e: e, - config: &packet.Config{ - Time: timeFunc, - }, + e: e, + config: config, } } diff --git a/pkg/integrity/clearsign_test.go b/pkg/integrity/clearsign_test.go index 0de5e321..e164f49a 100644 --- a/pkg/integrity/clearsign_test.go +++ b/pkg/integrity/clearsign_test.go @@ -39,12 +39,12 @@ func Test_clearsignEncoder_signMessage(t *testing.T) { }{ { name: "EncryptedKey", - en: newClearsignEncoder(encrypted, fixedTime), + en: newClearsignEncoder(encrypted, &packet.Config{Time: fixedTime}), wantErr: true, }, { name: "OK", - en: newClearsignEncoder(e, fixedTime), + en: newClearsignEncoder(e, &packet.Config{Time: fixedTime}), de: newClearsignDecoder(openpgp.EntityList{e}), wantHash: crypto.SHA256, }, diff --git a/pkg/integrity/sign.go b/pkg/integrity/sign.go index 002810a8..d6ed339f 100644 --- a/pkg/integrity/sign.go +++ b/pkg/integrity/sign.go @@ -17,6 +17,7 @@ import ( "time" "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/packet" "github.com/sigstore/sigstore/pkg/signature" "github.com/sylabs/sif/v2/pkg/sif" ) @@ -179,13 +180,14 @@ func (gs *groupSigner) sign(ctx context.Context) (sif.DescriptorInput, error) { } type signOpts struct { - ss []signature.Signer - e *openpgp.Entity - groupIDs []uint32 - objectIDs [][]uint32 - timeFunc func() time.Time - deterministic bool - ctx context.Context //nolint:containedctx + ss []signature.Signer + e *openpgp.Entity + groupIDs []uint32 + objectIDs [][]uint32 + timeFunc func() time.Time + deterministic bool + ctx context.Context //nolint:containedctx + withoutPGPSignatureSalt bool } // SignerOpt are used to configure so. @@ -257,6 +259,16 @@ func OptSignWithContext(ctx context.Context) SignerOpt { } } +// OptSignWithoutPGPSignatureSalt disables the addition of a salt notation for v4 and v5 PGP keys. +// While this increases determinism, it should be used with caution as the salt notation increases +// protection for certain kinds of attacks. +func OptSignWithoutPGPSignatureSalt() SignerOpt { + return func(so *signOpts) error { + so.withoutPGPSignatureSalt = true + return nil + } +} + // 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 { @@ -339,11 +351,10 @@ func NewSigner(f *sif.FileImage, opts ...SignerOpt) (*Signer, error) { case so.ss != nil: en = newDSSEEncoder(so.ss) case so.e != nil: - timeFunc := time.Now - if so.timeFunc != nil { - timeFunc = so.timeFunc - } - en = newClearsignEncoder(so.e, timeFunc) + en = newClearsignEncoder(so.e, &packet.Config{ + Time: so.timeFunc, + NonDeterministicSignaturesViaNotation: packet.BoolPointer(!so.withoutPGPSignatureSalt), + }) commonOpts = append(commonOpts, optSignGroupFingerprint(so.e.PrimaryKey.Fingerprint)) default: return nil, fmt.Errorf("integrity: %w", ErrNoKeyMaterial) diff --git a/pkg/integrity/sign_test.go b/pkg/integrity/sign_test.go index cb878aee..965a0609 100644 --- a/pkg/integrity/sign_test.go +++ b/pkg/integrity/sign_test.go @@ -16,6 +16,7 @@ import ( "testing" "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/packet" "github.com/sylabs/sif/v2/pkg/sif" ) @@ -195,7 +196,7 @@ func TestNewGroupSigner(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - en := newClearsignEncoder(getTestEntity(t), fixedTime) + en := newClearsignEncoder(getTestEntity(t), &packet.Config{Time: fixedTime}) s, err := newGroupSigner(en, tt.fi, tt.groupID, tt.opts...) if got, want := err, tt.wantErr; !errors.Is(got, want) { @@ -254,12 +255,12 @@ func TestGroupSigner_Sign(t *testing.T) { } e := getTestEntity(t) - clearsign := newClearsignEncoder(e, fixedTime) + clearsign := newClearsignEncoder(e, &packet.Config{Time: fixedTime}) encrypted := getTestEntity(t) encrypted.PrivateKey.Encrypted = true - clearsignEncrypted := newClearsignEncoder(encrypted, fixedTime) + clearsignEncrypted := newClearsignEncoder(encrypted, &packet.Config{Time: fixedTime}) tests := []struct { name string diff --git a/pkg/siftool/testdata/Test_command_getInfo/Three/out.golden b/pkg/siftool/testdata/Test_command_getInfo/Three/out.golden index bf5fddca..6be61ddc 100644 --- a/pkg/siftool/testdata/Test_command_getInfo/Three/out.golden +++ b/pkg/siftool/testdata/Test_command_getInfo/Three/out.golden @@ -3,6 +3,6 @@ Group ID: NONE Linked ID: 1 (G) Offset: 40960 - Size: 1054 + Size: 1048 Hash Type: SHA-256 Entity: 12045C8C0B1004D058DE4BEDA20C27EE7FF7BA84 diff --git a/pkg/siftool/testdata/Test_command_getList/OneGroupSignedPGP/out.golden b/pkg/siftool/testdata/Test_command_getList/OneGroupSignedPGP/out.golden index 5b663d3e..b03df369 100644 --- a/pkg/siftool/testdata/Test_command_getList/OneGroupSignedPGP/out.golden +++ b/pkg/siftool/testdata/Test_command_getList/OneGroupSignedPGP/out.golden @@ -3,4 +3,4 @@ ID |GROUP |LINK |SIF POSITION (start-end) |TYPE ------------------------------------------------------------------------------ 1 |1 |NONE |32768-32772 |FS (Raw/System/386) 2 |1 |NONE |36864-40960 |FS (Squashfs/*System/386) -3 |NONE |1 (G) |40960-42014 |Signature (SHA-256) +3 |NONE |1 (G) |40960-42008 |Signature (SHA-256) diff --git a/pkg/siftool/testdata/Test_command_getList/TwoGroupsSignedPGP/out.golden b/pkg/siftool/testdata/Test_command_getList/TwoGroupsSignedPGP/out.golden index 17240ba5..31875e98 100644 --- a/pkg/siftool/testdata/Test_command_getList/TwoGroupsSignedPGP/out.golden +++ b/pkg/siftool/testdata/Test_command_getList/TwoGroupsSignedPGP/out.golden @@ -4,5 +4,5 @@ ID |GROUP |LINK |SIF POSITION (start-end) |TYPE 1 |1 |NONE |32768-32772 |FS (Raw/System/386) 2 |1 |NONE |36864-40960 |FS (Squashfs/*System/386) 3 |2 |NONE |40960-303104 |FS (Ext3/System/amd64) -4 |NONE |1 (G) |303104-304158 |Signature (SHA-256) -5 |NONE |2 (G) |304158-305013 |Signature (SHA-256) +4 |NONE |1 (G) |303104-304152 |Signature (SHA-256) +5 |NONE |2 (G) |304152-305001 |Signature (SHA-256) diff --git a/test/images/gen_sifs.go b/test/images/gen_sifs.go index 61d24be8..701fc679 100755 --- a/test/images/gen_sifs.go +++ b/test/images/gen_sifs.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2020-2024, Sylabs Inc. All rights reserved. // This software is licensed under a 3-clause BSD license. Please consult the LICENSE.md file // distributed with the sources of this project regarding your rights to use or distribute this // software. @@ -294,6 +294,7 @@ func generateImages() error { opts = append(opts, integrity.OptSignWithTime(func() time.Time { return time.Date(2020, 6, 30, 0, 1, 56, 0, time.UTC) }), integrity.OptSignDeterministic(), + integrity.OptSignWithoutPGPSignatureSalt(), ) s, err := integrity.NewSigner(f, opts...) diff --git a/test/images/one-group-signed-pgp.sif b/test/images/one-group-signed-pgp.sif index b10950331c00ef4f984a288d13cbf61b444f0530..9c7825828856814bf7cbe7caef8981cae7ba6848 100755 GIT binary patch delta 425 zcmW;3yKb9M007X+@=}&ots>+RN-YX1YaHo%o26U7O9e# zP8~8fTk;e6tSCL_@7v_h+vN8!Nq@ZlI(zu?^7VO&oKHXfcRoE8-~PS+Jexj#_LLai zFL1#>6;BuU+=kdDn7xnWvgl$ahZzjlYk_o9U8IV>(9B)oC)r%^RLzstB$|5hZK^8UW9ia2 zgnN%H<0iWs>|N}e;qn^A>0UpU0Ju>C`i$`A@N%dzF4rOn+E&_+tuVhJ>#^=YtaQ4~w1spNPz*2$1#_tD?bIyXgbgNW} zrBjEhQ@b^PB0sBYzwg^i@B2&dE9vF$pFdrHe*5R@cb~fHzxwZ{e@Wl{dj5Fb|MF&T z9tH(sl>1Bi^B9Q(OUi_-8EUz0=R=Tj)!Ict3Zy|BHej;} zN-80YYi)7O(EKbaG9G$j=kNCSOe38%1}K@*oj?i+DaTIjI~7N!I$!FW(2oQ49D;^& zOnN;yiWWblp^1HDOY>cIIHIC1g$JwT^X?>^wmaQ&to_g$&m4{uy;bw%g{sH99;T8g yEFxKI94FR1xg9QWzS(% diff --git a/test/images/two-groups-signed-pgp.sif b/test/images/two-groups-signed-pgp.sif index d21497185e98ab2a256bd2d8e685426fbd9d6ee4..35ba895fca26598b833f31f71f54e869c25507df 100755 GIT binary patch delta 843 zcmZXQIgjIH0EXF_2x;8_Q>;d-6+#q9$U;8iBQCbE8+xJ?#efsOg7dKCzz5LGq`ucc%_ISN{yx#tNop_%=x7Puj z^XS}p1Adr%^}rtRQ;3+kFJ@gQ?-K$hX`@lKLYcd%R|jA$OA0@iMcJwwy>MRksTZ(X??n6 z7PY~x2WT18aXOgiz|pgq@Mxd@EVztzIu%>%7Osy@KDHlWhQ| z0CJACt2^b{K;n6!kj*Tdc9N~q5>02OsIaVvF!YulB#y6yDhO2u;S_n_eq;YQdG*VuZztbB`(OW@)idn35^sEf&Kme{lBUF6kPZ%&YK8LooxeU4mEc=*_Y!Cf929snNgIlz-FL)DhE14QSfnnudP5MQ%UeVH0PT)(#yByh?^Z>efpEWTa5U)) zoxOMoO&Y~fGfqiGVdn+!M^4$ew#JE52Y^(NQzeAd95=95vUUiu@n!kII_eN9V4*Q= z&YeJ^ouj=-v_iy55t{lL<%0g4RGW*H;7w5VFoY#Iz%iV$R{4}Djjps9d!?kJ$YZj^ zVTc!v`Q9b|dCp5|<}oF@Nno?2*#vLnjYdMrCYs?+<9K=%W+O8wP2}1a6gZryH~;nj E8~+p%@&Et; delta 855 zcmZ9}IgjIX0ETgPCPG>_z!WRALL)@6(#k^4I6jLh>^P3k#77)E?tCY{uQ-3FnFxug zktnvITtv@EP*KwE_n?OogxKwPyXS3xf4sZ->)p+F;QII1lc&#Ke*4MKpZ@vr^V`Ws z?|1w3_Y3Ok=JxG}wy!_j_Vv?0FJ4_eefjeH@SDry<=Nxq>hW^@%ViRL^<0-CxZ(^7)IGnf3}l_O<1-DWrw6X zuyk*lDQ~mGY@?Nq3rwk!&LpSb-$VCgFKBPnpC(`Y`uW?*56}LuFgH;qMqJCuy#>yc z@m7K(7_^~!mLnN2?<(I8mS|*~4Rt`^+;-xWYBruLb(>M|AfIg*t?s03UD`9U3f6T! zEJbcC`gp)GGZ^#`wsQ%|3p(gs=?o{ly)tJd6~!Xpz2yTGvZPbMbhzTkNDX%NalRf= zue(#62&8C^Z>B&31KYyWWjqG4i7oi^-dQnAwP6SnRFEgq=aCS*j`wNqWNp=!F-lBI z*6z+TaH$UW#FnLGHqqt0BC@o^{n}KiwvYv$&M1?G5F$`H9|M^!EVOm88Le^|`cB;k zD@A7jitsMS*3ofTMLWzSXkMpI1TQ0O&Qlp-AdnYJnM8o1Z4IUL4ENAz%Fa>6&d`nB N=|6=?I9l%&{skpO6jT5J