Skip to content

Commit

Permalink
Merge pull request #396 from wking/push-http-signature-store-to-subpa…
Browse files Browse the repository at this point in the history
…ckage

pkg/verify/store/sigstore: Factor HTTP store into its own package
  • Loading branch information
openshift-merge-robot committed Jul 10, 2020
2 parents 8b9931b + e5a075b commit 5c6d72c
Show file tree
Hide file tree
Showing 9 changed files with 370 additions and 171 deletions.
18 changes: 4 additions & 14 deletions pkg/cvo/cvo.go
Expand Up @@ -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 (
Expand Down Expand Up @@ -218,16 +219,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.
Expand All @@ -244,15 +235,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
}
Expand Down Expand Up @@ -287,7 +277,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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/cvo/cvo_test.go
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/verify/configmap.go
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion pkg/verify/configmap_test.go
Expand Up @@ -6,6 +6,8 @@ import (
"testing"

"golang.org/x/crypto/openpgp"

"github.com/openshift/cluster-version-operator/pkg/verify/store/sigstore"
)

type VerifierAccessor interface {
Expand Down Expand Up @@ -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
Expand Down
51 changes: 51 additions & 0 deletions 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
}
154 changes: 154 additions & 0 deletions 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)
}

0 comments on commit 5c6d72c

Please sign in to comment.