From 95d89543c9f97f68352207dc7739b1722a1403bf Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 29 Nov 2024 10:47:49 +0800 Subject: [PATCH 1/2] 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/testdata/tsaRootCert.cer | Bin 0 -> 1428 bytes internal/timestamp/testdata/tsaRootCert.crt | Bin 1415 -> 0 bytes internal/timestamp/timestamp.go | 53 +++ internal/timestamp/timestamp_test.go | 354 ++++++++++++++++---- 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 +- 12 files changed, 746 insertions(+), 281 deletions(-) create mode 100644 internal/algorithm/algorithm.go create mode 100644 internal/algorithm/algorithm_test.go create mode 100644 internal/timestamp/testdata/tsaRootCert.cer delete mode 100644 internal/timestamp/testdata/tsaRootCert.crt diff --git a/internal/algorithm/algorithm.go b/internal/algorithm/algorithm.go new file mode 100644 index 00000000..1923a17d --- /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 00000000..e2a1e02d --- /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/testdata/tsaRootCert.cer b/internal/timestamp/testdata/tsaRootCert.cer new file mode 100644 index 0000000000000000000000000000000000000000..99bcc84b7e68b5b28e4444f6fa21bc7c2baf497d GIT binary patch literal 1428 zcmXqLVx3^n#9Xm}nTe5!Nq}{>bojhJMWaWS?0c7&m&O?IvTn?JkswJuAWqo zP1e*s_a@Ho#N;1}iL*^!vmT3k6D_sp^~v*R*O)lOZ>&mtSAN1{MOt|H{E&z~9_{V^ z%MEUZy*pJM`*`h1|G1~7&kaxCnjCkhufO5ewuv(wCR84-IKFM;k*!%07R&;@H?Ej3 z(PORc_}XMAFtK2DXp^JS_1i4PT6q&0YZQI1>{%zxTpC-EcGJqxWtOqSeva!=o=Xlr zTe%?p?h^Gq3;iv(3Py;3SBY`!Px*c@v!iTAnQdgOQ(1fG^vo)c4-XazNvF*!Id#ul z?m1ubx@TA3Pnu*k&-M<(6Ia#FZL?e?wd)Q{*>Wi{_qFlOqxZd87|ztnOg-HHU2)SU z!R@>2KV9u9&~Z#ywJ}-3WvWzJQr)+P4ZmNcEHl2?$^LNf_GivZBz7z-XMD&%g-20# zQ;4Q&XUw*5U*l71pZrEK z{)j?gcK*iIZQcHduDQm~Rrs?|?&yL3MH}n5)MkEtlBqvKR`=`8m78RrN;5GtGB7T7 zGH@{92PS7(VMfOPEUX61K+1p*B)|_6U;*Z-HUn7@pN~b1MdZ!($4!?CV^e(Y>!sU2 z-!)^M48K2eDg$OPU@Bu|*qwN@c4f{!@gozZ4=-HA(EB(ggFozi`MQFie`k5k+~v@ z;|G_o$XKF&XYNn+bq1|Fzoq+H+4VTN((ZIU8a!-?Xwiwjs2;$JM? zvV8rD@42RPYNEQXEwY&TxuW}v?-ZoX1m*e!D1M{-ku zW1+3RZ-H_fkJp{XOJ|IxwD59pPM7gN@Ge`S#Ng5cOA~=sMNvkM7okS?3O#RXhzIyS z+vj_+bj^iRza4itFI{!{FsqBdj@j%-zaF{nP!7&v%TEujciZY?pQjO3sdj0}ilph6 zfR&$*WHWvetKHnrfA0t)=$1ze_=^}`TkG{L*Rlgt&`^}&Rl)f&LXk-+o%1iN!lfsC)s~w6 z?Tp>inj~vqxWQ7QGU@KxG3!Bhh+^5 ouE}lQ!_OaFs=4ZwaQTyaJ&lTM*#+DM*S6cTUo72o{&QL#0GZBIfB*mh literal 0 HcmV?d00001 diff --git a/internal/timestamp/testdata/tsaRootCert.crt b/internal/timestamp/testdata/tsaRootCert.crt deleted file mode 100644 index 3492b9555d8a927fc048accbdfd510d973a4bf8a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1415 zcmXqLVr@2PV$NQ`%*4pV#OL~KH*>S`;nq}-Xa8LT4S3l&wc0$|zVk9N@~|=(_!ue} z$gwepvTzHFyXWL5CFTTYrspXH<>!|uI6ErnDg>Ds3L6N5RB#D%AymKwnUMtzrbf z7RhT}D$uKbzGw38+~7wZH9Z3C7XQ;Wl_%Ccc-3RGYRi&upQm~WvFHBn+0=Rd!h*!| z6A>jFt|(i5kF`3r==NOBd$BIM6FxSc4WIO@ZchM9vdR7I$rAgr%{Ey+S)*dM{loih zGQ#1Lx82+H<5BDO;zc`5SGj(AFxPBhen{8HqOd%@j-!E2E}~Q3Ek0^7ZML6Z6u0|l zHZ#w1gFS`jPnW$>4}38Fd-9e>uasbWK_l0SS*5b`tlC%^=G|_%ruy>^>pb;H zpEAqnS3j37ywS{m@&A1T!JnWsAklyin0{sX85#exumCd!n*l$FFAUt}2QM&?2T3clNEnDUU{`>etANRnk)c`j?Td?lrv{y!cYtNV z#LQ-Z{64y*oN&oy5m`>wTczfjL3M=6i0;sxcI$G64QH=U6C z7a^^oy&`9ATM4e8`Et>2H?s`?&tUywgCL-KSq* zf7`U1Go!Z543}AQX18c(@Gc!z{wGJ~Np6%dx#}}_*W2&%>s6+*RIFg${jTxpt@xU5 zuZ0szRF`kI^FE;yw)gmn_ZE+0CVi;XtY?ygew1xBAHBZk2sXLxY73M8i@v-TJt6k1Drt4hk L&8$%;mk0m=(y?My diff --git a/internal/timestamp/timestamp.go b/internal/timestamp/timestamp.go index 40f9551d..a34bc4d8 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" @@ -52,5 +56,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/internal/timestamp/timestamp_test.go b/internal/timestamp/timestamp_test.go index 59fa1615..0c551cc3 100644 --- a/internal/timestamp/timestamp_test.go +++ b/internal/timestamp/timestamp_test.go @@ -17,22 +17,25 @@ import ( "context" "crypto" "crypto/x509" + "crypto/x509/pkix" "encoding/asn1" "errors" "os" "strings" "testing" + "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" "github.com/notaryproject/tspclient-go/pki" ) -const rfc3161TSAurl = "http://rfc3161timestamp.globalsign.com/advanced" +const rfc3161TSAurl = "http://timestamp.digicert.com" func TestTimestamp(t *testing.T) { - rootCerts, err := nx509.ReadCertificateFile("testdata/tsaRootCert.crt") + rootCerts, err := nx509.ReadCertificateFile("testdata/tsaRootCert.cer") if err != nil || len(rootCerts) == 0 { t.Fatal("failed to read root CA certificate:", err) } @@ -41,69 +44,261 @@ func TestTimestamp(t *testing.T) { rootCAs.AddCert(rootCert) // --------------- Success case ---------------------------------- - timestamper, err := tspclient.NewHTTPTimestamper(nil, rfc3161TSAurl) - if err != nil { - t.Fatal(err) - } - req := &signature.SignRequest{ - Timestamper: timestamper, - TSARootCAs: rootCAs, - } - opts := tspclient.RequestOptions{ - Content: []byte("notation"), - HashAlgorithm: crypto.SHA256, - } - _, err = Timestamp(req, opts) - if err != nil { - t.Fatal(err) - } + t.Run("Timestamping success", func(t *testing.T) { + timestamper, err := tspclient.NewHTTPTimestamper(nil, rfc3161TSAurl) + if err != nil { + t.Fatal(err) + } + req := &signature.SignRequest{ + Timestamper: timestamper, + TSARootCAs: rootCAs, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + _, err = Timestamp(req, opts) + if err != nil { + t.Fatal(err) + } + }) // ------------- Failure cases ------------------------ - opts = tspclient.RequestOptions{ - Content: []byte("notation"), - HashAlgorithm: crypto.SHA1, - } - expectedErr := "malformed timestamping request: unsupported hashing algorithm: SHA-1" - _, err = Timestamp(req, opts) - assertErrorEqual(expectedErr, err, t) + t.Run("Timestamping SHA-1", func(t *testing.T) { + timestamper, err := tspclient.NewHTTPTimestamper(nil, rfc3161TSAurl) + if err != nil { + t.Fatal(err) + } + req := &signature.SignRequest{ + Timestamper: timestamper, + TSARootCAs: rootCAs, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA1, + } + expectedErr := "malformed timestamping request: unsupported hashing algorithm: SHA-1" + _, err = Timestamp(req, opts) + assertErrorEqual(expectedErr, err, t) + }) - req = &signature.SignRequest{ - Timestamper: dummyTimestamper{}, - TSARootCAs: rootCAs, - } - opts = tspclient.RequestOptions{ - Content: []byte("notation"), - HashAlgorithm: crypto.SHA256, - } - expectedErr = "failed to timestamp" - _, err = Timestamp(req, opts) - if err == nil || !strings.Contains(err.Error(), expectedErr) { - t.Fatalf("expected error message to contain %s, but got %v", expectedErr, err) - } + t.Run("Timestamping failed", func(t *testing.T) { + req := &signature.SignRequest{ + Timestamper: dummyTimestamper{}, + TSARootCAs: rootCAs, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + expectedErr := "failed to timestamp" + _, err = Timestamp(req, opts) + if err == nil || !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("expected error message to contain %s, but got %v", expectedErr, err) + } + }) - req = &signature.SignRequest{ - Timestamper: dummyTimestamper{ - respWithRejectedStatus: true, - }, - TSARootCAs: rootCAs, - } - expectedErr = "invalid timestamping response: invalid response with status code 2: rejected" - _, err = Timestamp(req, opts) - assertErrorEqual(expectedErr, err, t) + t.Run("Timestamping rejected", func(t *testing.T) { + req := &signature.SignRequest{ + Timestamper: dummyTimestamper{ + respWithRejectedStatus: true, + }, + TSARootCAs: rootCAs, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + expectedErr := "invalid timestamping response: invalid response with status code 2: rejected" + _, err = Timestamp(req, opts) + assertErrorEqual(expectedErr, err, t) + }) + + t.Run("Timestamping with cms verification failure", func(t *testing.T) { + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + req := &signature.SignRequest{ + Timestamper: dummyTimestamper{ + invalidSignature: true, + }, + TSARootCAs: rootCAs, + } + expectedErr := "failed to verify signed token: cms verification failure: x509: certificate signed by unknown authority" + _, err = Timestamp(req, opts) + assertErrorEqual(expectedErr, err, t) + }) - opts = tspclient.RequestOptions{ - Content: []byte("notation"), - HashAlgorithm: crypto.SHA256, + t.Run("Timestamping revocation failed", func(t *testing.T) { + timestamper, err := tspclient.NewHTTPTimestamper(nil, rfc3161TSAurl) + if err != nil { + t.Fatal(err) + } + req := &signature.SignRequest{ + Timestamper: timestamper, + TSARootCAs: rootCAs, + TSARevocationValidator: &dummyTSARevocationValidator{ + failOnValidate: true, + }, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + expectedErr := "failed to validate the revocation status of timestamping certificate chain with error: failed in ValidateContext" + _, err = Timestamp(req, opts) + assertErrorEqual(expectedErr, err, t) + }) + + t.Run("Timestamping certificate revoked", func(t *testing.T) { + timestamper, err := tspclient.NewHTTPTimestamper(nil, rfc3161TSAurl) + if err != nil { + t.Fatal(err) + } + req := &signature.SignRequest{ + Timestamper: timestamper, + TSARootCAs: rootCAs, + TSARevocationValidator: &dummyTSARevocationValidator{ + revoked: true, + }, + } + opts := tspclient.RequestOptions{ + Content: []byte("notation"), + HashAlgorithm: crypto.SHA256, + } + expectedErr := `timestamping certificate with subject "CN=DigiCert Timestamp 2024,O=DigiCert,C=US" is revoked` + _, err = Timestamp(req, opts) + assertErrorEqual(expectedErr, err, t) + }) + +} + +func TestRevocationResult(t *testing.T) { + certResult := []*result.CertRevocationResult{ + { + // update leaf cert result in each sub-test + }, + { + Result: result.ResultNonRevokable, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultNonRevokable, + }, + }, + }, } - req = &signature.SignRequest{ - Timestamper: dummyTimestamper{ - invalidSignature: true, + certChain := []*x509.Certificate{ + { + Subject: pkix.Name{ + CommonName: "leafCert", + }, + }, + { + Subject: pkix.Name{ + CommonName: "rootCert", + }, }, - TSARootCAs: rootCAs, } - expectedErr = "failed to verify signed token: cms verification failure: crypto/rsa: verification error" - _, err = Timestamp(req, opts) - assertErrorEqual(expectedErr, err, t) + t.Run("OCSP error without fallback", func(t *testing.T) { + certResult[0] = &result.CertRevocationResult{ + Result: result.ResultUnknown, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + }, + } + err := revocationResult(certResult, certChain) + assertErrorEqual(`timestamping certificate with subject "CN=leafCert" revocation status is unknown`, err, t) + }) + + t.Run("OCSP error with fallback", func(t *testing.T) { + certResult[0] = &result.CertRevocationResult{ + Result: result.ResultOK, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + { + Result: result.ResultOK, + RevocationMethod: result.RevocationMethodCRL, + }, + }, + RevocationMethod: result.RevocationMethodOCSPFallbackCRL, + } + if err := revocationResult(certResult, certChain); err != nil { + t.Fatal(err) + } + }) + + t.Run("OCSP error with fallback and CRL error", func(t *testing.T) { + certResult[0] = &result.CertRevocationResult{ + Result: result.ResultUnknown, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultUnknown, + Error: errors.New("ocsp error"), + RevocationMethod: result.RevocationMethodOCSP, + }, + { + Result: result.ResultUnknown, + Error: errors.New("crl error"), + RevocationMethod: result.RevocationMethodCRL, + }, + }, + RevocationMethod: result.RevocationMethodOCSPFallbackCRL, + } + err := revocationResult(certResult, certChain) + assertErrorEqual(`timestamping certificate with subject "CN=leafCert" revocation status is unknown`, err, t) + }) + + t.Run("revoked", func(t *testing.T) { + certResult[0] = &result.CertRevocationResult{ + Result: result.ResultRevoked, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultRevoked, + Error: errors.New("revoked"), + RevocationMethod: result.RevocationMethodCRL, + }, + }, + } + err := revocationResult(certResult, certChain) + assertErrorEqual(`timestamping certificate with subject "CN=leafCert" is revoked`, err, t) + }) + + t.Run("revocation method unknown error(should never reach here)", func(t *testing.T) { + certResult[0] = &result.CertRevocationResult{ + Result: result.ResultUnknown, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultUnknown, + Error: errors.New("unknown error"), + RevocationMethod: result.RevocationMethodUnknown, + }, + }, + } + err := revocationResult(certResult, certChain) + assertErrorEqual(`timestamping certificate with subject "CN=leafCert" revocation status is unknown`, err, t) + }) + + t.Run("empty cert result", func(t *testing.T) { + err := revocationResult([]*result.CertRevocationResult{}, certChain) + assertErrorEqual("certificate revocation result cannot be empty", err, t) + }) + + t.Run("cert result length does not equal to cert chain", func(t *testing.T) { + err := revocationResult([]*result.CertRevocationResult{ + certResult[1], + }, certChain) + assertErrorEqual("length of certificate revocation result 1 does not match length of the certificate chain 2", err, t) + }) + } func assertErrorEqual(expected string, err error, t *testing.T) { @@ -141,3 +336,48 @@ func (d dummyTimestamper) Timestamp(context.Context, *tspclient.Request) (*tspcl } return nil, errors.New("failed to timestamp") } + +type dummyTSARevocationValidator struct { + failOnValidate bool + revoked bool +} + +func (v *dummyTSARevocationValidator) ValidateContext(ctx context.Context, validateContextOpts revocation.ValidateContextOptions) ([]*result.CertRevocationResult, error) { + if v.failOnValidate { + return nil, errors.New("failed in ValidateContext") + } + if v.revoked { + certResults := make([]*result.CertRevocationResult, len(validateContextOpts.CertChain)) + for i := range certResults { + certResults[i] = &result.CertRevocationResult{ + Result: result.ResultOK, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultOK, + RevocationMethod: result.RevocationMethodOCSP, + }, + }, + } + } + certResults[0] = &result.CertRevocationResult{ + Result: result.ResultRevoked, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultRevoked, + Error: errors.New("revoked"), + RevocationMethod: result.RevocationMethodCRL, + }, + }, + } + certResults[len(certResults)-1] = &result.CertRevocationResult{ + Result: result.ResultNonRevokable, + ServerResults: []*result.ServerResult{ + { + Result: result.ResultNonRevokable, + }, + }, + } + return certResults, nil + } + return nil, nil +} diff --git a/signature/algorithm.go b/signature/algorithm.go index 8e391924..40dbfb78 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 7e4e238f..da41d1fd 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 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/signature/types.go b/signature/types.go index df69236c..31ccef93 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 91d34430..e511ab35 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 } From 2cd55de6f450ef11e555e409a3bdec5b07d71365 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 6 Dec 2024 14:35:34 +0800 Subject: [PATCH 2/2] bump: bump up dependencies (#249) Signed-off-by: Patrick Zheng --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 31c01262..a9a4c0bd 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.22 require ( github.com/fxamacker/cbor/v2 v2.7.0 github.com/golang-jwt/jwt/v4 v4.5.1 - github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c + github.com/notaryproject/tspclient-go v1.0.0-rc.1 github.com/veraison/go-cose v1.3.0 golang.org/x/crypto v0.29.0 ) diff --git a/go.sum b/go.sum index e650ce51..f9b84e01 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo= github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= -github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c h1:bX6gGxFw9+DShmYTgbD+vr6neF1SoXIMUU2fDgdLsfA= -github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c/go.mod h1:LGyA/6Kwd2FlM0uk8Vc5il3j0CddbWSHBj/4kxQDbjs= +github.com/notaryproject/tspclient-go v1.0.0-rc.1 h1:KcHxlqg6Adt4kzGLw012i0YMLlwGwToiR129c6IQ7Ys= +github.com/notaryproject/tspclient-go v1.0.0-rc.1/go.mod h1:LGyA/6Kwd2FlM0uk8Vc5il3j0CddbWSHBj/4kxQDbjs= github.com/veraison/go-cose v1.3.0 h1:2/H5w8kdSpQJyVtIhx8gmwPJ2uSz1PkyWFx0idbd7rk= github.com/veraison/go-cose v1.3.0/go.mod h1:df09OV91aHoQWLmy1KsDdYiagtXgyAwAl8vFeFn1gMc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=