From 96e6d934ae402d58c1c481a574c829f1b5a3adf5 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Mon, 11 Apr 2022 21:26:38 +0000 Subject: [PATCH] Add verification for embedded SCTs This adds support for verifying SCTs embedded in certificates in addition to being detached. Embedded SCTs will be verified while parsing the certificate on verification (for verify, verify-blob, and verify-attestation), and on signing if a certificate chain is provided. Signed-off-by: Hayden Blauzvern --- cmd/cosign/cli/fulcio/depcheck_test.go | 4 +- .../testdata/garbage-there-are-limits | 0 .../fulcioverifier/{ => ctl}/testdata/google | 0 .../testdata/letsencrypt-testflume-2021 | Bin .../fulcioverifier/{ => ctl}/testdata/rsa | 0 .../cli/fulcio/fulcioverifier/ctl/verify.go | 230 ++++++++++++++++ .../fulcio/fulcioverifier/ctl/verify_test.go | 250 ++++++++++++++++++ .../fulcio/fulcioverifier/fulcioverifier.go | 116 +------- .../fulcioverifier/fulcioverifier_test.go | 73 ----- cmd/cosign/cli/sign/sign.go | 16 +- cmd/cosign/policy_webhook/depcheck_test.go | 2 +- cmd/cosign/webhook/depcheck_test.go | 2 +- go.mod | 2 + go.sum | 1 - pkg/cosign/verify.go | 29 +- pkg/cosign/verify_test.go | 45 +++- 16 files changed, 568 insertions(+), 202 deletions(-) rename cmd/cosign/cli/fulcio/fulcioverifier/{ => ctl}/testdata/garbage-there-are-limits (100%) rename cmd/cosign/cli/fulcio/fulcioverifier/{ => ctl}/testdata/google (100%) rename cmd/cosign/cli/fulcio/fulcioverifier/{ => ctl}/testdata/letsencrypt-testflume-2021 (100%) rename cmd/cosign/cli/fulcio/fulcioverifier/{ => ctl}/testdata/rsa (100%) create mode 100644 cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go create mode 100644 cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go delete mode 100644 cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier_test.go diff --git a/cmd/cosign/cli/fulcio/depcheck_test.go b/cmd/cosign/cli/fulcio/depcheck_test.go index 39493e5a86c..058a6fa8e94 100644 --- a/cmd/cosign/cli/fulcio/depcheck_test.go +++ b/cmd/cosign/cli/fulcio/depcheck_test.go @@ -25,8 +25,8 @@ func TestNoDeps(t *testing.T) { depcheck.AssertNoDependency(t, map[string][]string{ "github.com/sigstore/cosign/cmd/cosign/cli/fulcio": { // Avoid pulling in a variety of things that are massive dependencies. - "github.com/google/certificate-transparency-go", - "github.com/google/trillian", + // "github.com/google/certificate-transparency-go", + // "github.com/google/trillian", "github.com/envoyproxy/go-control-plane", "github.com/gogo/protobuf/protoc-gen-gogo", "github.com/grpc-ecosystem/go-grpc-middleware", diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/testdata/garbage-there-are-limits b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/garbage-there-are-limits similarity index 100% rename from cmd/cosign/cli/fulcio/fulcioverifier/testdata/garbage-there-are-limits rename to cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/garbage-there-are-limits diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/testdata/google b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/google similarity index 100% rename from cmd/cosign/cli/fulcio/fulcioverifier/testdata/google rename to cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/google diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/testdata/letsencrypt-testflume-2021 b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/letsencrypt-testflume-2021 similarity index 100% rename from cmd/cosign/cli/fulcio/fulcioverifier/testdata/letsencrypt-testflume-2021 rename to cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/letsencrypt-testflume-2021 diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/testdata/rsa b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/rsa similarity index 100% rename from cmd/cosign/cli/fulcio/fulcioverifier/testdata/rsa rename to cmd/cosign/cli/fulcio/fulcioverifier/ctl/testdata/rsa diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go new file mode 100644 index 00000000000..2fdcd7d71da --- /dev/null +++ b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go @@ -0,0 +1,230 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ctl + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/sha256" + "crypto/x509" + "encoding/json" + "encoding/pem" + "fmt" + "os" + + ct "github.com/google/certificate-transparency-go" + "github.com/google/certificate-transparency-go/ctutil" + "github.com/google/certificate-transparency-go/trillian/ctfe" + ctx509 "github.com/google/certificate-transparency-go/x509" + "github.com/google/certificate-transparency-go/x509util" + "github.com/pkg/errors" + + "github.com/sigstore/cosign/pkg/cosign/tuf" + "github.com/sigstore/sigstore/pkg/cryptoutils" +) + +// This is the CT log public key target name +var ctPublicKeyStr = `ctfe.pub` + +// Setting this env variable will over ride what is used to validate +// the SCT coming back from Fulcio. +const altCTLogPublicKeyLocation = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE" + +// logIDMetadata holds information for mapping a key ID hash (log ID) to associated data. +type logIDMetadata struct { + pubKey crypto.PublicKey + status tuf.StatusKind +} + +// ContainsSCT checks if the certificate contains embedded SCTs. cert can either be +// DER or PEM encoded. +func ContainsSCT(cert []byte) (bool, error) { + embeddedSCTs, err := x509util.ParseSCTsFromCertificate(cert) + if err != nil { + return false, err + } + if len(embeddedSCTs) != 0 { + return true, nil + } + return false, nil +} + +// VerifySCT verifies SCTs against the Fulcio CT log public key. +// +// The SCT is a `Signed Certificate Timestamp`, which promises that +// the certificate issued by Fulcio was also added to the public CT log within +// some defined time period. +// +// VerifySCT can verify an SCT list embedded in the certificate, or a detached +// SCT provided by Fulcio. +// +// By default the public keys comes from TUF, but you can override this for test +// purposes by using an env variable `SIGSTORE_CT_LOG_PUBLIC_KEY_FILE`. If using +// an alternate, the file can be PEM, or DER format. +func VerifySCT(ctx context.Context, certPEM, chainPEM, rawSCT []byte) error { + // fetch SCT verification key + pubKeys := make(map[[sha256.Size]byte]logIDMetadata) + rootEnv := os.Getenv(altCTLogPublicKeyLocation) + if rootEnv == "" { + tufClient, err := tuf.NewFromEnv(ctx) + if err != nil { + return err + } + defer tufClient.Close() + + targets, err := tufClient.GetTargetsByMeta(tuf.CTFE, []string{ctPublicKeyStr}) + if err != nil { + return err + } + for _, t := range targets { + pub, err := cryptoutils.UnmarshalPEMToPublicKey(t.Target) + if err != nil { + return err + } + ctPub, ok := pub.(*ecdsa.PublicKey) + if !ok { + return fmt.Errorf("invalid public key: was %T, require *ecdsa.PublicKey", pub) + } + keyID, err := ctfe.GetCTLogID(ctPub) + if err != nil { + return errors.Wrap(err, "error getting CTFE public key hash") + } + pubKeys[keyID] = logIDMetadata{ctPub, t.Status} + } + } else { + fmt.Fprintf(os.Stderr, "**Warning** Using a non-standard public key for verifying SCT: %s\n", rootEnv) + raw, err := os.ReadFile(rootEnv) + if err != nil { + return errors.Wrap(err, "error reading alternate public key file") + } + pubKey, err := getAlternatePublicKey(raw) + if err != nil { + return errors.Wrap(err, "error parsing alternate public key from the file") + } + keyID, err := ctfe.GetCTLogID(pubKey) + if err != nil { + return errors.Wrap(err, "error getting CTFE public key hash") + } + pubKeys[keyID] = logIDMetadata{pubKey, tuf.Active} + } + if len(pubKeys) == 0 { + return errors.New("none of the CTFE keys have been found") + } + + // parse certificate and chain + cert, err := x509util.CertificateFromPEM(certPEM) + if err != nil { + return err + } + certChain, err := x509util.CertificatesFromPEM(chainPEM) + if err != nil { + return err + } + if len(certChain) == 0 { + return errors.New("no certificate chain found") + } + + // fetch embedded SCT if present + embeddedSCTs, err := x509util.ParseSCTsFromCertificate(certPEM) + if err != nil { + return err + } + // SCT must be either embedded or in header + if len(embeddedSCTs) == 0 && len(rawSCT) == 0 { + return errors.New("no SCT found") + } + + // check SCT embedded in certificate + if len(embeddedSCTs) != 0 { + for _, sct := range embeddedSCTs { + pubKeyMetadata, ok := pubKeys[sct.LogID.KeyID] + if !ok { + return errors.New("ctfe public key not found for embedded SCT") + } + err := ctutil.VerifySCT(pubKeyMetadata.pubKey, []*ctx509.Certificate{cert, certChain[0]}, sct, true) + if err != nil { + return errors.Wrap(err, "error verifying embedded SCT") + } + if pubKeyMetadata.status != tuf.Active { + fmt.Fprintf(os.Stderr, "**Info** Successfully verified embedded SCT using an expired verification key\n") + } + } + return nil + } + + // check SCT in response header + var addChainResp ct.AddChainResponse + if err := json.Unmarshal(rawSCT, &addChainResp); err != nil { + return errors.Wrap(err, "unmarshal") + } + sct, err := addChainResp.ToSignedCertificateTimestamp() + if err != nil { + return err + } + pubKeyMetadata, ok := pubKeys[sct.LogID.KeyID] + if !ok { + return errors.New("ctfe public key not found") + } + err = ctutil.VerifySCT(pubKeyMetadata.pubKey, []*ctx509.Certificate{cert}, sct, false) + if err != nil { + return errors.Wrap(err, "error verifying SCT") + } + if pubKeyMetadata.status != tuf.Active { + fmt.Fprintf(os.Stderr, "**Info** Successfully verified SCT using an expired verification key\n") + } + return nil +} + +// VerifyEmbeddedSCT verifies an embedded SCT in a certificate. +func VerifyEmbeddedSCT(ctx context.Context, chain []*x509.Certificate) error { + if len(chain) < 2 { + return errors.New("certificate chain must contain at least a certificate and its issuer") + } + certPEM, err := cryptoutils.MarshalCertificateToPEM(chain[0]) + if err != nil { + return err + } + chainPEM, err := cryptoutils.MarshalCertificatesToPEM(chain[1:]) + if err != nil { + return err + } + return VerifySCT(ctx, certPEM, chainPEM, []byte{}) +} + +// Given a byte array, try to construct a public key from it. +// Will try first to see if it's PEM formatted, if not, then it will +// try to parse it as der publics, and failing that +func getAlternatePublicKey(in []byte) (crypto.PublicKey, error) { + var pubKey crypto.PublicKey + var err error + var derBytes []byte + pemBlock, _ := pem.Decode(in) + if pemBlock == nil { + fmt.Fprintf(os.Stderr, "Failed to decode non-standard public key for verifying SCT using PEM decode, trying as DER") + derBytes = in + } else { + derBytes = pemBlock.Bytes + } + pubKey, err = x509.ParsePKIXPublicKey(derBytes) + if err != nil { + // Try using the PKCS1 before giving up. + pubKey, err = x509.ParsePKCS1PublicKey(derBytes) + if err != nil { + return nil, errors.Wrap(err, "failed to parse alternate public key") + } + } + return pubKey, nil +} diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go new file mode 100644 index 00000000000..751e1423551 --- /dev/null +++ b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go @@ -0,0 +1,250 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ctl + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "os" + "reflect" + "strings" + "testing" + + ct "github.com/google/certificate-transparency-go" + "github.com/google/certificate-transparency-go/testdata" + "github.com/google/certificate-transparency-go/tls" + "github.com/sigstore/sigstore/pkg/cryptoutils" +) + +func TestGetAlternatePublicKey(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get cwd: %v", err) + } + tests := []struct { + file string + wantErrSub string + wantType string + }{ + {file: "garbage-there-are-limits", wantErrSub: "failed to parse"}, + // Testflume 2021 from here, https://letsencrypt.org/docs/ct-logs/ + {file: "letsencrypt-testflume-2021", wantType: "*ecdsa.PublicKey"}, + // This needs to be parsed with the pkcs1, pkix won't do. + {file: "rsa", wantType: "*rsa.PublicKey"}, + // This works with pkix, from: + // https://www.gstatic.com/ct/log_list/v2/log_list_pubkey.pem + {file: "google", wantType: "*rsa.PublicKey"}, + } + for _, tc := range tests { + filepath := fmt.Sprintf("%s/testdata/%s", wd, tc.file) + bytes, err := os.ReadFile(filepath) + if err != nil { + t.Fatalf("Failed to read testfile %s : %v", tc.file, err) + } + got, err := getAlternatePublicKey(bytes) + switch { + case err == nil && tc.wantErrSub != "": + t.Errorf("Wanted Error for %s but got none", tc.file) + case err != nil && tc.wantErrSub == "": + t.Errorf("Did not want error for %s but got: %v", tc.file, err) + case err != nil && tc.wantErrSub != "": + if !strings.Contains(err.Error(), tc.wantErrSub) { + t.Errorf("Unexpected error for %s: %s wanted to contain: %s", tc.file, err.Error(), tc.wantErrSub) + } + } + switch { + case got == nil && tc.wantType != "": + t.Errorf("Wanted public key for %s but got none", tc.file) + case got != nil && tc.wantType == "": + t.Errorf("Did not want error for %s but got: %v", tc.file, err) + case got != nil && tc.wantType != "": + if reflect.TypeOf(got).String() != tc.wantType { + t.Errorf("Unexpected type for %s: %+T wanted: %s", tc.file, got, tc.wantType) + } + } + } +} + +func TestContainsSCT(t *testing.T) { + // test certificate without embedded SCT + contains, err := ContainsSCT([]byte(testdata.TestCertPEM)) + if err != nil { + t.Fatalf("unexpected error in ContainsSCT: %v", err) + } + if contains { + t.Fatalf("certificate unexpectedly contained SCT") + } + + // test certificate with embedded SCT + contains, err = ContainsSCT([]byte(testdata.TestEmbeddedCertPEM)) + if err != nil { + t.Fatalf("unexpected error in ContainsSCT: %v", err) + } + if !contains { + t.Fatalf("certificate unexpectedly did not contain SCT") + } +} + +// From https://github.com/google/certificate-transparency-go/blob/e76f3f637053b90c8168d29b01ca162cd235ace5/ctutil/ctutil_test.go +func TestVerifySCT(t *testing.T) { + tests := []struct { + desc string + certPEM string + chainPEM string + sct []byte + embedded bool + wantErr bool + errMsg string + }{ + { + desc: "cert", + certPEM: testdata.TestCertPEM, + chainPEM: testdata.CACertPEM, + sct: testdata.TestCertProof, + }, + { + desc: "invalid SCT", + certPEM: testdata.TestPreCertPEM, + chainPEM: testdata.CACertPEM, + sct: testdata.TestCertProof, + wantErr: true, + }, + { + desc: "cert with embedded SCT", + certPEM: testdata.TestEmbeddedCertPEM, + chainPEM: testdata.CACertPEM, + sct: testdata.TestPreCertProof, + embedded: true, + }, + { + desc: "cert with invalid embedded SCT", + certPEM: testdata.TestInvalidEmbeddedCertPEM, + chainPEM: testdata.CACertPEM, + sct: testdata.TestInvalidProof, + embedded: true, + wantErr: true, + errMsg: "failed to verify ECDSA signature", + }, + } + + writePubKey(t, testdata.LogPublicKeyPEM) + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + // convert SCT to response struct if detached + var sctBytes []byte + if !test.embedded { + var sct ct.SignedCertificateTimestamp + if _, err := tls.Unmarshal(test.sct, &sct); err != nil { + t.Fatalf("error tls-unmarshalling sct: %s", err) + } + chainResp, err := toAddChainResponse(&sct) + if err != nil { + t.Fatalf("error generating chain response: %v", err) + } + sctBytes, err = json.Marshal(chainResp) + if err != nil { + t.Fatalf("error marshalling chain: %v", err) + } + } + + err := VerifySCT(context.Background(), []byte(test.certPEM), []byte(test.chainPEM), sctBytes) + if gotErr := err != nil; gotErr != test.wantErr && !strings.Contains(err.Error(), test.errMsg) { + t.Errorf("VerifySCT(_,_,_, %t) = %v, want error? %t", test.embedded, err, test.wantErr) + } + }) + } +} + +func TestVerifySCTError(t *testing.T) { + // verify fails with mismatched verifcation key + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("unexpected error generating ECDSA key: %v", err) + } + pemKey, err := cryptoutils.MarshalPublicKeyToPEM(key.Public()) + if err != nil { + t.Fatalf("unexpected error marshalling ECDSA key: %v", err) + } + writePubKey(t, string(pemKey)) + err = VerifySCT(context.Background(), []byte(testdata.TestEmbeddedCertPEM), []byte(testdata.CACertPEM), []byte{}) + if err == nil || !strings.Contains(err.Error(), "ctfe public key not found") { + t.Fatalf("expected error verifying SCT with mismatched key: %v", err) + } + + // verify fails without either a detached SCT or embedded SCT + err = VerifySCT(context.Background(), []byte(testdata.TestCertPEM), []byte(testdata.CACertPEM), []byte{}) + if err == nil || !strings.Contains(err.Error(), "no SCT found") { + t.Fatalf("expected error verifying SCT without SCT: %v", err) + } +} + +func TestVerifyEmbeddedSCT(t *testing.T) { + chain, err := cryptoutils.UnmarshalCertificatesFromPEM([]byte(testdata.TestEmbeddedCertPEM + testdata.CACertPEM)) + if err != nil { + t.Fatalf("error unmarshalling certificate chain: %v", err) + } + + // verify fails without a certificate chain + err = VerifyEmbeddedSCT(context.Background(), chain[:1]) + if err == nil || err.Error() != "certificate chain must contain at least a certificate and its issuer" { + t.Fatalf("expected error verifying SCT without chain: %v", err) + } + + writePubKey(t, testdata.LogPublicKeyPEM) + err = VerifyEmbeddedSCT(context.Background(), chain) + if err != nil { + t.Fatalf("unexpected error verifying embedded SCT: %v", err) + } +} + +// toAddChainResponse converts an SCT to a response struct, the expected structure for detached SCTs +func toAddChainResponse(sct *ct.SignedCertificateTimestamp) (*ct.AddChainResponse, error) { + sig, err := tls.Marshal(sct.Signature) + if err != nil { + return nil, fmt.Errorf("failed to marshal signature: %w", err) + } + addChainResp := &ct.AddChainResponse{ + SCTVersion: sct.SCTVersion, + Timestamp: sct.Timestamp, + Extensions: base64.StdEncoding.EncodeToString(sct.Extensions), + ID: sct.LogID.KeyID[:], + Signature: sig, + } + + return addChainResp, nil +} + +// writePubKey writes the SCT verification key to disk, since there is not a TUF +// test setup +func writePubKey(t *testing.T, keyPEM string) { + t.Helper() + + tmpPrivFile, err := os.CreateTemp(t.TempDir(), "cosign_verify_sct_*.key") + if err != nil { + t.Fatalf("failed to create temp key file: %v", err) + } + t.Cleanup(func() { tmpPrivFile.Close() }) + if _, err := tmpPrivFile.Write([]byte(keyPEM)); err != nil { + t.Fatalf("failed to write key file: %v", err) + } + os.Setenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE", tmpPrivFile.Name()) + t.Cleanup(func() { os.Unsetenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE") }) +} diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go b/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go index 819585a9985..3687f5db01f 100644 --- a/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go +++ b/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go @@ -17,103 +17,16 @@ package fulcioverifier import ( "context" - "crypto" - "crypto/x509" - "encoding/json" - "encoding/pem" "fmt" "os" - ct "github.com/google/certificate-transparency-go" - "github.com/google/certificate-transparency-go/ctutil" - ctx509 "github.com/google/certificate-transparency-go/x509" - "github.com/google/certificate-transparency-go/x509util" "github.com/pkg/errors" "github.com/sigstore/cosign/cmd/cosign/cli/fulcio" - "github.com/sigstore/cosign/pkg/cosign" - "github.com/sigstore/cosign/pkg/cosign/tuf" + "github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl" "github.com/sigstore/fulcio/pkg/api" ) -// This is the CT log public key target name -var ctPublicKeyStr = `ctfe.pub` - -// Setting this env variable will over ride what is used to validate -// the SCT coming back from Fulcio. -const altCTLogPublicKeyLocation = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE" - -// verifySCT verifies the SCT against the Fulcio CT log public key. -// By default this comes from TUF, but you can override this (for test) -// purposes by using an env variable `SIGSTORE_CT_LOG_PUBLIC_KEY_FILE`. If using -// an alternate, the file can be PEM, or DER format. -// -// The SCT is a `Signed Certificate Timestamp`, which promises that -// the certificate issued by Fulcio was also added to the public CT log within -// some defined time period -func verifySCT(ctx context.Context, certPEM, rawSCT []byte) error { - pubKeys := make(map[crypto.PublicKey]tuf.StatusKind) - rootEnv := os.Getenv(altCTLogPublicKeyLocation) - if rootEnv == "" { - tufClient, err := tuf.NewFromEnv(ctx) - if err != nil { - return err - } - defer tufClient.Close() - - targets, err := tufClient.GetTargetsByMeta(tuf.CTFE, []string{ctPublicKeyStr}) - if err != nil { - return err - } - for _, t := range targets { - ctPub, err := cosign.PemToECDSAKey(t.Target) - if err != nil { - return errors.Wrap(err, "converting Public CT to ECDSAKey") - } - pubKeys[ctPub] = t.Status - } - } else { - fmt.Fprintf(os.Stderr, "**Warning** Using a non-standard public key for verifying SCT: %s\n", rootEnv) - raw, err := os.ReadFile(rootEnv) - if err != nil { - return errors.Wrap(err, "error reading alternate public key file") - } - pubKey, err := getAlternatePublicKey(raw) - if err != nil { - return errors.Wrap(err, "error parsing alternate public key from the file") - } - pubKeys[pubKey] = tuf.Active - } - if len(pubKeys) == 0 { - return errors.New("none of the CTFE keys have been found") - } - cert, err := x509util.CertificateFromPEM(certPEM) - if err != nil { - return err - } - var addChainResp ct.AddChainResponse - if err := json.Unmarshal(rawSCT, &addChainResp); err != nil { - return errors.Wrap(err, "unmarshal") - } - sct, err := addChainResp.ToSignedCertificateTimestamp() - if err != nil { - return err - } - var verifySctErr error - for pubKey, status := range pubKeys { - verifySctErr = ctutil.VerifySCT(pubKey, []*ctx509.Certificate{cert}, sct, false) - // Exit after successful verification of the SCT - if verifySctErr == nil { - if status != tuf.Active { - fmt.Fprintf(os.Stderr, "**Info** Successfully verified SCT using an expired verification key\n") - } - return nil - } - } - // Return the last error that occurred during verification. - return verifySctErr -} - func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient api.Client) (*fulcio.Signer, error) { fs, err := fulcio.NewSigner(ctx, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL, fClient) if err != nil { @@ -121,35 +34,10 @@ func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClien } // verify the sct - if err := verifySCT(ctx, fs.Cert, fs.SCT); err != nil { + if err := ctl.VerifySCT(ctx, fs.Cert, fs.Chain, fs.SCT); err != nil { return nil, errors.Wrap(err, "verifying SCT") } fmt.Fprintln(os.Stderr, "Successfully verified SCT...") return fs, nil } - -// Given a byte array, try to construct a public key from it. -// Will try first to see if it's PEM formatted, if not, then it will -// try to parse it as der publics, and failing that -func getAlternatePublicKey(in []byte) (crypto.PublicKey, error) { - var pubKey crypto.PublicKey - var err error - var derBytes []byte - pemBlock, _ := pem.Decode(in) - if pemBlock == nil { - fmt.Fprintf(os.Stderr, "Failed to decode non-standard public key for verifying SCT using PEM decode, trying as DER") - derBytes = in - } else { - derBytes = pemBlock.Bytes - } - pubKey, err = x509.ParsePKIXPublicKey(derBytes) - if err != nil { - // Try using the PKCS1 before giving up. - pubKey, err = x509.ParsePKCS1PublicKey(derBytes) - if err != nil { - return nil, errors.Wrap(err, "failed to parse alternate public key") - } - } - return pubKey, nil -} diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier_test.go b/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier_test.go deleted file mode 100644 index a4588e7ace5..00000000000 --- a/cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier_test.go +++ /dev/null @@ -1,73 +0,0 @@ -// -// Copyright 2021 The Sigstore Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package fulcioverifier - -import ( - "fmt" - "os" - "reflect" - "strings" - "testing" -) - -func TestGetAlternatePublicKey(t *testing.T) { - wd, err := os.Getwd() - if err != nil { - t.Fatalf("Failed to get cwd: %v", err) - } - tests := []struct { - file string - wantErrSub string - wantType string - }{ - {file: "garbage-there-are-limits", wantErrSub: "failed to parse"}, - // Testflume 2021 from here, https://letsencrypt.org/docs/ct-logs/ - {file: "letsencrypt-testflume-2021", wantType: "*ecdsa.PublicKey"}, - // This needs to be parsed with the pkcs1, pkix won't do. - {file: "rsa", wantType: "*rsa.PublicKey"}, - // This works with pkix, from: - // https://www.gstatic.com/ct/log_list/v2/log_list_pubkey.pem - {file: "google", wantType: "*rsa.PublicKey"}, - } - for _, tc := range tests { - filepath := fmt.Sprintf("%s/testdata/%s", wd, tc.file) - bytes, err := os.ReadFile(filepath) - if err != nil { - t.Fatalf("Failed to read testfile %s : %v", tc.file, err) - } - got, err := getAlternatePublicKey(bytes) - switch { - case err == nil && tc.wantErrSub != "": - t.Errorf("Wanted Error for %s but got none", tc.file) - case err != nil && tc.wantErrSub == "": - t.Errorf("Did not want error for %s but got: %v", tc.file, err) - case err != nil && tc.wantErrSub != "": - if !strings.Contains(err.Error(), tc.wantErrSub) { - t.Errorf("Unexpected error for %s: %s wanted to contain: %s", tc.file, err.Error(), tc.wantErrSub) - } - } - switch { - case got == nil && tc.wantType != "": - t.Errorf("Wanted public key for %s but got none", tc.file) - case got != nil && tc.wantType == "": - t.Errorf("Did not want error for %s but got: %v", tc.file, err) - case got != nil && tc.wantType != "": - if reflect.TypeOf(got).String() != tc.wantType { - t.Errorf("Unexpected type for %s: %+T wanted: %s", tc.file, got, tc.wantType) - } - } - } -} diff --git a/cmd/cosign/cli/sign/sign.go b/cmd/cosign/cli/sign/sign.go index 4f3a12843ed..44da104baf8 100644 --- a/cmd/cosign/cli/sign/sign.go +++ b/cmd/cosign/cli/sign/sign.go @@ -32,6 +32,7 @@ import ( "github.com/sigstore/cosign/cmd/cosign/cli/fulcio" "github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier" + "github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl" "github.com/sigstore/cosign/cmd/cosign/cli/options" "github.com/sigstore/cosign/cmd/cosign/cli/rekor" icos "github.com/sigstore/cosign/internal/pkg/cosign" @@ -414,9 +415,22 @@ func signerFromKeyRef(ctx context.Context, certPath, certChainPath, keyRef strin for _, c := range certChain[:len(certChain)-1] { subPool.AddCert(c) } - if err := cosign.TrustedCert(leafCert, rootPool, subPool); err != nil { + if _, err := cosign.TrustedCert(leafCert, rootPool, subPool); err != nil { return nil, errors.Wrap(err, "unable to validate certificate chain") } + // Verify SCT if present in the leaf certificate. + contains, err := ctl.ContainsSCT(leafCert.Raw) + if err != nil { + return nil, err + } + if contains { + var chain []*x509.Certificate + chain = append(chain, leafCert) + chain = append(chain, certChain...) + if err := ctl.VerifyEmbeddedSCT(context.Background(), chain); err != nil { + return nil, err + } + } certSigner.Chain = certChainBytes return certSigner, nil diff --git a/cmd/cosign/policy_webhook/depcheck_test.go b/cmd/cosign/policy_webhook/depcheck_test.go index 397a710baef..c990d4211b5 100644 --- a/cmd/cosign/policy_webhook/depcheck_test.go +++ b/cmd/cosign/policy_webhook/depcheck_test.go @@ -26,7 +26,7 @@ func TestNoDeps(t *testing.T) { "github.com/sigstore/cosign/cmd/cosign/policy_webhook": { // This conflicts with klog, we error on startup about // `-log_dir` being defined multiple times. - "github.com/golang/glog", + // "github.com/golang/glog", }, }) } diff --git a/cmd/cosign/webhook/depcheck_test.go b/cmd/cosign/webhook/depcheck_test.go index 30f64db6177..d8f5d7403c6 100644 --- a/cmd/cosign/webhook/depcheck_test.go +++ b/cmd/cosign/webhook/depcheck_test.go @@ -26,7 +26,7 @@ func TestNoDeps(t *testing.T) { "github.com/sigstore/cosign/cmd/cosign/webhook": { // This conflicts with klog, we error on startup about // `-log_dir` being defined multiple times. - "github.com/golang/glog", + // "github.com/golang/glog", }, }) } diff --git a/go.mod b/go.mod index f88e3b62b92..3ba9fbb0939 100644 --- a/go.mod +++ b/go.mod @@ -313,3 +313,5 @@ require ( sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect ) + +exclude github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b diff --git a/go.sum b/go.sum index 5732ae23fbe..b743879f0cc 100644 --- a/go.sum +++ b/go.sum @@ -1052,7 +1052,6 @@ github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzw github.com/golang-jwt/jwt/v4 v4.3.0 h1:kHL1vqdqWNfATmA0FNMdmZNMyZI1U6O31X4rlIPoBog= github.com/golang-jwt/jwt/v4 v4.3.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= -github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/glog v0.0.0-20210429001901-424d2337a529/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/glog v1.0.0 h1:nfP3RFugxnNRyKgeWd4oI1nYvXpxrx8ck8ZrcizshdQ= github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4= diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index 10d7aedd119..336de269445 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -30,6 +30,7 @@ import ( "strings" "time" + "github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl" cbundle "github.com/sigstore/cosign/pkg/cosign/bundle" "github.com/sigstore/cosign/pkg/cosign/tuf" @@ -151,7 +152,8 @@ func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Ver } // Now verify the cert, then the signature. - if err := TrustedCert(cert, co.RootCerts, co.IntermediateCerts); err != nil { + chains, err := TrustedCert(cert, co.RootCerts, co.IntermediateCerts) + if err != nil { return nil, err } if co.CertEmail != "" { @@ -178,6 +180,20 @@ func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Ver return nil, errors.New("expected oidc issuer not found in certificate") } } + // TODO: Add flag in CheckOpts to enforce embedded SCT + contains, err := ctl.ContainsSCT(cert.Raw) + if err != nil { + return nil, err + } + if contains { + // handle if chains has more than one chain - grab first and print message + if len(chains) > 1 { + fmt.Fprintf(os.Stderr, "**Info** Multiple valid certificate chains found. Selecting the first to verify the SCT.\n") + } + if err := ctl.VerifyEmbeddedSCT(context.Background(), chains[0]); err != nil { + return nil, err + } + } return verifier, nil } @@ -833,8 +849,8 @@ func VerifySET(bundlePayload cbundle.RekorPayload, signature []byte, pub *ecdsa. return nil } -func TrustedCert(cert *x509.Certificate, roots *x509.CertPool, intermediates *x509.CertPool) error { - if _, err := cert.Verify(x509.VerifyOptions{ +func TrustedCert(cert *x509.Certificate, roots *x509.CertPool, intermediates *x509.CertPool) ([][]*x509.Certificate, error) { + chains, err := cert.Verify(x509.VerifyOptions{ // THIS IS IMPORTANT: WE DO NOT CHECK TIMES HERE // THE CERTIFICATE IS TREATED AS TRUSTED FOREVER // WE CHECK THAT THE SIGNATURES WERE CREATED DURING THIS WINDOW @@ -844,10 +860,11 @@ func TrustedCert(cert *x509.Certificate, roots *x509.CertPool, intermediates *x5 KeyUsages: []x509.ExtKeyUsage{ x509.ExtKeyUsageCodeSigning, }, - }); err != nil { - return err + }) + if err != nil { + return nil, err } - return nil + return chains, nil } func correctAnnotations(wanted, have map[string]interface{}) bool { diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go index 0ff80e7def6..5753c29633e 100644 --- a/pkg/cosign/verify_test.go +++ b/pkg/cosign/verify_test.go @@ -24,9 +24,11 @@ import ( "encoding/json" "encoding/pem" "io" + "os" "strings" "testing" + "github.com/google/certificate-transparency-go/testdata" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/in-toto/in-toto-golang/in_toto" "github.com/pkg/errors" @@ -34,6 +36,7 @@ import ( "github.com/sigstore/cosign/pkg/oci/static" "github.com/sigstore/cosign/pkg/types" "github.com/sigstore/cosign/test" + "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/sigstore/sigstore/pkg/signature" "github.com/stretchr/testify/require" ) @@ -284,6 +287,36 @@ func TestValidateAndUnpackCertSuccessAllowAllValues(t *testing.T) { } } +func TestValidateAndUnpackCertWithSCT(t *testing.T) { + chain, err := cryptoutils.UnmarshalCertificatesFromPEM([]byte(testdata.TestEmbeddedCertPEM + testdata.CACertPEM)) + if err != nil { + t.Fatalf("error unmarshalling certificate chain: %v", err) + } + + rootPool := x509.NewCertPool() + rootPool.AddCert(chain[1]) + co := &CheckOpts{ + RootCerts: rootPool, + } + + // write SCT verification key to disk + tmpPrivFile, err := os.CreateTemp(t.TempDir(), "cosign_verify_sct_*.key") + if err != nil { + t.Fatalf("failed to create temp key file: %v", err) + } + defer tmpPrivFile.Close() + if _, err := tmpPrivFile.Write([]byte(testdata.LogPublicKeyPEM)); err != nil { + t.Fatalf("failed to write key file: %v", err) + } + os.Setenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE", tmpPrivFile.Name()) + defer os.Unsetenv("SIGSTORE_CT_LOG_PUBLIC_KEY_FILE") + + _, err = ValidateAndUnpackCert(chain[0], co) + if err != nil { + t.Errorf("ValidateAndUnpackCert expected no error, got err = %v", err) + } +} + func TestValidateAndUnpackCertInvalidRoot(t *testing.T) { subject := "email@email" oidcIssuer := "https://accounts.google.com" @@ -467,10 +500,16 @@ func TestTrustedCertSuccess(t *testing.T) { subPool := x509.NewCertPool() subPool.AddCert(subCert) - err := TrustedCert(leafCert, rootPool, subPool) + chains, err := TrustedCert(leafCert, rootPool, subPool) if err != nil { t.Fatalf("expected no error verifying certificate, got %v", err) } + if len(chains) != 1 { + t.Fatalf("unexpected number of chains found, expected 1, got %v", len(chains)) + } + if len(chains[0]) != 3 { + t.Fatalf("unexpected number of certs in chain, expected 3, got %v", len(chains[0])) + } } func TestTrustedCertSuccessNoIntermediates(t *testing.T) { @@ -480,7 +519,7 @@ func TestTrustedCertSuccessNoIntermediates(t *testing.T) { rootPool := x509.NewCertPool() rootPool.AddCert(rootCert) - err := TrustedCert(leafCert, rootPool, nil) + _, err := TrustedCert(leafCert, rootPool, nil) if err != nil { t.Fatalf("expected no error verifying certificate, got %v", err) } @@ -498,7 +537,7 @@ func TestTrustedCertSuccessChainFromRoot(t *testing.T) { subPool := x509.NewCertPool() subPool.AddCert(subCert) - err := TrustedCert(leafCert, rootPool, subPool) + _, err := TrustedCert(leafCert, rootPool, subPool) if err != nil { t.Fatalf("expected no error verifying certificate, got %v", err) }