From a5ea0164e384f81fd2766e964bf4746f492d14e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 02:33:19 +0200 Subject: [PATCH] Use loadBytesFromConfigSources in prSignedBy.isSignatureAuthorAccepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend loadBytesFromConfigSources to return multiple values, and to support reading the from files; then share the code. Signed-off-by: Miloslav Trmač --- signature/policy_eval_signedby.go | 38 ++++++--------------- signature/policy_eval_sigstore.go | 56 +++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index 896ca5a60d..e5c9329185 100644 --- a/signature/policy_eval_signedby.go +++ b/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 diff --git a/signature/policy_eval_sigstore.go b/signature/policy_eval_sigstore.go index 99d3a1e01a..610a820806 100644 --- a/signature/policy_eval_sigstore.go +++ b/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,7 +65,7 @@ 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, @@ -61,11 +73,11 @@ func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { 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,7 +101,7 @@ 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, @@ -97,8 +109,13 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) 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,7 +130,7 @@ 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, @@ -121,8 +138,13 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) 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) }