Skip to content

Commit

Permalink
Merge pull request #240 from vaikas/no-fulcio-api-hits
Browse files Browse the repository at this point in the history 
Rely exclusively on TUF root for fulcio root. Do not fetch them oob.
  • Loading branch information
@vaikas
vaikas committed Sep 16, 2022
2 parents 61204b9 + 6fff6db commit 76ea7a4
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 245 deletions.
1 change: 0 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ require (
github.com/pierrec/lz4 v2.6.1+incompatible
github.com/ryanuber/go-glob v1.0.0
github.com/sigstore/cosign v1.11.1
github.com/sigstore/fulcio v0.5.3
github.com/sigstore/rekor v0.11.0
github.com/sigstore/sigstore v1.4.1-0.20220823190236-c645ceb9d075
github.com/stretchr/testify v1.8.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1752,8 +1752,6 @@ github.com/shurcooL/go-goon v0.0.0-20170922171312-37c2f522c041/go.mod h1:N5mDOms
github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
github.com/sigstore/cosign v1.11.1 h1:y9IlANx0kTe5bt4wVkauJkfgWjMwmFId1H2y782hXmc=
github.com/sigstore/cosign v1.11.1/go.mod h1:PURIOXUUu1KmXTJ1x11DHH/X9CyaoYpa9AxRphagu+s=
github.com/sigstore/fulcio v0.5.3 h1:fwdl2BHv1RjL3GJJ44T+tPsvmQ028zv54psxVhSwUGA=
github.com/sigstore/fulcio v0.5.3/go.mod h1:4yzMqOao6r9Nul1Dgt4LL7loKdkkgbDemLYrXUuAc+Y=
github.com/sigstore/rekor v0.11.0 h1:2x1Sy3fu3VSWbl/2fwTyFPqs5fehY++EqdTFWWT6+Mo=
github.com/sigstore/rekor v0.11.0/go.mod h1:xEfHnfiQJ/yJVCz41/OglUrDID71gICzixJjYFrQeN0=
github.com/sigstore/sigstore v1.4.1-0.20220823190236-c645ceb9d075 h1:pQzybny/k/kdos7vYwB/7eHFEtbUZKYsTvbjqyoOWz4=
Expand Down
34 changes: 11 additions & 23 deletions pkg/webhook/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package webhook
import (
"context"
"crypto"
"crypto/x509"
"encoding/json"
"errors"
"fmt"
Expand All @@ -31,12 +30,12 @@ import (
ociremote "github.com/sigstore/cosign/pkg/oci/remote"
"github.com/sigstore/cosign/pkg/policy"
csigs "github.com/sigstore/cosign/pkg/signature"
"github.com/sigstore/fulcio/pkg/api"
"github.com/sigstore/policy-controller/pkg/apis/config"
policyduckv1beta1 "github.com/sigstore/policy-controller/pkg/apis/duck/v1beta1"
webhookcip "github.com/sigstore/policy-controller/pkg/webhook/clusterimagepolicy"
rekor "github.com/sigstore/rekor/pkg/client"
"github.com/sigstore/rekor/pkg/generated/client"
"github.com/sigstore/sigstore/pkg/fulcioroots"
"github.com/sigstore/sigstore/pkg/signature"
admissionv1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -587,20 +586,22 @@ func ValidatePolicySignaturesForAuthority(ctx context.Context, ref name.Referenc
return ociSignatureToPolicySignature(ctx, sps), nil

case authority.Keyless != nil:
if authority.Keyless != nil && authority.Keyless.URL != nil {
logging.FromContext(ctx).Debugf("Fetching FulcioRoot for %s : From: %s ", ref.Name(), authority.Keyless.URL)
fulcioroot, err := getFulcioCert(authority.Keyless.URL)
if authority.Keyless.URL != nil {
// TODO: This will probably need to change for:
// https://github.com/sigstore/policy-controller/issues/138
fulcioRoots, err := fulcioroots.Get()
if err != nil {
return nil, fmt.Errorf("fetching FulcioRoot: %w", err)
}
sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroot, rekorClient, authority.Keyless.Identities, remoteOpts...)
sps, err := validSignaturesWithFulcio(ctx, ref, fulcioRoots, rekorClient, authority.Keyless.Identities, remoteOpts...)
if err != nil {
logging.FromContext(ctx).Errorf("failed validSignatures for authority %s with fulcio for %s: %v", name, ref.Name(), err)
return nil, fmt.Errorf("signature keyless validation failed for authority %s for %s: %w", name, ref.Name(), err)
}
logging.FromContext(ctx).Debugf("validated signature for %s, got %d signatures", ref.Name(), len(sps))
return ociSignatureToPolicySignature(ctx, sps), nil
}
return nil, fmt.Errorf("no Keyless URL specified")
}

// This should never happen because authority has to have been validated to
Expand Down Expand Up @@ -642,12 +643,13 @@ func ValidatePolicyAttestationsForAuthority(ctx context.Context, ref name.Refere

case authority.Keyless != nil:
if authority.Keyless != nil && authority.Keyless.URL != nil {
logging.FromContext(ctx).Debugf("Fetching FulcioRoot for %s : From: %s ", ref.Name(), authority.Keyless.URL)
fulcioroot, err := getFulcioCert(authority.Keyless.URL)
// TODO: This will probably need to change for:
// https://github.com/sigstore/policy-controller/issues/138
fulcioRoots, err := fulcioroots.Get()
if err != nil {
return nil, fmt.Errorf("fetching FulcioRoot: %w", err)
}
va, err := validAttestationsWithFulcio(ctx, ref, fulcioroot, rekorClient, authority.Keyless.Identities, remoteOpts...)
va, err := validAttestationsWithFulcio(ctx, ref, fulcioRoots, rekorClient, authority.Keyless.Identities, remoteOpts...)
if err != nil {
logging.FromContext(ctx).Errorf("failed validAttestationsWithFulcio for authority %s with fulcio for %s: %v", name, ref.Name(), err)
return nil, fmt.Errorf("attestation keyless validation failed for authority %s for %s: %w", name, ref.Name(), err)
Expand Down Expand Up @@ -823,20 +825,6 @@ func (v *Validator) resolvePodSpec(ctx context.Context, ps *corev1.PodSpec, opt
resolveContainers(ps.Containers)
}

func getFulcioCert(u *apis.URL) (*x509.CertPool, error) {
fClient := api.NewClient(u.URL())
rootCertResponse, err := fClient.RootCert()
if err != nil {
return nil, fmt.Errorf("getting root cert: %w", err)
}

cp := x509.NewCertPool()
if !cp.AppendCertsFromPEM(rootCertResponse.ChainPEM) {
return nil, errors.New("error appending to root cert pool")
}
return cp, nil
}

// getNamespace tries to extract the namespace from the HTTPRequest
// if the namespace passed as argument is empty. This is a workaround
// for a bug in k8s <= 1.24.
Expand Down
36 changes: 18 additions & 18 deletions pkg/webhook/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,7 @@ func TestValidatePodSpec(t *testing.T) {
// Non-existent URL for testing complete failure
badURL := apis.HTTP("http://example.com/")

// Spin up a Fulcio that responds with a Root Cert
fulcioServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.Write([]byte(fulcioRootCert))
}))
t.Cleanup(fulcioServer.Close)
fulcioURL, err := apis.ParseURL(fulcioServer.URL)
fulcioURL, err := apis.ParseURL("https://fulcio.sigstore.dev")
if err != nil {
t.Fatalf("Failed to parse fake Fulcio URL")
}
Expand Down Expand Up @@ -296,11 +291,11 @@ func TestValidatePodSpec(t *testing.T) {
),
want: func() *apis.FieldError {
var errs *apis.FieldError
fe := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("initContainers", 0)
fe.Details = fmt.Sprintf("%s %s", digest.String(), `fetching FulcioRoot: getting root cert: request: parse "http://http:%2F%2Fexample.com%2F/api/v1/rootCert": invalid port ":%2F%2Fexample.com%2F" after host`)
fe := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("containers", 0)
fe.Details = fmt.Sprintf("%s %s", digest.String(), `signature keyless validation failed for authority for gcr.io/distroless/static@sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4: bad signature`)
errs = errs.Also(fe)
fe2 := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("containers", 0)
fe2.Details = fmt.Sprintf("%s %s", digest.String(), `fetching FulcioRoot: getting root cert: request: parse "http://http:%2F%2Fexample.com%2F/api/v1/rootCert": invalid port ":%2F%2Fexample.com%2F" after host`)
fe2 := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("initContainers", 0)
fe2.Details = fe.Details
errs = errs.Also(fe2)
return errs
}(),
Expand Down Expand Up @@ -1404,11 +1399,11 @@ func TestValidatePolicy(t *testing.T) {
rw.Write([]byte(fulcioRootCert))
}))
t.Cleanup(fulcioServer.Close)
fulcioURL, err := apis.ParseURL(fulcioServer.URL)

fulcioURL, err := apis.ParseURL("https://fulcio.sigstore.dev")
if err != nil {
t.Fatalf("Failed to parse fake Fulcio URL")
}
t.Logf("fulcioURL: %s", fulcioURL.String())

rekorServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.Write([]byte(rekorResponse))
Expand Down Expand Up @@ -1474,6 +1469,11 @@ func TestValidatePolicy(t *testing.T) {

// Let's say it is verified if it is the expected Public Key
authorityPublicKeyCVS := func(ctx context.Context, signedImgRef name.Reference, co *cosign.CheckOpts) (checkedSignatures []oci.Signature, bundleVerified bool, err error) {
// Because we use this below and it gets called for both key / keyless
// in the keyless case there's no SigVerifier, so fail it.
if co.SigVerifier == nil {
return nil, false, errors.New("Keyless used for key")
}
actualPublicKey, _ := co.SigVerifier.PublicKey()
actualECDSAPubkey := actualPublicKey.(*ecdsa.PublicKey)
actualKeyData := elliptic.Marshal(actualECDSAPubkey, actualECDSAPubkey.X, actualECDSAPubkey.Y)
Expand Down Expand Up @@ -1558,7 +1558,7 @@ func TestValidatePolicy(t *testing.T) {
}},
}},
},
wantErrs: []string{`fetching FulcioRoot: getting root cert: request: parse "http://http:%2F%2Fexample.com%2F/api/v1/rootCert": invalid port ":%2F%2Fexample.com%2F" after host`},
wantErrs: []string{`signature keyless validation failed for authority authority-1 for gcr.io/distroless/static@sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4: Keyless used for key`},
cvs: authorityPublicKeyCVS,
}, {
name: "simple, static set to pass",
Expand Down Expand Up @@ -1694,7 +1694,7 @@ func TestValidatePodSpecNonDefaultNamespace(t *testing.T) {
rw.Write([]byte(fulcioRootCert))
}))
t.Cleanup(fulcioServer.Close)
fulcioURL, err := apis.ParseURL(fulcioServer.URL)
fulcioURL, err := apis.ParseURL("https://fulcio.sigstore.dev")
if err != nil {
t.Fatalf("Failed to parse fake Fulcio URL")
}
Expand Down Expand Up @@ -1926,11 +1926,11 @@ func TestValidatePodSpecNonDefaultNamespace(t *testing.T) {
),
want: func() *apis.FieldError {
var errs *apis.FieldError
fe := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("initContainers", 0)
fe.Details = fmt.Sprintf("%s %s", digest.String(), `fetching FulcioRoot: getting root cert: request: parse "http://http:%2F%2Fexample.com%2F/api/v1/rootCert": invalid port ":%2F%2Fexample.com%2F" after host`)
fe := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("containers", 0)
fe.Details = fmt.Sprintf("%s %s", digest.String(), `signature keyless validation failed for authority for gcr.io/distroless/static@sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4: bad signature`)
errs = errs.Also(fe)
fe2 := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("containers", 0)
fe2.Details = fmt.Sprintf("%s %s", digest.String(), `fetching FulcioRoot: getting root cert: request: parse "http://http:%2F%2Fexample.com%2F/api/v1/rootCert": invalid port ":%2F%2Fexample.com%2F" after host`)
fe2 := apis.ErrGeneric("failed policy: cluster-image-policy-keyless", "image").ViaFieldIndex("initContainers", 0)
fe2.Details = fe.Details
errs = errs.Also(fe2)
return errs
}(),
Expand Down

0 comments on commit 76ea7a4

Please sign in to comment.