pkg/verify/store/sigstore: Factor HTTP store into its own package

pkg/cvo/cvo.go

@@ -49,6 +49,7 @@ import (
 	"github.com/openshift/cluster-version-operator/pkg/verify/store"
 	"github.com/openshift/cluster-version-operator/pkg/verify/store/configmap"
 	"github.com/openshift/cluster-version-operator/pkg/verify/store/serial"
+	"github.com/openshift/cluster-version-operator/pkg/verify/store/sigstore"
 )
 
 const (
@@ -224,16 +225,6 @@ func New(
 	return optr
 }
 
-// verifyClientBuilder is a wrapper around the operator's HTTPClient method.
-// It is used by the releaseVerifier to get an up-to-date http client.
-type verifyClientBuilder struct {
-	builder func() (*http.Client, error)
-}
-
-func (vcb *verifyClientBuilder) HTTPClient() (*http.Client, error) {
-	return vcb.builder()
-}
-
 // InitializeFromPayload retrieves the payload contents and verifies the initial state, then configures the
 // controller that loads and applies content to the cluster. It returns an error if the payload appears to
 // be in error rather than continuing.
@@ -250,15 +241,14 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
 	optr.releaseCreated = update.ImageRef.CreationTimestamp.Time
 	optr.releaseVersion = update.ImageRef.Name
 
-	// Wraps operator's HTTPClient method to allow releaseVerifier to create http client with up-to-date config.
-	clientBuilder := &verifyClientBuilder{builder: optr.HTTPClient}
+	httpClientConstructor := sigstore.NewCachedHTTPClientConstructor(optr.HTTPClient, nil)
 	configClient, err := coreclientsetv1.NewForConfig(restConfig)
 	if err != nil {
 		return fmt.Errorf("unable to create a configuration client: %v", err)
 	}
 
 	// attempt to load a verifier as defined in the payload
-	verifier, signatureStore, err := loadConfigMapVerifierDataFromUpdate(update, clientBuilder, configClient)
+	verifier, signatureStore, err := loadConfigMapVerifierDataFromUpdate(update, httpClientConstructor.HTTPClient, configClient)
 	if err != nil {
 		return err
 	}
@@ -293,7 +283,7 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
 // It returns an error if the data is not valid, or no verifier if no config map is found. See the verify
 // package for more details on the algorithm for verification. If the annotation is set, a verifier or error
 // is always returned.
-func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder verify.ClientBuilder, configMapClient coreclientsetv1.ConfigMapsGetter) (verify.Interface, *verify.StorePersister, error) {
+func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder sigstore.HTTPClient, configMapClient coreclientsetv1.ConfigMapsGetter) (verify.Interface, *verify.StorePersister, error) {
 	configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
 	for _, manifest := range update.Manifests {
 		if manifest.GVK != configMapGVK {

pkg/cvo/cvo_test.go

@@ -40,6 +40,7 @@ import (
 	"github.com/openshift/cluster-version-operator/lib"
 	"github.com/openshift/cluster-version-operator/pkg/payload"
 	"github.com/openshift/cluster-version-operator/pkg/verify"
+	"github.com/openshift/cluster-version-operator/pkg/verify/store/sigstore"
 )
 
 var (
@@ -3420,7 +3421,7 @@ func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := kfake.NewSimpleClientset()
-			got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, verify.DefaultClient, f.CoreV1())
+			got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, sigstore.DefaultClient, f.CoreV1())
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
 			}

pkg/verify/configmap.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/openshift/cluster-version-operator/pkg/verify/store"
 	"github.com/openshift/cluster-version-operator/pkg/verify/store/parallel"
+	"github.com/openshift/cluster-version-operator/pkg/verify/store/sigstore"
 )
 
 // ReleaseAnnotationConfigMapVerifier is an annotation set on a config map in the
@@ -49,7 +50,7 @@ const ReleaseAnnotationConfigMapVerifier = "release.openshift.io/verification-co
 // The returned verifier will require that any new release image will only be considered verified
 // if each provided public key has signed the release image digest. The signature may be in any
 // store and the lookup order is internally defined.
-func NewFromConfigMapData(src string, data map[string]string, clientBuilder ClientBuilder) (*ReleaseVerifier, error) {
+func NewFromConfigMapData(src string, data map[string]string, clientBuilder sigstore.HTTPClient) (*ReleaseVerifier, error) {
 	verifiers := make(map[string]openpgp.EntityList)
 	var stores []store.Store
 	for k, v := range data {
@@ -71,9 +72,9 @@ func NewFromConfigMapData(src string, data map[string]string, clientBuilder Clie
 					directory: u.Path,
 				})
 			} else {
-				stores = append(stores, &httpStore{
-					uri:        u,
-					httpClient: clientBuilder.HTTPClient,
+				stores = append(stores, &sigstore.Store{
+					URI:        u,
+					HTTPClient: clientBuilder,
 				})
 			}
 		default:

pkg/verify/configmap_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 
 	"golang.org/x/crypto/openpgp"
+
+	"github.com/openshift/cluster-version-operator/pkg/verify/store/sigstore"
 )
 
 type VerifierAccessor interface {
@@ -56,7 +58,7 @@ func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := NewFromConfigMapData("from_test", tt.data, DefaultClient)
+			got, err := NewFromConfigMapData("from_test", tt.data, sigstore.DefaultClient)
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
 				return

pkg/verify/store/sigstore/client.go

@@ -0,0 +1,51 @@
+package sigstore
+
+import (
+	"net/http"
+	"sync"
+	"time"
+
+	"golang.org/x/time/rate"
+)
+
+// HTTPClient returns a client suitable for retrieving signatures. It is not
+// required to be unique per call, but may be called concurrently.
+type HTTPClient func() (*http.Client, error)
+
+// DefaultClient creates an http.Client with no configuration.
+func DefaultClient() (*http.Client, error) {
+	return &http.Client{}, nil
+}
+
+// CachedHTTPClientConstructor wraps an HTTPClient implementation so
+// that it is not called more frequently than the configured limiter.
+type CachedHTTPClientConstructor struct {
+	wrapped HTTPClient
+	limiter *rate.Limiter
+
+	lock       sync.Mutex
+	lastClient *http.Client
+	lastError  error
+}
+
+// NewCachedHTTPClientConstructor creates a new cached constructor.
+// If limiter is not specified it defaults to one call every 30 seconds.
+func NewCachedHTTPClientConstructor(wrapped HTTPClient, limiter *rate.Limiter) *CachedHTTPClientConstructor {
+	if limiter == nil {
+		limiter = rate.NewLimiter(rate.Every(30*time.Second), 1)
+	}
+	return &CachedHTTPClientConstructor{
+		wrapped: wrapped,
+		limiter: limiter,
+	}
+}
+
+func (c *CachedHTTPClientConstructor) HTTPClient() (*http.Client, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+	r := c.limiter.Reserve()
+	if r.OK() {
+		c.lastClient, c.lastError = c.wrapped()
+	}
+	return c.lastClient, c.lastError
+}

pkg/verify/store/sigstore/sigstore.go

@@ -0,0 +1,154 @@
+// Package sigstore retrieves signatures using the sig-store protocol
+// described in [1].
+//
+// A URL (scheme http:// or https://) location that contains
+// signatures. These signatures are in the atomic container signature
+// format. The URL will have the digest of the image appended to it as
+// "<STORE>/<ALGO>=<DIGEST>/signature-<NUMBER>" as described in the
+// container image signing format. Signatures are searched starting at
+// NUMBER 1 and incrementing if the signature exists but is not valid.
+//
+// [1]: https://github.com/containers/image/blob/ab49b0a48428c623a8f03b41b9083d48966b34a9/docs/signature-protocols.md
+package sigstore
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"path"
+	"strconv"
+	"strings"
+
+	"k8s.io/klog"
+
+	"github.com/openshift/cluster-version-operator/pkg/verify/store"
+)
+
+// maxSignatureSearch prevents unbounded recursion on malicious signature stores (if
+// an attacker was able to take ownership of the store to perform DoS on clusters).
+const maxSignatureSearch = 10
+
+var errNotFound = errors.New("no more signatures to check")
+
+// Store provides access to signatures stored in memory.
+type Store struct {
+	// URI is the base from which signature URIs are constructed.
+	URI *url.URL
+
+	// HTTPClient is called once for each Signatures call to ensure
+	// requests are made with the currently-recommended parameters.
+	HTTPClient HTTPClient
+}
+
+// Signatures fetches signatures for the provided digest.
+func (s *Store) Signatures(ctx context.Context, name string, digest string, fn store.Callback) error {
+	parts := strings.SplitN(digest, ":", 3)
+	if len(parts) != 2 || len(parts[0]) == 0 || len(parts[1]) == 0 {
+		return fmt.Errorf("the provided release image digest must be of the form ALGO:HASH")
+	}
+	algo, hash := parts[0], parts[1]
+	equalDigest := fmt.Sprintf("%s=%s", algo, hash)
+
+	switch s.URI.Scheme {
+	case "http", "https":
+		client, err := s.HTTPClient()
+		if err != nil {
+			_, err = fn(ctx, nil, err)
+			return err
+		}
+
+		copied := *s.URI
+		copied.Path = path.Join(copied.Path, equalDigest)
+		if err := checkHTTPSignatures(ctx, client, copied, maxSignatureSearch, fn); err != nil {
+			return err
+		}
+	default:
+		return fmt.Errorf("the store %s scheme is unrecognized", s.URI)
+	}
+
+	return nil
+}
+
+// checkHTTPSignatures reads signatures as "signature-1", "signature-2", etc. as children of the provided URL
+// over HTTP or HTTPS.  No more than maxSignaturesToCheck will be read. If the provided context is cancelled
+// search will be terminated.
+func checkHTTPSignatures(ctx context.Context, client *http.Client, u url.URL, maxSignaturesToCheck int, fn store.Callback) error {
+	base := path.Join(u.Path, "signature-")
+	sigURL := u
+	for i := 1; i < maxSignatureSearch; i++ {
+		if err := ctx.Err(); err != nil {
+			return err
+		}
+
+		sigURL.Path = base + strconv.Itoa(i)
+
+		req, err := http.NewRequest("GET", sigURL.String(), nil)
+		if err != nil {
+			_, err = fn(ctx, nil, fmt.Errorf("could not build request to check signature: %v", err))
+			return err // even if the callback ate the error, no sense in checking later indexes which will fail the same way
+		}
+		req = req.WithContext(ctx)
+		// load the body, being careful not to allow unbounded reads
+		resp, err := client.Do(req)
+		if err != nil {
+			klog.V(4).Infof("unable to load signature: %v", err)
+			done, err := fn(ctx, nil, err)
+			if done || err != nil {
+				return err
+			}
+			continue
+		}
+		data, err := func() ([]byte, error) {
+			body := resp.Body
+			r := io.LimitReader(body, 50*1024)
+
+			defer func() {
+				// read the remaining body to avoid breaking the connection
+				io.Copy(ioutil.Discard, r)
+				body.Close()
+			}()
+
+			if resp.StatusCode == http.StatusNotFound {
+				return nil, errNotFound
+			}
+			if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+				if i == 1 {
+					klog.V(4).Infof("Could not find signature at store location %v", sigURL)
+				}
+				return nil, fmt.Errorf("unable to retrieve signature from %v: %d", sigURL, resp.StatusCode)
+			}
+
+			return ioutil.ReadAll(resp.Body)
+		}()
+		if err == errNotFound {
+			break
+		}
+		if err != nil {
+			klog.V(4).Info(err)
+			done, err := fn(ctx, nil, err)
+			if done || err != nil {
+				return err
+			}
+			continue
+		}
+		if len(data) == 0 {
+			continue
+		}
+
+		done, err := fn(ctx, data, nil)
+		if done || err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// String returns a description of where this store finds
+// signatures.
+func (s *Store) String() string {
+	return fmt.Sprintf("containers/image signature store under %s", s.URI)
+}