From e3d02ab2e1210ef283e273b11e27ce3e123e65a3 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 10:28:01 +0100 Subject: [PATCH 1/6] Simplify and rename hash functions --- internal/util/crypto.go | 9 +++------ pkg/attestation/sign.go | 2 +- pkg/attestation/verify.go | 2 +- pkg/signerverifier/common.go | 2 +- pkg/signerverifier/keyid.go | 2 +- pkg/tlog/tl.go | 3 +-- pkg/tlog/tl_test.go | 2 +- pkg/tuf/registry_test.go | 2 +- pkg/tuf/tuf.go | 2 +- 9 files changed, 11 insertions(+), 15 deletions(-) diff --git a/internal/util/crypto.go b/internal/util/crypto.go index 497655f..f28d63a 100644 --- a/internal/util/crypto.go +++ b/internal/util/crypto.go @@ -5,14 +5,11 @@ import ( "encoding/hex" ) -func HexHashBytes(input []byte) string { - s256 := sha256.New() - s256.Write(input) - hashSum := s256.Sum(nil) - return hex.EncodeToString(hashSum) +func SHA256Hex(input []byte) string { + return hex.EncodeToString(SHA256(input)) } -func S256(data []byte) []byte { +func SHA256(data []byte) []byte { h := sha256.Sum256(data) return h[:] } diff --git a/pkg/attestation/sign.go b/pkg/attestation/sign.go index 1af1a9d..f5efc89 100644 --- a/pkg/attestation/sign.go +++ b/pkg/attestation/sign.go @@ -19,7 +19,7 @@ func SignDSSE(ctx context.Context, payload []byte, payloadType string, signer ds encPayload := dsse.PAE(payloadType, payload) // statement message digest - hash := util.S256(encPayload) + hash := util.SHA256(encPayload) // sign message digest sig, err := signer.Sign(ctx, hash) diff --git a/pkg/attestation/verify.go b/pkg/attestation/verify.go index bb3ffce..42471db 100644 --- a/pkg/attestation/verify.go +++ b/pkg/attestation/verify.go @@ -121,7 +121,7 @@ func verifySignature(ctx context.Context, sig Signature, payload []byte, keys Ke return fmt.Errorf("error failed to decode signature: %w", err) } // verify payload ecdsa signature - ok = ecdsa.VerifyASN1(publicKey, util.S256(payload), signature) + ok = ecdsa.VerifyASN1(publicKey, util.SHA256(payload), signature) if !ok { return fmt.Errorf("payload signature is not valid") } diff --git a/pkg/signerverifier/common.go b/pkg/signerverifier/common.go index 95c8554..75e9212 100644 --- a/pkg/signerverifier/common.go +++ b/pkg/signerverifier/common.go @@ -38,7 +38,7 @@ func (s *ECDSA256_SignerVerifier) Verify(ctx context.Context, data []byte, sig [ if !ok { return fmt.Errorf("public key is not ecdsa") } - ok = ecdsa.VerifyASN1(pub, util.S256(data), sig) + ok = ecdsa.VerifyASN1(pub, util.SHA256(data), sig) if !ok { return fmt.Errorf("payload signature is not valid") } diff --git a/pkg/signerverifier/keyid.go b/pkg/signerverifier/keyid.go index b38a934..4bd0c4a 100644 --- a/pkg/signerverifier/keyid.go +++ b/pkg/signerverifier/keyid.go @@ -13,5 +13,5 @@ func KeyID(pubKey crypto.PublicKey) (string, error) { if err != nil { return "", fmt.Errorf("error marshalling public key: %w", err) } - return util.HexHashBytes(pub), nil + return util.SHA256Hex(pub), nil } diff --git a/pkg/tlog/tl.go b/pkg/tlog/tl.go index 8c30ec0..8bddeab 100644 --- a/pkg/tlog/tl.go +++ b/pkg/tlog/tl.go @@ -7,7 +7,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" - "encoding/hex" "encoding/pem" "fmt" "math/big" @@ -214,7 +213,7 @@ func (tl *RekorTL) VerifyEntryPayload(entryBytes, payload, publicKey []byte) err } // compare payload hashes - payloadHash := hex.EncodeToString(util.S256(payload)) + payloadHash := util.SHA256Hex(payload) if rekord.Hash != payloadHash { return fmt.Errorf("error payload and tl entry hash mismatch") } diff --git a/pkg/tlog/tl_test.go b/pkg/tlog/tl_test.go index 3354e3e..a51c3c9 100644 --- a/pkg/tlog/tl_test.go +++ b/pkg/tlog/tl_test.go @@ -44,7 +44,7 @@ func TestCreateX509Cert(t *testing.T) { func TestUploadAndVerifyLogEntry(t *testing.T) { // message digest payload := []byte("test") - hash := util.S256(payload) + hash := util.SHA256(payload) // generate ephemeral keys to sign message digest signer, err := signerverifier.GenKeyPair() diff --git a/pkg/tuf/registry_test.go b/pkg/tuf/registry_test.go index 2ab176a..cb8311a 100644 --- a/pkg/tuf/registry_test.go +++ b/pkg/tuf/registry_test.go @@ -121,7 +121,7 @@ func TestFindFileInManifest(t *testing.T) { // make test image manifest file := "test.json" data := []byte("test") - hash := v1.Hash{Algorithm: "sha256", Hex: util.HexHashBytes(data)} + hash := v1.Hash{Algorithm: "sha256", Hex: util.SHA256Hex(data)} img := empty.Image img = mutate.MediaType(img, types.OCIManifestSchema1) img = mutate.ConfigMediaType(img, types.OCIConfigJSON) diff --git a/pkg/tuf/tuf.go b/pkg/tuf/tuf.go index 48d0711..00808c5 100644 --- a/pkg/tuf/tuf.go +++ b/pkg/tuf/tuf.go @@ -44,7 +44,7 @@ func NewTufClient(initialRoot []byte, tufPath, metadataSource, targetsSource str tufSource = OciSource } - tufRootDigest := util.HexHashBytes(initialRoot) + tufRootDigest := util.SHA256Hex(initialRoot) // create a directory for each initial root.json metadataPath := filepath.Join(tufPath, tufRootDigest) From c69a9586c5da5253927a405c346033fa491a91d0 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 10:28:19 +0100 Subject: [PATCH 2/6] Remove string contains func (it's in the stdlib) --- internal/util/strings.go | 10 ---------- pkg/policy/policy.go | 4 ++-- 2 files changed, 2 insertions(+), 12 deletions(-) delete mode 100644 internal/util/strings.go diff --git a/internal/util/strings.go b/internal/util/strings.go deleted file mode 100644 index b0320b9..0000000 --- a/internal/util/strings.go +++ /dev/null @@ -1,10 +0,0 @@ -package util - -func StringInSlice(str string, list []string) bool { - for _, v := range list { - if v == str { - return true - } - } - return false -} diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index ceb8de3..f45ecf7 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -6,10 +6,10 @@ import ( "os" "path" "path/filepath" + "slices" "strings" "github.com/distribution/reference" - "github.com/docker/attest/internal/util" "github.com/docker/attest/pkg/oci" "github.com/docker/attest/pkg/tuf" @@ -148,7 +148,7 @@ func findPolicyMatch(named reference.Named, mappings *PolicyMappings) (*PolicyMa } // now search mirrors for _, mirror := range mappings.Mirrors { - if util.StringInSlice(reference.Domain(named), mirror.Mirror.Domains) && + if slices.Contains(mirror.Mirror.Domains, reference.Domain(named)) && strings.HasPrefix(reference.Path(named), mirror.Mirror.Prefix) { for _, mapping := range mappings.Policies { if mapping.Name == mirror.Name { From da22f712078c0b15bc310a4b88e12cedb5ea1b41 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 11:08:22 +0100 Subject: [PATCH 3/6] Use maps.Clone from stdlib --- pkg/attestation/attestation.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/attestation/attestation.go b/pkg/attestation/attestation.go index 81e3f5e..ece2f48 100644 --- a/pkg/attestation/attestation.go +++ b/pkg/attestation/attestation.go @@ -3,6 +3,7 @@ package attestation import ( "encoding/json" "fmt" + "maps" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/partial" @@ -64,10 +65,7 @@ func GetAttestationsFromImage(image v1.Image) ([]AttestationLayer, error) { return nil, fmt.Errorf("failed to get descriptor for layer: %w", err) } // copy original annotations - ann := make(map[string]string) - for k, v := range layerDesc.Annotations { - ann[k] = v - } + ann := maps.Clone(layerDesc.Annotations) // only decode intoto statements var stmt = new(intoto.Statement) if mt == types.MediaType(intoto.PayloadType) { From 8d45522fe848e7d0672c09e573a021adfa9f95c6 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 11:11:14 +0100 Subject: [PATCH 4/6] Use assert.NoError for nil checks on errors --- pkg/mirror/metadata_test.go | 18 +++++++++--------- pkg/mirror/targets_test.go | 20 ++++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/mirror/metadata_test.go b/pkg/mirror/metadata_test.go index 2f9ed6c..a34e8d7 100644 --- a/pkg/mirror/metadata_test.go +++ b/pkg/mirror/metadata_test.go @@ -21,10 +21,10 @@ func TestGetTufMetadataMirror(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") m, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) tufMetadata, err := m.getTufMetadataMirror(server.URL + "/metadata") - assert.Nil(t, err) + assert.NoError(t, err) // check that all roles are not empty assert.Greater(t, len(tufMetadata.Root), 0) @@ -39,15 +39,15 @@ func TestGetMetadataManifest(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") m, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) img, err := m.GetMetadataManifest(server.URL + "/metadata") - assert.Nil(t, err) + assert.NoError(t, err) assert.NotNil(t, img) image := *img mf, err := image.RawManifest() - assert.Nil(t, err) + assert.NoError(t, err) type Annotations struct { Annotations map[string]string `json:"annotations"` @@ -57,7 +57,7 @@ func TestGetMetadataManifest(t *testing.T) { } l := &Layers{} err = json.Unmarshal(mf, l) - assert.Nil(t, err) + assert.NoError(t, err) // check that layers are annotated and use consistent snapshot naming for _, layer := range l.Layers { @@ -69,7 +69,7 @@ func TestGetMetadataManifest(t *testing.T) { continue } _, err := strconv.Atoi(parts[0]) - assert.Nil(t, err) + assert.NoError(t, err) } } @@ -79,10 +79,10 @@ func TestGetDelegatedMetadataMirrors(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") m, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) delegations, err := m.GetDelegatedMetadataMirrors() - assert.Nil(t, err) + assert.NoError(t, err) assert.NotNil(t, delegations) assert.Greater(t, len(delegations), 0) diff --git a/pkg/mirror/targets_test.go b/pkg/mirror/targets_test.go index 117063a..3b544da 100644 --- a/pkg/mirror/targets_test.go +++ b/pkg/mirror/targets_test.go @@ -27,22 +27,22 @@ func TestGetTufTargetsMirror(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") m, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) targets, err := m.GetTufTargetMirrors() - assert.Nil(t, err) + assert.NoError(t, err) assert.Greater(t, len(targets), 0) // check for image layer annotations for _, target := range targets { img := *target.Image mf, err := img.RawManifest() - assert.Nil(t, err) + assert.NoError(t, err) // unmarshal manifest with annotations l := &Layers{} err = json.Unmarshal(mf, l) - assert.Nil(t, err) + assert.NoError(t, err) // check that layers are annotated for _, layer := range l.Layers { @@ -61,10 +61,10 @@ func TestTargetDelegationMetadata(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") tm, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) targets, err := tm.TufClient.LoadDelegatedTargets("test-role", "targets") - assert.Nil(t, err) + assert.NoError(t, err) assert.Greater(t, len(targets.Signed.Targets), 0) } @@ -74,22 +74,22 @@ func TestGetDelegatedTargetMirrors(t *testing.T) { path := test.CreateTempDir(t, "", "tuf_temp") m, err := NewTufMirror(embed.DevRoot, path, server.URL+"/metadata", server.URL+"/targets") - assert.Nil(t, err) + assert.NoError(t, err) mirrors, err := m.GetDelegatedTargetMirrors() - assert.Nil(t, err) + assert.NoError(t, err) assert.Greater(t, len(mirrors), 0) // check for index image annotations for _, mirror := range mirrors { idx := *mirror.Index mf, err := idx.RawManifest() - assert.Nil(t, err) + assert.NoError(t, err) // unmarshal manifest with annotations l := &Layers{} err = json.Unmarshal(mf, l) - assert.Nil(t, err) + assert.NoError(t, err) // check that layers are annotated for _, layer := range l.Layers { From bd849d9b43e3f81fb85294dd9efca844ef951703 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 11:12:14 +0100 Subject: [PATCH 5/6] Simplify some string concats --- pkg/mirror/targets.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/mirror/targets.go b/pkg/mirror/targets.go index b720e84..78c86d0 100644 --- a/pkg/mirror/targets.go +++ b/pkg/mirror/targets.go @@ -35,7 +35,7 @@ func (m *TufMirror) GetTufTargetMirrors() ([]*MirrorImage, error) { if !ok { return nil, fmt.Errorf("missing sha256 hash for target %s", t.Path) } - name := strings.Join([]string{hash.String(), t.Path}, ".") + name := hash.String() + "." + t.Path ann := map[string]string{tufFileAnnotation: name} layer := mutate.Addendum{Layer: static.NewLayer(data, tufTargetMediaType), Annotations: ann} img, err = mutate.Append(img, layer) @@ -86,7 +86,7 @@ func (m *TufMirror) GetDelegatedTargetMirrors() ([]*MirrorIndex, error) { if !ok { return nil, fmt.Errorf("failed to find target subdirectory [%s] in path: %s", subdir, target.Path) } - name := strings.Join([]string{hash.String(), filename}, ".") + name := hash.String() + "." + filename ann := map[string]string{tufFileAnnotation: name} layer := mutate.Addendum{Layer: static.NewLayer(data, tufTargetMediaType), Annotations: ann} img, err = mutate.Append(img, layer) From bd6d130e1783af8dea005243a6939d57fbaa507a Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Wed, 8 May 2024 13:12:40 +0100 Subject: [PATCH 6/6] Don't use builtin print function --- pkg/attest/example_verify_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/attest/example_verify_test.go b/pkg/attest/example_verify_test.go index a221073..7e80bff 100644 --- a/pkg/attest/example_verify_test.go +++ b/pkg/attest/example_verify_test.go @@ -2,6 +2,7 @@ package attest_test import ( "context" + "fmt" "os" "path/filepath" @@ -63,9 +64,9 @@ func ExampleVerify_remote() { panic(err) // failed policy or attestation signature verification } if policy { - print("policy passed: %v\n", policy) + fmt.Printf("policy passed: %v\n", policy) return // passed policy } // no policy found for image - print("no policy for image") + fmt.Printf("no policy for image") }