From: Patrick Zheng Date: Fri, 29 Nov 2024 10:47:49 +0800 Subject: fix: add tsa cert chain revocation check after timestamping (#246) Signed-off-by: Patrick Zheng --- internal/algorithm/algorithm.go | 121 ++++++++++++++ internal/algorithm/algorithm_test.go | 244 ++++++++++++++++++++++++++++ internal/timestamp/timestamp.go | 53 ++++++ signature/algorithm.go | 104 ++---------- signature/algorithm_test.go | 128 --------------- signature/cose/envelope_test.go | 6 +- signature/jws/envelope_test.go | 4 +- signature/types.go | 6 + x509/helper.go | 7 +- 11 files changed, 449 insertions(+), 224 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 0000000..1923a17 --- /dev/null +++ b/internal/algorithm/algorithm.go @@ -0,0 +1,121 @@ +// 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 includes signature algorithms accepted by Notary Project +package algorithm + +import ( + "crypto" + "crypto/ecdsa" + "crypto/rsa" + "crypto/x509" + "errors" + "fmt" +) + +// Algorithm defines the signature algorithm. +type Algorithm int + +// Signature algorithms supported by this library. +// +// Reference: https://github.com/notaryproject/notaryproject/blob/main/specs/signature-specification.md#algorithm-selection +const ( + AlgorithmPS256 Algorithm = 1 + iota // RSASSA-PSS with SHA-256 + AlgorithmPS384 // RSASSA-PSS with SHA-384 + AlgorithmPS512 // RSASSA-PSS with SHA-512 + AlgorithmES256 // ECDSA on secp256r1 with SHA-256 + AlgorithmES384 // ECDSA on secp384r1 with SHA-384 + AlgorithmES512 // ECDSA on secp521r1 with SHA-512 +) + +// Hash returns the hash function of the algorithm. +func (alg Algorithm) Hash() crypto.Hash { + switch alg { + case AlgorithmPS256, AlgorithmES256: + return crypto.SHA256 + case AlgorithmPS384, AlgorithmES384: + return crypto.SHA384 + case AlgorithmPS512, AlgorithmES512: + return crypto.SHA512 + } + return 0 +} + +// 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 +} + +// SignatureAlgorithm returns the signing algorithm associated with the KeySpec. +func (k KeySpec) SignatureAlgorithm() Algorithm { + switch k.Type { + case KeyTypeEC: + switch k.Size { + case 256: + return AlgorithmES256 + case 384: + return AlgorithmES384 + case 521: + return AlgorithmES512 + } + case KeyTypeRSA: + switch k.Size { + case 2048: + return AlgorithmPS256 + case 3072: + return AlgorithmPS384 + case 4096: + return AlgorithmPS512 + } + } + return 0 +} + +// 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 0000000..e2a1e02 --- /dev/null +++ b/internal/algorithm/algorithm_test.go @@ -0,0 +1,244 @@ +// 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" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "reflect" + "strconv" + "testing" + + "github.com/notaryproject/notation-core-go/testhelper" +) + +func TestHash(t *testing.T) { + tests := []struct { + name string + alg Algorithm + expect crypto.Hash + }{ + { + name: "PS256", + alg: AlgorithmPS256, + expect: crypto.SHA256, + }, + { + name: "ES256", + alg: AlgorithmES256, + expect: crypto.SHA256, + }, + { + name: "PS384", + alg: AlgorithmPS384, + expect: crypto.SHA384, + }, + { + name: "ES384", + alg: AlgorithmES384, + expect: crypto.SHA384, + }, + { + name: "PS512", + alg: AlgorithmPS512, + expect: crypto.SHA512, + }, + { + name: "ES512", + alg: AlgorithmES512, + expect: crypto.SHA512, + }, + { + name: "UnsupportedAlgorithm", + alg: 0, + expect: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hash := tt.alg.Hash() + if hash != tt.expect { + t.Fatalf("Expected %v, got %v", tt.expect, hash) + } + }) + } +} + +func TestSignatureAlgorithm(t *testing.T) { + tests := []struct { + name string + keySpec KeySpec + expect Algorithm + }{ + { + name: "EC 256", + keySpec: KeySpec{ + Type: KeyTypeEC, + Size: 256, + }, + expect: AlgorithmES256, + }, + { + name: "EC 384", + keySpec: KeySpec{ + Type: KeyTypeEC, + Size: 384, + }, + expect: AlgorithmES384, + }, + { + name: "EC 521", + keySpec: KeySpec{ + Type: KeyTypeEC, + Size: 521, + }, + expect: AlgorithmES512, + }, + { + name: "RSA 2048", + keySpec: KeySpec{ + Type: KeyTypeRSA, + Size: 2048, + }, + expect: AlgorithmPS256, + }, + { + name: "RSA 3072", + keySpec: KeySpec{ + Type: KeyTypeRSA, + Size: 3072, + }, + expect: AlgorithmPS384, + }, + { + name: "RSA 4096", + keySpec: KeySpec{ + Type: KeyTypeRSA, + Size: 4096, + }, + expect: AlgorithmPS512, + }, + { + name: "Unsupported key spec", + keySpec: KeySpec{ + Type: 0, + Size: 0, + }, + expect: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + alg := tt.keySpec.SignatureAlgorithm() + if alg != tt.expect { + t.Errorf("unexpected signature algorithm: %v, expect: %v", alg, tt.expect) + } + }) + } +} + +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/internal/timestamp/timestamp.go b/internal/timestamp/timestamp.go index bfb4ce8..853b9e2 100644 --- a/internal/timestamp/timestamp.go +++ b/internal/timestamp/timestamp.go @@ -16,7 +16,11 @@ package timestamp import ( "crypto/x509" + "errors" + "fmt" + "github.com/notaryproject/notation-core-go/revocation" + "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/signature" nx509 "github.com/notaryproject/notation-core-go/x509" "github.com/notaryproject/tspclient-go" @@ -61,5 +65,54 @@ func Timestamp(req *signature.SignRequest, opts tspclient.RequestOptions) ([]byt if err := nx509.ValidateTimestampingCertChain(tsaCertChain); err != nil { return nil, err } + // certificate chain revocation check after timestamping + if req.TSARevocationValidator != nil { + certResults, err := req.TSARevocationValidator.ValidateContext(ctx, revocation.ValidateContextOptions{ + CertChain: tsaCertChain, + }) + if err != nil { + return nil, fmt.Errorf("failed to validate the revocation status of timestamping certificate chain with error: %w", err) + } + if err := revocationResult(certResults, tsaCertChain); err != nil { + return nil, err + } + } return resp.TimestampToken.FullBytes, nil } + +// revocationResult returns an error if any cert in the cert chain has +// a revocation status other than ResultOK or ResultNonRevokable. +// When ResultRevoked presents, always return the revoked error. +func revocationResult(certResults []*result.CertRevocationResult, certChain []*x509.Certificate) error { + //sanity check + if len(certResults) == 0 { + return errors.New("certificate revocation result cannot be empty") + } + if len(certResults) != len(certChain) { + return fmt.Errorf("length of certificate revocation result %d does not match length of the certificate chain %d", len(certResults), len(certChain)) + } + + numOKResults := 0 + var problematicCertSubject string + var hasUnknownResult bool + for i := len(certResults) - 1; i >= 0; i-- { + cert := certChain[i] + certResult := certResults[i] + if certResult.Result == result.ResultOK || certResult.Result == result.ResultNonRevokable { + numOKResults++ + } else { + if certResult.Result == result.ResultRevoked { // revoked + return fmt.Errorf("timestamping certificate with subject %q is revoked", cert.Subject.String()) + } + if !hasUnknownResult { // unknown + // not returning because a following cert can be revoked + problematicCertSubject = cert.Subject.String() + hasUnknownResult = true + } + } + } + if numOKResults != len(certResults) { + return fmt.Errorf("timestamping certificate with subject %q revocation status is unknown", problematicCertSubject) + } + return nil +} diff --git a/signature/algorithm.go b/signature/algorithm.go index 8e39192..40dbfb7 100644 --- a/signature/algorithm.go +++ b/signature/algorithm.go @@ -14,112 +14,44 @@ package signature import ( - "crypto" - "crypto/ecdsa" - "crypto/rsa" "crypto/x509" - "fmt" + + "github.com/notaryproject/notation-core-go/internal/algorithm" ) // Algorithm defines the signature algorithm. -type Algorithm int +type Algorithm = algorithm.Algorithm // Signature algorithms supported by this library. // // Reference: https://github.com/notaryproject/notaryproject/blob/main/specs/signature-specification.md#algorithm-selection const ( - AlgorithmPS256 Algorithm = 1 + iota // RSASSA-PSS with SHA-256 - AlgorithmPS384 // RSASSA-PSS with SHA-384 - AlgorithmPS512 // RSASSA-PSS with SHA-512 - AlgorithmES256 // ECDSA on secp256r1 with SHA-256 - AlgorithmES384 // ECDSA on secp384r1 with SHA-384 - AlgorithmES512 // ECDSA on secp521r1 with SHA-512 + AlgorithmPS256 = algorithm.AlgorithmPS256 // RSASSA-PSS with SHA-256 + AlgorithmPS384 = algorithm.AlgorithmPS384 // RSASSA-PSS with SHA-384 + AlgorithmPS512 = algorithm.AlgorithmPS512 // RSASSA-PSS with SHA-512 + AlgorithmES256 = algorithm.AlgorithmES256 // ECDSA on secp256r1 with SHA-256 + AlgorithmES384 = algorithm.AlgorithmES384 // ECDSA on secp384r1 with SHA-384 + AlgorithmES512 = algorithm.AlgorithmES512 // ECDSA on secp521r1 with SHA-512 ) // KeyType defines the key type. -type KeyType int +type KeyType = algorithm.KeyType const ( - KeyTypeRSA KeyType = 1 + iota // KeyType RSA - KeyTypeEC // KeyType EC + KeyTypeRSA = algorithm.KeyTypeRSA // KeyType RSA + KeyTypeEC = algorithm.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 -} - -// Hash returns the hash function of the algorithm. -func (alg Algorithm) Hash() crypto.Hash { - switch alg { - case AlgorithmPS256, AlgorithmES256: - return crypto.SHA256 - case AlgorithmPS384, AlgorithmES384: - return crypto.SHA384 - case AlgorithmPS512, AlgorithmES512: - return crypto.SHA512 - } - return 0 -} +type KeySpec = algorithm.KeySpec // 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), - } - } - } - return KeySpec{}, &UnsupportedSigningKeyError{ - Msg: "unsupported public key type", - } -} - -// SignatureAlgorithm returns the signing algorithm associated with the KeySpec. -func (k KeySpec) SignatureAlgorithm() Algorithm { - switch k.Type { - case KeyTypeEC: - switch k.Size { - case 256: - return AlgorithmES256 - case 384: - return AlgorithmES384 - case 521: - return AlgorithmES512 - } - case KeyTypeRSA: - switch k.Size { - case 2048: - return AlgorithmPS256 - case 3072: - return AlgorithmPS384 - case 4096: - return AlgorithmPS512 + ks, err := algorithm.ExtractKeySpec(signingCert) + if err != nil { + return KeySpec{}, &UnsupportedSigningKeyError{ + Msg: err.Error(), } } - return 0 + return ks, nil } diff --git a/signature/algorithm_test.go b/signature/algorithm_test.go index 7e4e238..da41d1f 100644 --- a/signature/algorithm_test.go +++ b/signature/algorithm_test.go @@ -14,7 +14,6 @@ package signature import ( - "crypto" "crypto/ecdsa" "crypto/ed25519" "crypto/elliptic" @@ -28,59 +27,6 @@ import ( "github.com/notaryproject/notation-core-go/testhelper" ) -func TestHash(t *testing.T) { - tests := []struct { - name string - alg Algorithm - expect crypto.Hash - }{ - { - name: "PS256", - alg: AlgorithmPS256, - expect: crypto.SHA256, - }, - { - name: "ES256", - alg: AlgorithmES256, - expect: crypto.SHA256, - }, - { - name: "PS384", - alg: AlgorithmPS384, - expect: crypto.SHA384, - }, - { - name: "ES384", - alg: AlgorithmES384, - expect: crypto.SHA384, - }, - { - name: "PS512", - alg: AlgorithmPS512, - expect: crypto.SHA512, - }, - { - name: "ES512", - alg: AlgorithmES512, - expect: crypto.SHA512, - }, - { - name: "UnsupportedAlgorithm", - alg: 0, - expect: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - hash := tt.alg.Hash() - if hash != tt.expect { - t.Fatalf("Expected %v, got %v", tt.expect, hash) - } - }) - } -} - func TestExtractKeySpec(t *testing.T) { type testCase struct { name string @@ -168,77 +114,3 @@ func TestExtractKeySpec(t *testing.T) { }) } } - -func TestSignatureAlgorithm(t *testing.T) { - tests := []struct { - name string - keySpec KeySpec - expect Algorithm - }{ - { - name: "EC 256", - keySpec: KeySpec{ - Type: KeyTypeEC, - Size: 256, - }, - expect: AlgorithmES256, - }, - { - name: "EC 384", - keySpec: KeySpec{ - Type: KeyTypeEC, - Size: 384, - }, - expect: AlgorithmES384, - }, - { - name: "EC 521", - keySpec: KeySpec{ - Type: KeyTypeEC, - Size: 521, - }, - expect: AlgorithmES512, - }, - { - name: "RSA 2048", - keySpec: KeySpec{ - Type: KeyTypeRSA, - Size: 2048, - }, - expect: AlgorithmPS256, - }, - { - name: "RSA 3072", - keySpec: KeySpec{ - Type: KeyTypeRSA, - Size: 3072, - }, - expect: AlgorithmPS384, - }, - { - name: "RSA 4096", - keySpec: KeySpec{ - Type: KeyTypeRSA, - Size: 4096, - }, - expect: AlgorithmPS512, - }, - { - name: "Unsupported key spec", - keySpec: KeySpec{ - Type: 0, - Size: 0, - }, - expect: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - alg := tt.keySpec.SignatureAlgorithm() - if alg != tt.expect { - t.Errorf("unexpected signature algorithm: %v, expect: %v", alg, tt.expect) - } - }) - } -} diff --git a/signature/cose/envelope_test.go b/signature/cose/envelope_test.go index bccae19..31e1ae9 100644 --- a/signature/cose/envelope_test.go +++ b/signature/cose/envelope_test.go @@ -33,7 +33,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 ( @@ -128,7 +128,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) diff --git a/signature/jws/envelope_test.go b/signature/jws/envelope_test.go index d8f59a8..106511d 100644 --- a/signature/jws/envelope_test.go +++ b/signature/jws/envelope_test.go @@ -36,7 +36,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 { diff --git a/signature/types.go b/signature/types.go index df69236..31ccef9 100644 --- a/signature/types.go +++ b/signature/types.go @@ -20,6 +20,7 @@ import ( "fmt" "time" + "github.com/notaryproject/notation-core-go/revocation" "github.com/notaryproject/tspclient-go" ) @@ -112,6 +113,11 @@ type SignRequest struct { // TSARootCAs is the set of caller trusted TSA root certificates TSARootCAs *x509.CertPool + // TSARevocationValidator is used for timestamping certificate + // chain revocation check after signing. + // When present, only used when timestamping is performed. + TSARevocationValidator revocation.Validator + // ctx is the caller context. It should only be modified via WithContext. // It is unexported to prevent people from using Context wrong // and mutating the contexts held by callers of the same request. diff --git a/x509/helper.go b/x509/helper.go index 91d3443..e511ab3 100644 --- a/x509/helper.go +++ b/x509/helper.go @@ -20,7 +20,7 @@ import ( "strings" "time" - "github.com/notaryproject/notation-core-go/signature" + "github.com/notaryproject/notation-core-go/internal/algorithm" ) func isSelfSigned(cert *x509.Certificate) (bool, error) { @@ -95,13 +95,10 @@ func validateLeafKeyUsage(cert *x509.Certificate) error { } func validateSignatureAlgorithm(cert *x509.Certificate) error { - keySpec, err := signature.ExtractKeySpec(cert) + _, err := algorithm.ExtractKeySpec(cert) if err != nil { return fmt.Errorf("certificate with subject %q: %w", cert.Subject, err) } - if keySpec.SignatureAlgorithm() == 0 { - return fmt.Errorf("certificate with subject %q: unsupported signature algorithm with key spec %+v", cert.Subject, keySpec) - } return nil }