Skip to content

Commit

Permalink
Support non-ECDSA key types for verify-blob (#2203)
Browse files Browse the repository at this point in the history 
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 <hblauzvern@google.com>

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
  • Loading branch information
@haydentherapper
haydentherapper authored Aug 26, 2022
1 parent 0416187 commit 31e6654
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 90 deletions.
30 changes: 2 additions & 28 deletions cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"crypto/sha256"
"crypto/x509"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -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
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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
}
50 changes: 0 additions & 50 deletions cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"encoding/json"
"fmt"
"os"
"reflect"
"strings"
"testing"

Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
9 changes: 4 additions & 5 deletions pkg/signature/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 31e6654

Please sign in to comment.