fix multi key support in APKINDEX verification (#1490)

The current logic uses the last .SIGN. file content as signature and
verify that against all known keys. This PR fixes the logic to use
matching signature to the right verification key.

---------

Signed-off-by: Nghia Tran <[email protected]>
Signed-off-by: Nghia Tran <[email protected]>

pkg/apk/apk/apkindex_test.go

@@ -3,6 +3,7 @@ package apk
 import (
 	"archive/tar"
 	"compress/gzip"
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -238,3 +239,52 @@ k:9001
 	require.Len(t, pkg.Provides, 0, "Expected no provides")
 	require.Len(t, pkg.Dependencies, 0, "Expected no dependencies")
 }
+
+func TestMultipleKeys(t *testing.T) {
+	assert := assert.New(t)
+	// read all the keys from testdata/signing/keys
+	folder := "testdata/signing/keys"
+	// get all the files in the folder
+	files, _ := os.ReadDir(folder)
+	keys := make(map[string][]byte)
+	for _, file := range files {
+		if file.IsDir() {
+			continue
+		}
+		// read the file
+		keyFile, err := os.Open(fmt.Sprintf("%s/%s", folder, file.Name()))
+		require.Nil(t, err)
+		// parse the key
+		key, err := os.ReadFile(keyFile.Name())
+		require.Nil(t, err)
+		keys[file.Name()] = key
+	}
+	// read the index file into []byte
+	indexBytes, err := os.ReadFile("testdata/signing/APKINDEX.tar.gz")
+	require.Nil(t, err)
+
+	ctx := context.Background()
+	// There are 2^N-1 combinations of keys, where N is the number of keys
+	// We will test all of them
+	for comb := 1; comb < (1 << len(keys)); comb++ {
+		// get the keys to use
+		usedKeys := make(map[string][]byte)
+		for i := 0; i < len(keys); i++ {
+			if (comb & (1 << i)) != 0 {
+				usedKeys[files[i].Name()] = keys[files[i].Name()]
+			}
+		}
+		// parse the index
+		apkIndex, err := parseRepositoryIndex(ctx, "testdata/signing/APKINDEX.tar.gz",
+			usedKeys, "aarch64", indexBytes, &indexOpts{})
+		require.Nil(t, err)
+		assert.Greater(len(apkIndex.Signature), 0, "Signature missing")
+	}
+	// Now, test the case where we have no matching key
+	_, err = parseRepositoryIndex(ctx, "testdata/signing/APKINDEX.tar.gz",
+		map[string][]byte{
+			"unused-key": []byte("unused-key-data"),
+		},
+		"aarch64", indexBytes, &indexOpts{})
+	require.NotNil(t, err)
+}

pkg/apk/apk/index.go

@@ -44,6 +44,12 @@ import (
 
 var signatureFileRegex = regexp.MustCompile(`^\.SIGN\.(DSA|RSA|RSA256|RSA512)\.(.*\.rsa\.pub)$`)
 
+type Signature struct {
+	KeyID           string
+	Signature       []byte
+	DigestAlgorithm crypto.Hash
+}
+
 // This is terrible but simpler than plumbing around a cache for now.
 // We just hold the parsed index in memory rather than re-parsing it every time,
 // which requires gunzipping, which is (somewhat) expensive.
@@ -332,9 +338,11 @@ func fetchRepositoryIndex(ctx context.Context, u string, etag string, opts *inde
 func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, arch string, b []byte, opts *indexOpts) (*APKIndex, error) { //nolint:gocyclo
 	_, span := otel.Tracer("go-apk").Start(ctx, "parseRepositoryIndex")
 	defer span.End()
-
 	// validate the signature
 	if shouldCheckSignatureForIndex(u, arch, opts) {
+		if len(keys) == 0 {
+			return nil, fmt.Errorf("no keys provided to verify signature")
+		}
 		buf := bytes.NewReader(b)
 		gzipReader, err := gzip.NewReader(buf)
 		if err != nil {
@@ -348,14 +356,12 @@ func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte,
 
 		tarReader := tar.NewReader(gzipReader)
 
-		var keyfile string
-		var signature []byte
-		var indexDigestType crypto.Hash
+		sigs := make([]Signature, 0, len(keys))
 		for {
 			// read the signature(s)
 			signatureFile, err := tarReader.Next()
 			// found everything, end of stream
-			if len(keyfile) > 0 && errors.Is(err, io.EOF) {
+			if errors.Is(err, io.EOF) {
 				break
 			}
 			// oops something went wrong
@@ -366,61 +372,67 @@ func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte,
 			if len(matches) != 3 {
 				return nil, fmt.Errorf("failed to find key name in signature file name: %s", signatureFile.Name)
 			}
-			// It is lucky that golang only iterates over sorted file names, and that
-			// lexically latest is the strongest hash
+			keyfile := matches[2]
+			if _, ok := keys[keyfile]; !ok {
+				// Ignore this signature if we don't have the key
+				continue
+			}
+			var digestAlgorithm crypto.Hash
 			switch signatureType := matches[1]; signatureType {
 			case "DSA":
 				// Obsolete
 				continue
 			case "RSA":
 				// Current legacy compat
-				indexDigestType = crypto.SHA1
+				digestAlgorithm = crypto.SHA1
 			case "RSA256":
 				// Current best practice
-				indexDigestType = crypto.SHA256
+				digestAlgorithm = crypto.SHA256
 			case "RSA512":
 				// Too big, too slow, not compiled in
 				continue
 			default:
 				return nil, fmt.Errorf("unknown signature format: %s", signatureType)
 			}
-			keyfile = matches[2]
-			signature, err = io.ReadAll(tarReader)
+			signature, err := io.ReadAll(tarReader)
 			if err != nil {
 				return nil, fmt.Errorf("failed to read signature from repository index: %w", err)
 			}
+			sigs = append(sigs, Signature{
+				KeyID:           keyfile,
+				Signature:       signature,
+				DigestAlgorithm: digestAlgorithm,
+			})
+		}
+		if len(sigs) == 0 {
+			return nil, fmt.Errorf("no signature with known key found in repository index")
 		}
 		// we now have the signature bytes and name, get the contents of the rest;
 		// this should be everything else in the raw gzip file as is.
 		allBytes := len(b)
 		unreadBytes := buf.Len()
 		readBytes := allBytes - unreadBytes
 		indexData := b[readBytes:]
-		indexDigest, err := sign.HashData(indexData, indexDigestType)
-		if err != nil {
-			return nil, err
-		}
-		// now we can check the signature
-		if keys == nil {
-			return nil, fmt.Errorf("no keys provided to verify signature")
-		}
-		var verified bool
-		keyData, ok := keys[keyfile]
-		if ok {
-			if err := sign.RSAVerifyDigest(indexDigest, indexDigestType, signature, keyData); err != nil {
-				verified = false
-			}
-		}
-		if !verified {
-			for _, keyData := range keys {
-				if err := sign.RSAVerifyDigest(indexDigest, indexDigestType, signature, keyData); err == nil {
-					verified = true
-					break
+		indexDigest := make(map[crypto.Hash][]byte, len(keys))
+		verified := false
+		for _, sig := range sigs {
+			// compute the digest if not already done
+			if _, hasDigest := indexDigest[sig.DigestAlgorithm]; !hasDigest {
+				digest, err := sign.HashData(indexData, sig.DigestAlgorithm)
+				if err != nil {
+					return nil, fmt.Errorf("failed to compute digest: %w", err)
 				}
+				indexDigest[sig.DigestAlgorithm] = digest
+			}
+			if err := sign.RSAVerifyDigest(indexDigest[sig.DigestAlgorithm], sig.DigestAlgorithm, sig.Signature, keys[sig.KeyID]); err == nil {
+				verified = true
+				break
+			} else {
+				clog.FromContext(ctx).Warnf("failed to verify signature for keyfile %s: %v", sig.KeyID, err)
 			}
 		}
 		if !verified {
-			return nil, fmt.Errorf("no key found to verify signature for keyfile %s; tried all other keys as well", keyfile)
+			return nil, errors.New("signature verification failed for repository index, for all provided keys")
 		}
 	}
 	// with a valid signature, convert it to an ApkIndex

pkg/apk/apk/testdata/signing/keys/chainguard-0106f58bac88057c2ff5c2829850df492717a876ed700443550353c7ab23f5a0.rsa.pub

@@ -0,0 +1,14 @@
+-----BEGIN PUBLIC KEY-----
+MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA2TxPZPC1Cdb647P0kvi5
+wtwcoaFCOGHspL3RmmD2idG2FNZc/rpwyIp2bnprFvOCWm7bKAEl/9JDfAGm7DAP
+q/EC7UeJ9yx03jY1I7+Oj1/1UUIyuvfDwyiMxEwcABaivAn9WhYZ5YeZKK8pUYoX
+hOPNqgI+N3BY1H+hgg3xgTrzWmfRSsvTEhSqoLHdl4qbsM5EE/T5W7u+96y9J2vn
+M89rUP557K3n1QKGzTIGjlesFQd8y+uH0TdAe6AFmxFpftdXRu4KInjdrDOArfBO
+cKLB0gHOERkd08JVMWYHd6Ne7BfWpwxmge371GgTj8XYOJrkAj3cg8ked/51Maw0
+FLt4aErQR6y02QIluGZ4gwbIZ8y+iBWvoh8egiab3Whr3SwGuX6X8WJbOgKLCyGK
+CmrxgPO/ahS4XUWtuTp3woIZBnnHGOBdHjRARkl9PdBgN2HhZPS9vj8rrIqbTp3c
+dmqhcsjsgmss6Sb3rOcOL3N3V8+dMiD7VZXeIgVocOpRtLJGQFCupjAfmz5ub4t5
+lBI2EgbR9LdSg9mrFoYbG11sU6MLa0iPMA8gU3nG4dzLDpsF9PbXy6sfBXXc8W25
+tfwZnV4wHouWEhiQ9Hfc4NOGcs+01Zwg9CZRS2SBKC9iqUPsz3RaTAKWsS/QNTND
+Zy9znhL5yGrbHaq2BOlnVqMCAwEAAQ==
+-----END PUBLIC KEY-----

pkg/apk/apk/testdata/signing/keys/chainguard-60912bbc46bfc8ed6bda0b50db3a8a5f3c4344d6bc8549ec9b84d96140b475d1.rsa.pub

@@ -0,0 +1,14 @@
+-----BEGIN PUBLIC KEY-----
+MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAizNP/QKfB5NKlVBba6dX
+Jzz0/icPE++eM3aAtCARwlAKyRm3ivWM5sAz88lwHFjMAQV0QPD3hDgswcAExTXa
+EPN8M0RroxxnWuJLGhz99ONaGOLo66GDkcdBaaZIinXoWmhqg1zcequ6dLDOGArm
+ZR99egaBFKyQHvBYHIl5dQ1yT8us/m1Yfb7DlERcU2lf3lFhLDrJsBgVw5QYa5h+
+aSneA/B2ZB1BwfyE3F5+UUn0qWRy3Yg526oaWpjjwCaPVq4yaJfg4XuMS82wdVm9
+G6awP1PE0EWwgkcc8z8w7BmxFhdCRt0VNjmept14tJwiU0d81j1HPHsIkqWr2TBA
+f7Wwp34+ByfA7iHbKXFPgK1MKmNzyANoyPaPRo+x+MU8D2AqUBslKOKrhckyd4tQ
+ch3SyaMKMa7hc0PWnFrCCtDTmNZQaquNrUWnbbMtdkjCA/is//W+Lqn+O6BjmR9U
+yztR26l2OwGMu6uf3CMUADX/ra4k2BuxSnaPTgm7M19AAzXSnICa+aupgFsIfx1w
+UyGlq8dvCz8bnipXW4EbTWTSPdFrxQl/LQUB5w82kMorML1TQEpSLjoSmnhNg+rh
+rqIsjp7fwEnX/qTbiwUhgcN1fyLkGGv/UV/H62WcclStKiRHp+ND0ePMABefA+cv
+f9tYynbOV78qct+dQKkQ17kCAwEAAQ==
+-----END PUBLIC KEY-----

pkg/apk/apk/testdata/signing/keys/chainguard-61cb6bccd1f584b007db7be51ebce2b0530a54cc5a94e7650b570d113d537cf3.rsa.pub

@@ -0,0 +1,14 @@
+-----BEGIN PUBLIC KEY-----
+MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAwlV2kG9xGtkVWc/dS9nC
+Qqef/hx0AysYfrMFetY+mZ0MPvELqOaDJgtjlbMM6w/ujqDtScxbnNNf5/vY/wBh
+rHTeKU+oDlkRTsh8ZxKINFfGGSDxR2pKkvPZeVzxdhTNEPK9ZggbFHe5RhXUT9iK
+CNgF+Xa3p5A1Yi8zR6zEFJ5EDBMnTMagV2ueLfJrpFMm/4hBn6zWtjyDhDPVZyTD
+DyQc2/FIDXM1tJONWDT8wO2xuHf7xKGr6lO/CZca5Pnd/QS5jODoNbTo9VvG+oQt
+mRsWsdVFst0vS54s3mchtuAB2NXnWAZZuux9uYPgvO8eqI0RLqCKX7VLJ8tGOTmL
+3dblTzKbKbVcy4uHsyH2mZPvPUTO+ck7c58iSeUrOFV50B6zqynAXgCrDo3XMjpI
+mLC26LRMqF/Q/RWc9R0K+ZgiLQ7ootyOJILPSR69RhgdQJVt7/gz55dM89yoq/B9
+U2V+1mv0rBQmmNgCM1U3QdgUv5WeIm4Ed2avKZBwJLBVR5AxlY9xlwJ7sEU4Ms2t
+cEblWhqDze/sSsjCYOAmD4/M/90OnBelg+/BU2jOD4nqQdEQDOZdo+EUVeCIuPwD
+kz8kTv8pPtLjKI0jRhYLn2dhg/vTIDJf19iXexGQXOyK/rNwi41i+9snypVb8YSe
+cWTnq+m2CFOiXFsDWQh7RsUCAwEAAQ==
+-----END PUBLIC KEY-----