From 3ef5c276a47b1a6914a67270812699771556607f Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 25 Nov 2024 18:18:12 +0800 Subject: [PATCH] refactor Signed-off-by: Patrick Zheng --- internal/algorithm/algorithm.go | 66 +++++++++++++++ internal/algorithm/algorithm_test.go | 116 +++++++++++++++++++++++++++ signature/algorithm.go | 40 +++------ signature/cose/envelope_test.go | 6 +- signature/jws/envelope_test.go | 4 +- x509/helper.go | 26 ++---- 6 files changed, 204 insertions(+), 54 deletions(-) create mode 100644 internal/algorithm/algorithm.go create mode 100644 internal/algorithm/algorithm_test.go diff --git a/internal/algorithm/algorithm.go b/internal/algorithm/algorithm.go new file mode 100644 index 00000000..0ee032e2 --- /dev/null +++ b/internal/algorithm/algorithm.go @@ -0,0 +1,66 @@ +// Copyright The Notary Project 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 algorithm + +import ( + "crypto/ecdsa" + "crypto/rsa" + "crypto/x509" + "errors" + "fmt" +) + +// KeyType defines the key type. +type KeyType int + +const ( + KeyTypeRSA KeyType = 1 + iota // KeyType RSA + KeyTypeEC // KeyType EC +) + +// KeySpec defines a key type and size. +type KeySpec struct { + // KeyType is the type of the key. + Type KeyType + + // KeySize is the size of the key in bits. + Size int +} + +// ExtractKeySpec extracts KeySpec from the signing certificate. +func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) { + switch key := signingCert.PublicKey.(type) { + case *rsa.PublicKey: + switch bitSize := key.Size() << 3; bitSize { + case 2048, 3072, 4096: + return KeySpec{ + Type: KeyTypeRSA, + Size: bitSize, + }, nil + default: + return KeySpec{}, fmt.Errorf("rsa key size %d bits is not supported", bitSize) + } + case *ecdsa.PublicKey: + switch bitSize := key.Curve.Params().BitSize; bitSize { + case 256, 384, 521: + return KeySpec{ + Type: KeyTypeEC, + Size: bitSize, + }, nil + default: + return KeySpec{}, fmt.Errorf("ecdsa key size %d bits is not supported", bitSize) + } + } + return KeySpec{}, errors.New("unsupported public key type") +} diff --git a/internal/algorithm/algorithm_test.go b/internal/algorithm/algorithm_test.go new file mode 100644 index 00000000..72c4d7e4 --- /dev/null +++ b/internal/algorithm/algorithm_test.go @@ -0,0 +1,116 @@ +// Copyright The Notary Project 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 algorithm + +import ( + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "reflect" + "strconv" + "testing" + + "github.com/notaryproject/notation-core-go/testhelper" +) + +func TestExtractKeySpec(t *testing.T) { + type testCase struct { + name string + cert *x509.Certificate + expect KeySpec + expectErr bool + } + // invalid cases + tests := []testCase{ + { + name: "RSA wrong size", + cert: testhelper.GetUnsupportedRSACert().Cert, + expect: KeySpec{}, + expectErr: true, + }, + { + name: "ECDSA wrong size", + cert: testhelper.GetUnsupportedECCert().Cert, + expect: KeySpec{}, + expectErr: true, + }, + { + name: "Unsupported type", + cert: &x509.Certificate{ + PublicKey: ed25519.PublicKey{}, + }, + expect: KeySpec{}, + expectErr: true, + }, + } + + // append valid RSA cases + for _, k := range []int{2048, 3072, 4096} { + rsaRoot := testhelper.GetRSARootCertificate() + priv, _ := rsa.GenerateKey(rand.Reader, k) + + certTuple := testhelper.GetRSACertTupleWithPK( + priv, + "Test RSA_"+strconv.Itoa(priv.Size()), + &rsaRoot, + ) + tests = append(tests, testCase{ + name: "RSA " + strconv.Itoa(k), + cert: certTuple.Cert, + expect: KeySpec{ + Type: KeyTypeRSA, + Size: k, + }, + expectErr: false, + }) + } + + // append valid EDCSA cases + for _, curve := range []elliptic.Curve{elliptic.P256(), elliptic.P384(), elliptic.P521()} { + ecdsaRoot := testhelper.GetECRootCertificate() + priv, _ := ecdsa.GenerateKey(curve, rand.Reader) + bitSize := priv.Params().BitSize + + certTuple := testhelper.GetECDSACertTupleWithPK( + priv, + "Test EC_"+strconv.Itoa(bitSize), + &ecdsaRoot, + ) + tests = append(tests, testCase{ + name: "EC " + strconv.Itoa(bitSize), + cert: certTuple.Cert, + expect: KeySpec{ + Type: KeyTypeEC, + Size: bitSize, + }, + expectErr: false, + }) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keySpec, err := ExtractKeySpec(tt.cert) + + if (err != nil) != tt.expectErr { + t.Errorf("error = %v, expectErr = %v", err, tt.expectErr) + } + if !reflect.DeepEqual(keySpec, tt.expect) { + t.Errorf("expect %+v, got %+v", tt.expect, keySpec) + } + }) + } +} diff --git a/signature/algorithm.go b/signature/algorithm.go index 8e391924..f7ad12d9 100644 --- a/signature/algorithm.go +++ b/signature/algorithm.go @@ -15,10 +15,9 @@ package signature import ( "crypto" - "crypto/ecdsa" - "crypto/rsa" "crypto/x509" - "fmt" + + "github.com/notaryproject/notation-core-go/internal/algorithm" ) // Algorithm defines the signature algorithm. @@ -68,35 +67,16 @@ func (alg Algorithm) Hash() crypto.Hash { // ExtractKeySpec extracts KeySpec from the signing certificate. func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) { - switch key := signingCert.PublicKey.(type) { - case *rsa.PublicKey: - switch bitSize := key.Size() << 3; bitSize { - case 2048, 3072, 4096: - return KeySpec{ - Type: KeyTypeRSA, - Size: bitSize, - }, nil - default: - return KeySpec{}, &UnsupportedSigningKeyError{ - Msg: fmt.Sprintf("rsa key size %d bits is not supported", bitSize), - } - } - case *ecdsa.PublicKey: - switch bitSize := key.Curve.Params().BitSize; bitSize { - case 256, 384, 521: - return KeySpec{ - Type: KeyTypeEC, - Size: bitSize, - }, nil - default: - return KeySpec{}, &UnsupportedSigningKeyError{ - Msg: fmt.Sprintf("ecdsa key size %d bits is not supported", bitSize), - } + ks, err := algorithm.ExtractKeySpec(signingCert) + if err != nil { + return KeySpec{}, &UnsupportedSigningKeyError{ + Msg: err.Error(), } } - return KeySpec{}, &UnsupportedSigningKeyError{ - Msg: "unsupported public key type", - } + return KeySpec{ + Type: KeyType(ks.Type), + Size: ks.Size, + }, nil } // SignatureAlgorithm returns the signing algorithm associated with the KeySpec. diff --git a/signature/cose/envelope_test.go b/signature/cose/envelope_test.go index c1e9f545..d7faa96f 100644 --- a/signature/cose/envelope_test.go +++ b/signature/cose/envelope_test.go @@ -34,7 +34,7 @@ import ( const ( payloadString = "{\"targetArtifact\":{\"mediaType\":\"application/vnd.oci.image.manifest.v1+json\",\"digest\":\"sha256:73c803930ea3ba1e54bc25c2bdc53edd0284c62ed651fe7b00369da519a3c333\",\"size\":16724,\"annotations\":{\"io.wabbit-networks.buildId\":\"123\"}}}" - rfc3161TSAurl = "http://rfc3161timestamp.globalsign.com/advanced" + rfc3161TSAurl = "http://timestamp.digicert.com" ) var ( @@ -129,7 +129,7 @@ func TestSign(t *testing.T) { } } - t.Run("with timestmap countersignature request", func(t *testing.T) { + t.Run("with timestamp countersignature request", func(t *testing.T) { signRequest, err := newSignRequest("notary.x509", signature.KeyTypeRSA, 3072) if err != nil { t.Fatalf("newSignRequest() failed. Error = %s", err) @@ -138,7 +138,7 @@ func TestSign(t *testing.T) { if err != nil { t.Fatal(err) } - rootCerts, err := nx509.ReadCertificateFile("../../internal/timestamp/testdata/tsaRootCert.crt") + rootCerts, err := nx509.ReadCertificateFile("../../internal/timestamp/testdata/tsaRootCert.cer") if err != nil || len(rootCerts) == 0 { t.Fatal("failed to read root CA certificate:", err) } diff --git a/signature/jws/envelope_test.go b/signature/jws/envelope_test.go index 6075e5f6..d2ca93d5 100644 --- a/signature/jws/envelope_test.go +++ b/signature/jws/envelope_test.go @@ -37,7 +37,7 @@ import ( "github.com/notaryproject/tspclient-go" ) -const rfc3161TSAurl = "http://rfc3161timestamp.globalsign.com/advanced" +const rfc3161TSAurl = "http://timestamp.digicert.com" // remoteMockSigner is used to mock remote signer type remoteMockSigner struct { @@ -341,7 +341,7 @@ func TestSignWithTimestamp(t *testing.T) { if err != nil { t.Fatal(err) } - rootCerts, err := nx509.ReadCertificateFile("../../internal/timestamp/testdata/tsaRootCert.crt") + rootCerts, err := nx509.ReadCertificateFile("../../internal/timestamp/testdata/tsaRootCert.cer") if err != nil || len(rootCerts) == 0 { t.Fatal("failed to read root CA certificate:", err) } diff --git a/x509/helper.go b/x509/helper.go index 3ac76b57..e511ab35 100644 --- a/x509/helper.go +++ b/x509/helper.go @@ -15,12 +15,12 @@ package x509 import ( "bytes" - "crypto/ecdsa" - "crypto/rsa" "crypto/x509" "fmt" "strings" "time" + + "github.com/notaryproject/notation-core-go/internal/algorithm" ) func isSelfSigned(cert *x509.Certificate) (bool, error) { @@ -95,23 +95,11 @@ func validateLeafKeyUsage(cert *x509.Certificate) error { } func validateSignatureAlgorithm(cert *x509.Certificate) error { - switch key := cert.PublicKey.(type) { - case *rsa.PublicKey: - switch bitSize := key.Size() << 3; bitSize { - case 2048, 3072, 4096: - return nil - default: - return fmt.Errorf("certificate with subject %q: rsa key size %d bits is not supported", cert.Subject, bitSize) - } - case *ecdsa.PublicKey: - switch bitSize := key.Curve.Params().BitSize; bitSize { - case 256, 384, 521: - return nil - default: - return fmt.Errorf("certificate with subject %q: ecdsa key size %d bits is not supported", cert.Subject, bitSize) - } - } - return fmt.Errorf("certificate with subject %q: unsupported public key type", cert.Subject) + _, err := algorithm.ExtractKeySpec(cert) + if err != nil { + return fmt.Errorf("certificate with subject %q: %w", cert.Subject, err) + } + return nil } func ekuToString(eku x509.ExtKeyUsage) string {