Add proxy support to CVO image verification.

This commit adds support for using the cluster proxy and trust bundle to verify image signatures in the CVO. Before this commit, the CVO would create the release verifier and store inside the verifier an http client for all traffic. The verifier is created before the proxy lister and values have been established by the operator, so those values cannot be read in order to properly configure the client; furthermore, the client needs to take into account that the cluster proxy is subject to change. To address these problems, the verifier has been updated to accept an interface which can provide an up-to-date client whenever a client is needed. The initial creation of the verifier accepts a reference to the operator to implement the interface.

Author: patrickdillon

pkg/cvo/cvo.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strconv"
 	"sync"
@@ -24,6 +25,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/transport"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog"
 
@@ -207,6 +209,16 @@ 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.
@@ -223,8 +235,11 @@ 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}
+
 	// attempt to load a verifier as defined in the payload
-	verifier, err := verify.LoadFromPayload(update)
+	verifier, err := verify.LoadFromPayload(update, clientBuilder)
 	if err != nil {
 		return err
 	}
@@ -655,6 +670,27 @@ func (optr *Operator) defaultPreconditionChecks() precondition.List {
 	}
 }
 
+// HTTPClient provides a method for generating an HTTP client
+// with the proxy and trust settings, if set in the cluster.
+func (optr *Operator) HTTPClient() (*http.Client, error) {
+	proxyURL, tlsConfig, err := optr.getTransportOpts()
+	if err != nil {
+		return nil, err
+	}
+	transportOption := &http.Transport{
+		Proxy:           http.ProxyURL(proxyURL),
+		TLSClientConfig: tlsConfig,
+	}
+	transportConfig := &transport.Config{Transport: transportOption}
+	transport, err := transport.New(transportConfig)
+	if err != nil {
+		return nil, err
+	}
+	return &http.Client{
+		Transport: transport,
+	}, nil
+}
+
 // getTransportOpts retrieves the URL of the cluster proxy and the CA
 // trust, if they exist.
 func (optr *Operator) getTransportOpts() (*url.URL, *tls.Config, error) {

pkg/verify/verify.go

@@ -18,13 +18,11 @@ import (
 	"strings"
 	"time"
 
-	"k8s.io/klog"
 	"github.com/pkg/errors"
 	"golang.org/x/crypto/openpgp"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/client-go/transport"
+	"k8s.io/klog"
 
 	"github.com/openshift/cluster-version-operator/pkg/payload"
 )
@@ -74,7 +72,7 @@ var Reject Interface = rejectVerifier{}
 // 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 LoadFromPayload(update *payload.Update) (Interface, error) {
+func LoadFromPayload(update *payload.Update, clientBuilder ClientBuilder) (Interface, error) {
 	configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
 	for _, manifest := range update.Manifests {
 		if manifest.GVK != configMapGVK {
@@ -117,16 +115,10 @@ func LoadFromPayload(update *payload.Update) (Interface, error) {
 			return nil, fmt.Errorf("%s did not provide any GPG public keys to verify signatures from and cannot be used", src)
 		}
 
-		transport, err := transport.New(&transport.Config{})
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to create HTTP client for verifying signatures: %v", err)
-		}
 		return &releaseVerifier{
-			verifiers: verifiers,
-			stores:    stores,
-			client: &http.Client{
-				Transport: transport,
-			},
+			verifiers:     verifiers,
+			stores:        stores,
+			clientBuilder: clientBuilder,
 		}, nil
 	}
 	return nil, nil
@@ -148,9 +140,23 @@ var validReleaseDigest = regexp.MustCompile(`^[a-zA-Z0-9:]+$`)
 // digest - all verifiers must have at least one valid signature attesting the release
 // digest. If any failure occurs the caller should assume the content is unverified.
 type releaseVerifier struct {
-	verifiers map[string]openpgp.EntityList
-	stores    []*url.URL
-	client    *http.Client
+	verifiers     map[string]openpgp.EntityList
+	stores        []*url.URL
+	clientBuilder ClientBuilder
+}
+
+// ClientBuilder provides a method for generating an HTTP Client configured
+// with cluster proxy settings, if they exist.
+type ClientBuilder interface {
+	HTTPClient() (*http.Client, error)
+}
+
+// simpleClientBuilder implements the ClientBuilder interface and should be used for testing.
+type simpleClientBuilder struct{}
+
+// HTTPClient from simpleClientBuilder creates an httpClient with no configuration.
+func (s *simpleClientBuilder) HTTPClient() (*http.Client, error) {
+	return &http.Client{}, nil
 }
 
 // String summarizes the verifier for human consumption
@@ -243,6 +249,11 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro
 		return len(remaining) > 0, nil
 	}
 
+	client, err := v.clientBuilder.HTTPClient()
+	if err != nil {
+		return err
+	}
+
 	for _, store := range v.stores {
 		if len(remaining) == 0 {
 			break
@@ -256,7 +267,7 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro
 		case "http", "https":
 			copied := *store
 			copied.Path = path.Join(store.Path, name)
-			if err := checkHTTPSignatures(ctx, v.client, copied, maxSignatureSearch, verifier); err != nil {
+			if err := checkHTTPSignatures(ctx, client, copied, maxSignatureSearch, verifier); err != nil {
 				return err
 			}
 		default:
@@ -325,7 +336,6 @@ func checkHTTPSignatures(ctx context.Context, client *http.Client, u url.URL, ma
 			return fmt.Errorf("could not build request to check signature: %v", err)
 		}
 		req = req.WithContext(ctx)
-
 		// load the body, being careful not to allow unbounded reads
 		resp, err := client.Do(req)
 		if err != nil {

pkg/verify/verify_test.go

@@ -10,8 +10,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"k8s.io/client-go/transport"
-
 	"github.com/openshift/cluster-version-operator/lib"
 	"github.com/openshift/cluster-version-operator/pkg/payload"
 	"golang.org/x/crypto/openpgp"
@@ -161,16 +159,13 @@ func Test_releaseVerifier_Verify(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			transport, err := transport.New(&transport.Config{})
 			if err != nil {
 				t.Fatal(err)
 			}
 			v := &releaseVerifier{
-				verifiers: tt.verifiers,
-				stores:    tt.stores,
-				client: &http.Client{
-					Transport: transport,
-				},
+				verifiers:     tt.verifiers,
+				stores:        tt.stores,
+				clientBuilder: &simpleClientBuilder{},
 			}
 			if err := v.Verify(context.Background(), tt.releaseDigest); (err != nil) != tt.wantErr {
 				t.Errorf("releaseVerifier.Verify() error = %v, wantErr %v", err, tt.wantErr)
@@ -344,7 +339,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := LoadFromPayload(tt.update)
+			got, err := LoadFromPayload(tt.update, &simpleClientBuilder{})
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
 				return