From 31e665415f2da47356e4657e751443fcf5f394ed Mon Sep 17 00:00:00 2001 From: Hayden B Date: Fri, 26 Aug 2022 11:07:14 -0700 Subject: [PATCH] Support non-ECDSA key types for verify-blob (#2203) Also cleans up some code in the CT log verification to use the Sigstore function that handles multiple PEM formats. It removes DER encoding support, but that should not have been used. Signed-off-by: Hayden Blauzvern Signed-off-by: Hayden Blauzvern --- .../cli/fulcio/fulcioverifier/ctl/verify.go | 30 +---------- .../fulcio/fulcioverifier/ctl/verify_test.go | 50 ------------------- go.mod | 4 +- go.sum | 10 ++-- pkg/signature/keys.go | 9 ++-- 5 files changed, 13 insertions(+), 90 deletions(-) diff --git a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go index d3673eef55a..38287ab3e0d 100644 --- a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go +++ b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go @@ -20,7 +20,6 @@ import ( "crypto/sha256" "crypto/x509" "encoding/json" - "encoding/pem" "errors" "fmt" "os" @@ -87,7 +86,7 @@ func VerifySCT(ctx context.Context, certPEM, chainPEM, rawSCT []byte) error { return err } for _, t := range targets { - pub, err := getPublicKey(t.Target) + pub, err := cryptoutils.UnmarshalPEMToPublicKey(t.Target) if err != nil { return err } @@ -103,7 +102,7 @@ func VerifySCT(ctx context.Context, certPEM, chainPEM, rawSCT []byte) error { if err != nil { return fmt.Errorf("error reading alternate public key file") } - pubKey, err := getPublicKey(raw) + pubKey, err := cryptoutils.UnmarshalPEMToPublicKey(raw) if err != nil { return fmt.Errorf("error parsing alternate public key from the file") } @@ -196,28 +195,3 @@ func VerifyEmbeddedSCT(ctx context.Context, chain []*x509.Certificate) error { } return VerifySCT(ctx, certPEM, chainPEM, []byte{}) } - -// Given a byte array, try to construct a public key from it. -// Supports PEM encoded public keys, falling back to DER. Supports -// PKIX and PKCS1 encoded keys. -func getPublicKey(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, fmt.Errorf("failed to parse CT log public key: %w", err) - } - } - 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 index 6f6bd308784..027e5b67085 100644 --- a/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go +++ b/cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go @@ -23,7 +23,6 @@ import ( "encoding/json" "fmt" "os" - "reflect" "strings" "testing" @@ -33,55 +32,6 @@ import ( "github.com/sigstore/sigstore/pkg/cryptoutils" ) -func TestGetPublicKey(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 := getPublicKey(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)) diff --git a/go.mod b/go.mod index c85c4c2ee69..875cfbd9d94 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/secure-systems-lab/go-securesystemslib v0.4.0 github.com/sigstore/fulcio v0.5.3 github.com/sigstore/rekor v0.11.0 - github.com/sigstore/sigstore v1.4.0 + github.com/sigstore/sigstore v1.4.1-0.20220823190236-c645ceb9d075 github.com/spf13/cobra v1.5.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.12.0 @@ -80,7 +80,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/ecr v1.15.0 // indirect github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.12.0 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.12 // indirect - github.com/aws/aws-sdk-go-v2/service/kms v1.18.4 // indirect + github.com/aws/aws-sdk-go-v2/service/kms v1.18.5 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.11.17 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.16.13 // indirect github.com/aws/smithy-go v1.12.1 // indirect diff --git a/go.sum b/go.sum index 16834150b43..8ca58e9b614 100644 --- a/go.sum +++ b/go.sum @@ -272,7 +272,7 @@ github.com/aws/aws-sdk-go v1.27.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN github.com/aws/aws-sdk-go v1.36.30/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= github.com/aws/aws-sdk-go v1.37.0/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= github.com/aws/aws-sdk-go v1.43.16/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= -github.com/aws/aws-sdk-go v1.44.80 h1:jEXGecSgPdvM5KnyDsSgFhZSm7WwaTp4h544Im4SfhI= +github.com/aws/aws-sdk-go v1.44.83 h1:7+Rtc2Eio6EKUNoZeMV/IVxzVrY5oBQcNPtCcgIHYJA= github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g= github.com/aws/aws-sdk-go-v2 v1.7.1/go.mod h1:L5LuPC1ZgDr2xQS7AmIec/Jlc7O/Y1u2KxJyNVab250= github.com/aws/aws-sdk-go-v2 v1.14.0/go.mod h1:ZA3Y8V0LrlWj63MQAnRHgKf/5QB//LSZCPNWlWrNGLU= @@ -305,8 +305,8 @@ github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.12.0/go.mod h1:IArQ3IBR00Fkura github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.2.1/go.mod h1:zceowr5Z1Nh2WVP8bf/3ikB41IZW59E4yIYbg+pC6mw= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.12 h1:7iPTTX4SAI2U2VOogD7/gmHlsgnYSgoNHt7MSQXtG2M= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.12/go.mod h1:1TODGhheLWjpQWSuhYuAUWYTCKwEjx2iblIFKDHjeTc= -github.com/aws/aws-sdk-go-v2/service/kms v1.18.4 h1:tsokBawk9+eD3RfMbJJRla/y8FinZ79Ylj5tZ3Ayxcw= -github.com/aws/aws-sdk-go-v2/service/kms v1.18.4/go.mod h1:WG8HUJKtDqXJM3+CNZeN+2wvdcJb5vprKo01fr1KQW4= +github.com/aws/aws-sdk-go-v2/service/kms v1.18.5 h1:bgIax/BQB8/U8qXavwKFOPzbnSAAQjPQ5hDM4FwN2gA= +github.com/aws/aws-sdk-go-v2/service/kms v1.18.5/go.mod h1:WG8HUJKtDqXJM3+CNZeN+2wvdcJb5vprKo01fr1KQW4= github.com/aws/aws-sdk-go-v2/service/sso v1.3.1/go.mod h1:J3A3RGUvuCZjvSuZEcOpHDnzZP/sKbhDWV2T1EOzFIM= github.com/aws/aws-sdk-go-v2/service/sso v1.11.17 h1:pXxu9u2z1UqSbjO9YA8kmFJBhFc1EVTDaf7A+S+Ivq8= github.com/aws/aws-sdk-go-v2/service/sso v1.11.17/go.mod h1:mS5xqLZc/6kc06IpXn5vRxdLaED+jEuaSRv5BxtnsiY= @@ -1714,8 +1714,8 @@ 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.0 h1:5A3eUhbSQkhiqJNUPi/2UMKdTyb3NKfWcVjaTBkkaJk= -github.com/sigstore/sigstore v1.4.0/go.mod h1:z3kt1jm2A39M+g7emkQ8jdErL/haCMEjkNxvqTf41/k= +github.com/sigstore/sigstore v1.4.1-0.20220823190236-c645ceb9d075 h1:pQzybny/k/kdos7vYwB/7eHFEtbUZKYsTvbjqyoOWz4= +github.com/sigstore/sigstore v1.4.1-0.20220823190236-c645ceb9d075/go.mod h1:RSHR6Id/5Cxvs8ptISpVWiD2pXRK9LU8MNX5fJO1r64= github.com/sirupsen/logrus v1.0.4-0.20170822132746-89742aefa4b2/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= github.com/sirupsen/logrus v1.0.6/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= diff --git a/pkg/signature/keys.go b/pkg/signature/keys.go index 724777735a8..21f22ce5ef8 100644 --- a/pkg/signature/keys.go +++ b/pkg/signature/keys.go @@ -78,14 +78,13 @@ func loadKey(keyPath string, pf cosign.PassFunc) (signature.SignerVerifier, erro return cosign.LoadPrivateKey(kb, pass) } -// LoadPublicKeyRaw loads a verifier from a raw public key passed in +// LoadPublicKeyRaw loads a verifier from a PEM-encoded public key func LoadPublicKeyRaw(raw []byte, hashAlgorithm crypto.Hash) (signature.Verifier, error) { - // PEM encoded file. - ed, err := cosign.PemToECDSAKey(raw) + pub, err := cryptoutils.UnmarshalPEMToPublicKey(raw) if err != nil { - return nil, fmt.Errorf("pem to ecdsa: %w", err) + return nil, err } - return signature.LoadECDSAVerifier(ed, hashAlgorithm) + return signature.LoadVerifier(pub, hashAlgorithm) } func SignerFromKeyRef(ctx context.Context, keyRef string, pf cosign.PassFunc) (signature.Signer, error) {