Use loadBytesFromConfigSources in prSignedBy.isSignatureAuthorAccepted

Extend loadBytesFromConfigSources to return multiple values, and to support
reading the from files; then share the code.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

signature/policy_eval_signedby.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"slices"
 
 	"github.com/containers/image/v5/internal/multierr"
@@ -27,33 +26,18 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva
 	}
 
 	// FIXME: move this to per-context initialization
-	var data [][]byte
-	keySources := 0
-	if pr.KeyPath != "" {
-		keySources++
-		d, err := os.ReadFile(pr.KeyPath)
-		if err != nil {
-			return sarRejected, nil, err
-		}
-		data = [][]byte{d}
-	}
-	if pr.KeyPaths != nil {
-		keySources++
-		data = [][]byte{}
-		for _, path := range pr.KeyPaths {
-			d, err := os.ReadFile(path)
-			if err != nil {
-				return sarRejected, nil, err
-			}
-			data = append(data, d)
-		}
-	}
-	if pr.KeyData != nil {
-		keySources++
-		data = [][]byte{pr.KeyData}
+	const notOneSourceErrorText = `Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified`
+	data, err := loadBytesFromConfigSources(configBytesSources{
+		inconsistencyErrorMessage: notOneSourceErrorText,
+		path:                      pr.KeyPath,
+		paths:                     pr.KeyPaths,
+		data:                      pr.KeyData,
+	})
+	if err != nil {
+		return sarRejected, nil, err
 	}
-	if keySources != 1 {
-		return sarRejected, nil, errors.New(`Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified`)
+	if data == nil {
+		return sarRejected, nil, errors.New(notOneSourceErrorText)
 	}
 
 	// FIXME: move this to per-context initialization

signature/policy_eval_sigstore.go

@@ -22,27 +22,39 @@ import (
 
 // configBytesSources contains configuration fields which may result in one or more []byte values
 type configBytesSources struct {
-	inconsistencyErrorMessage string // Error to return if more than one source is set
-	path                      string // …Path: a path to a file containing the data, or ""
-	data                      []byte // …Data: The raw data, or nil
+	inconsistencyErrorMessage string   // Error to return if more than one source is set
+	path                      string   // …Path: a path to a file containing the data, or ""
+	paths                     []string // …Paths: paths to files containing the data, or nil
+	data                      []byte   // …Data: a single instance ofhe raw data, or nil
 }
 
 // loadBytesFromConfigSources ensures at most one of the sources in src is set,
 // and returns the referenced data, or nil if neither is set.
-func loadBytesFromConfigSources(src configBytesSources) ([]byte, error) {
+func loadBytesFromConfigSources(src configBytesSources) ([][]byte, error) {
 	sources := 0
-	var data []byte // = nil
+	var data [][]byte // = nil
 	if src.path != "" {
 		sources++
 		d, err := os.ReadFile(src.path)
 		if err != nil {
 			return nil, err
 		}
-		data = d
+		data = [][]byte{d}
+	}
+	if src.paths != nil {
+		sources++
+		data = [][]byte{}
+		for _, path := range src.paths {
+			d, err := os.ReadFile(path)
+			if err != nil {
+				return nil, err
+			}
+			data = append(data, d)
+		}
 	}
 	if src.data != nil {
 		sources++
-		data = src.data
+		data = [][]byte{src.data}
 	}
 	if sources > 1 {
 		return nil, errors.New(src.inconsistencyErrorMessage)
@@ -53,19 +65,19 @@ func loadBytesFromConfigSources(src configBytesSources) ([]byte, error) {
 // prepareTrustRoot creates a fulcioTrustRoot from the input data.
 // (This also prevents external implementations of this interface, ensuring that prSigstoreSignedFulcio is the only one.)
 func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) {
-	caCertBytes, err := loadBytesFromConfigSources(configBytesSources{
+	caCertPEMs, err := loadBytesFromConfigSources(configBytesSources{
 		inconsistencyErrorMessage: `Internal inconsistency: both "caPath" and "caData" specified`,
 		path:                      f.CAPath,
 		data:                      f.CAData,
 	})
 	if err != nil {
 		return nil, err
 	}
-	if caCertBytes == nil {
-		return nil, errors.New(`Internal inconsistency: Fulcio specified with neither "caPath" nor "caData"`)
+	if len(caCertPEMs) != 1 {
+		return nil, errors.New(`Internal inconsistency: Fulcio specified with not exactly one of "caPath" nor "caData"`)
 	}
 	certs := x509.NewCertPool()
-	if ok := certs.AppendCertsFromPEM(caCertBytes); !ok {
+	if ok := certs.AppendCertsFromPEM(caCertPEMs[0]); !ok {
 		return nil, errors.New("error loading Fulcio CA certificates")
 	}
 	fulcio := fulcioTrustRoot{
@@ -89,16 +101,21 @@ type sigstoreSignedTrustRoot struct {
 func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) {
 	res := sigstoreSignedTrustRoot{}
 
-	publicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{
+	publicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{
 		inconsistencyErrorMessage: `Internal inconsistency: both "keyPath" and "keyData" specified`,
 		path:                      pr.KeyPath,
 		data:                      pr.KeyData,
 	})
 	if err != nil {
 		return nil, err
 	}
-	if publicKeyPEM != nil {
-		pk, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM)
+	if publicKeyPEMs != nil {
+		if len(publicKeyPEMs) != 1 {
+			// Coverage: This should never happen, we only provide single-element sources
+			// to loadBytesFromConfigSources, and at most one is allowed.
+			return nil, errors.New(`Internal inconsistency: got more than one element in "keyPath" and "keyData"`)
+		}
+		pk, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEMs[0])
 		if err != nil {
 			return nil, fmt.Errorf("parsing public key: %w", err)
 		}
@@ -113,16 +130,21 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error)
 		res.fulcio = f
 	}
 
-	rekorPublicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{
+	rekorPublicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{
 		inconsistencyErrorMessage: `Internal inconsistency: both "rekorPublicKeyPath" and "rekorPublicKeyData" specified`,
 		path:                      pr.RekorPublicKeyPath,
 		data:                      pr.RekorPublicKeyData,
 	})
 	if err != nil {
 		return nil, err
 	}
-	if rekorPublicKeyPEM != nil {
-		pk, err := cryptoutils.UnmarshalPEMToPublicKey(rekorPublicKeyPEM)
+	if rekorPublicKeyPEMs != nil {
+		if len(rekorPublicKeyPEMs) != 1 {
+			// Coverage: This should never happen, we only provide single-element sources
+			// to loadBytesFromConfigSources, and at most one is allowed.
+			return nil, errors.New(`Internal inconsistency: got more than one element in "rekorPublicKeyPath" and "rekorPublicKeyData"`)
+		}
+		pk, err := cryptoutils.UnmarshalPEMToPublicKey(rekorPublicKeyPEMs[0])
 		if err != nil {
 			return nil, fmt.Errorf("parsing Rekor public key: %w", err)
 		}