From e218e8f9411c8d1743b80d592b6d1e76165fddc7 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 29 Jul 2024 15:47:56 +0800 Subject: [PATCH 01/25] refactored revocation Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 19 ++-------- revocation/ocsp/ocsp_test.go | 3 +- revocation/revocation.go | 35 +++++++++++++----- revocation/revocation_test.go | 70 +++++++++++++++++++++++++++-------- revocation/x509/cert.go | 13 +++++++ 5 files changed, 100 insertions(+), 40 deletions(-) create mode 100644 revocation/x509/cert.go diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index 359ce7e9..f0244c56 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -32,26 +32,15 @@ import ( "time" "github.com/notaryproject/notation-core-go/revocation/result" + revX509 "github.com/notaryproject/notation-core-go/revocation/x509" coreX509 "github.com/notaryproject/notation-core-go/x509" "golang.org/x/crypto/ocsp" ) -// Purpose is an enum for purpose of the certificate chain whose OCSP status -// is checked -type Purpose int - -const ( - // PurposeCodeSigning means the certificate chain is a code signing chain - PurposeCodeSigning Purpose = iota - - // PurposeTimestamping means the certificate chain is a timestamping chain - PurposeTimestamping -) - // Options specifies values that are needed to check OCSP revocation type Options struct { CertChain []*x509.Certificate - CertChainPurpose Purpose // default value is `PurposeCodeSigning` + CertChainPurpose revX509.Purpose // default value is `PurposeCodeSigning` SigningTime time.Time HTTPClient *http.Client } @@ -78,11 +67,11 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { // Thus, it is better to pass nil here than fail for a cert's NotBefore // being after zero time switch opts.CertChainPurpose { - case PurposeCodeSigning: + case revX509.PurposeCodeSigning: if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { return nil, result.InvalidChainError{Err: err} } - case PurposeTimestamping: + case revX509.PurposeTimestamping: if err := coreX509.ValidateTimestampingCertChain(opts.CertChain); err != nil { return nil, result.InvalidChainError{Err: err} } diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index 14d43df6..59b14e75 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/notaryproject/notation-core-go/revocation/result" + revX509 "github.com/notaryproject/notation-core-go/revocation/x509" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" ) @@ -535,7 +536,7 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("check codesigning cert with PurposeTimestamping", func(t *testing.T) { opts := Options{ CertChain: okChain, - CertChainPurpose: PurposeTimestamping, + CertChainPurpose: revX509.PurposeTimestamping, SigningTime: time.Now(), HTTPClient: http.DefaultClient, } diff --git a/revocation/revocation.go b/revocation/revocation.go index 801a0780..0652fd26 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -18,11 +18,13 @@ package revocation import ( "crypto/x509" "errors" + "fmt" "net/http" "time" "github.com/notaryproject/notation-core-go/revocation/ocsp" "github.com/notaryproject/notation-core-go/revocation/result" + revX509 "github.com/notaryproject/notation-core-go/revocation/x509" ) // Revocation is an interface that specifies methods used for revocation checking @@ -33,10 +35,19 @@ type Revocation interface { Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } +// Options specifies values that are needed to check revocation +type Options struct { + // OCSPHTTPClient is a required HTTP client for OCSP request + OCSPHTTPClient *http.Client + + // CertChainPurpose is the purpose of the certificate chain + CertChainPurpose revX509.Purpose +} + // revocation is an internal struct used for revocation checking type revocation struct { httpClient *http.Client - certChainPurpose ocsp.Purpose + certChainPurpose revX509.Purpose } // New constructs a revocation object for code signing certificate chain @@ -46,19 +57,25 @@ func New(httpClient *http.Client) (Revocation, error) { } return &revocation{ httpClient: httpClient, - certChainPurpose: ocsp.PurposeCodeSigning, + certChainPurpose: revX509.PurposeCodeSigning, }, nil } -// NewTimestamp contructs a revocation object for timestamping certificate -// chain -func NewTimestamp(httpClient *http.Client) (Revocation, error) { - if httpClient == nil { - return nil, errors.New("invalid input: a non-nil httpClient must be specified") +// NewWithOptions constructs a revocation object with the specified options +func NewWithOptions(opts *Options) (Revocation, error) { + if opts.OCSPHTTPClient == nil { + return nil, errors.New("invalid input: a non-nil OCSPHTTPClient must be specified") } + + switch opts.CertChainPurpose { + case revX509.PurposeCodeSigning, revX509.PurposeTimestamping: + default: + return nil, fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose) + } + return &revocation{ - httpClient: httpClient, - certChainPurpose: ocsp.PurposeTimestamping, + httpClient: opts.OCSPHTTPClient, + certChainPurpose: opts.CertChainPurpose, }, nil } diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index f9d4f4e5..eeb9f836 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -23,6 +23,7 @@ import ( revocationocsp "github.com/notaryproject/notation-core-go/revocation/ocsp" "github.com/notaryproject/notation-core-go/revocation/result" + revX509 "github.com/notaryproject/notation-core-go/revocation/x509" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" ) @@ -99,12 +100,24 @@ func TestNew(t *testing.T) { } } -func TestNewTimestamp(t *testing.T) { - expectedErrMsg := "invalid input: a non-nil httpClient must be specified" - _, err := NewTimestamp(nil) - if err == nil || err.Error() != expectedErrMsg { - t.Fatalf("expected %s, but got %s", expectedErrMsg, err) - } +func TestNewWithOptions(t *testing.T) { + t.Run("nil OCSP HTTP Client", func(t *testing.T) { + _, err := NewWithOptions(&Options{}) + if err == nil { + t.Error("Expected NewWithOptions to fail with an error, but it succeeded") + } + }) + + t.Run("invalid CertChainPurpose", func(t *testing.T) { + _, err := NewWithOptions(&Options{ + OCSPHTTPClient: &http.Client{}, + CertChainPurpose: -1, + }) + if err == nil { + t.Error("Expected NewWithOptions to fail with an error, but it succeeded") + } + }) + } func TestCheckRevocationStatusForSingleCert(t *testing.T) { @@ -477,7 +490,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } t.Run("empty chain", func(t *testing.T) { - r, err := NewTimestamp(&http.Client{Timeout: 5 * time.Second}) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -492,7 +508,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { }) t.Run("check non-revoked chain", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -513,7 +532,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check chain with 1 Unknown cert", func(t *testing.T) { // 3rd cert will be unknown, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -539,7 +561,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 revoked cert", func(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -565,7 +590,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 unknown and 1 revoked cert", func(t *testing.T) { // 3rd cert will be unknown, 5th will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -597,7 +625,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { revokedTime := time.Now().Add(time.Hour) // 3rd cert will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -619,7 +650,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { revokedTime := time.Now().Add(time.Hour) // 3rd cert will be unknown, 5th will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -645,7 +679,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 revoked cert before signing time", func(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } @@ -675,7 +712,10 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { if !zeroTime.IsZero() { t.Errorf("exected zeroTime.IsZero() to be true") } - r, err := NewTimestamp(client) + r, err := NewWithOptions(&Options{ + OCSPHTTPClient: client, + CertChainPurpose: revX509.PurposeTimestamping, + }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } diff --git a/revocation/x509/cert.go b/revocation/x509/cert.go new file mode 100644 index 00000000..f7d7c5b8 --- /dev/null +++ b/revocation/x509/cert.go @@ -0,0 +1,13 @@ +package x509 + +// Purpose is an enum for purpose of the certificate chain whose revocation +// status is checked +type Purpose int + +const ( + // PurposeCodeSigning means the certificate chain is a code signing chain + PurposeCodeSigning Purpose = iota + + // PurposeTimestamping means the certificate chain is a timestamping chain + PurposeTimestamping +) From bba1705179d67d947ca838e0b7a819879a4d0822 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 29 Jul 2024 15:52:13 +0800 Subject: [PATCH 02/25] fixed license Signed-off-by: Patrick Zheng --- revocation/x509/cert.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/revocation/x509/cert.go b/revocation/x509/cert.go index f7d7c5b8..3be7479c 100644 --- a/revocation/x509/cert.go +++ b/revocation/x509/cert.go @@ -1,3 +1,17 @@ +// 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 x509 provides common usage related to x509 certificates package x509 // Purpose is an enum for purpose of the certificate chain whose revocation From b886f0a6d88fdc55a958ce608d37765d3a48699b Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 29 Jul 2024 17:03:23 +0800 Subject: [PATCH 03/25] updated per code review Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 7 +++---- revocation/ocsp/ocsp_test.go | 3 +-- revocation/revocation.go | 30 +++++++++++++++++++++------ revocation/revocation_test.go | 38 +++++++++++++++++------------------ revocation/x509/cert.go | 27 ------------------------- 5 files changed, 47 insertions(+), 58 deletions(-) delete mode 100644 revocation/x509/cert.go diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index f0244c56..86974073 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -32,7 +32,6 @@ import ( "time" "github.com/notaryproject/notation-core-go/revocation/result" - revX509 "github.com/notaryproject/notation-core-go/revocation/x509" coreX509 "github.com/notaryproject/notation-core-go/x509" "golang.org/x/crypto/ocsp" ) @@ -40,7 +39,7 @@ import ( // Options specifies values that are needed to check OCSP revocation type Options struct { CertChain []*x509.Certificate - CertChainPurpose revX509.Purpose // default value is `PurposeCodeSigning` + CertChainPurpose x509.ExtKeyUsage // default value is `PurposeCodeSigning` SigningTime time.Time HTTPClient *http.Client } @@ -67,11 +66,11 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { // Thus, it is better to pass nil here than fail for a cert's NotBefore // being after zero time switch opts.CertChainPurpose { - case revX509.PurposeCodeSigning: + case x509.ExtKeyUsageCodeSigning: if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { return nil, result.InvalidChainError{Err: err} } - case revX509.PurposeTimestamping: + case x509.ExtKeyUsageTimeStamping: if err := coreX509.ValidateTimestampingCertChain(opts.CertChain); err != nil { return nil, result.InvalidChainError{Err: err} } diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index 59b14e75..9317ecf8 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -22,7 +22,6 @@ import ( "time" "github.com/notaryproject/notation-core-go/revocation/result" - revX509 "github.com/notaryproject/notation-core-go/revocation/x509" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" ) @@ -536,7 +535,7 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("check codesigning cert with PurposeTimestamping", func(t *testing.T) { opts := Options{ CertChain: okChain, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, SigningTime: time.Now(), HTTPClient: http.DefaultClient, } diff --git a/revocation/revocation.go b/revocation/revocation.go index 0652fd26..cc3f27ad 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -16,6 +16,7 @@ package revocation import ( + "context" "crypto/x509" "errors" "fmt" @@ -24,7 +25,6 @@ import ( "github.com/notaryproject/notation-core-go/revocation/ocsp" "github.com/notaryproject/notation-core-go/revocation/result" - revX509 "github.com/notaryproject/notation-core-go/revocation/x509" ) // Revocation is an interface that specifies methods used for revocation checking @@ -35,19 +35,27 @@ type Revocation interface { Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } +// ContextRevocation is an interface that specifies methods used for revocation checking +type ContextRevocation interface { + // ValidateContext checks the revocation status for a certificate chain using OCSP + // and returns an array of CertRevocationResults that contain the results + // and any errors that are encountered during the process + ValidateContext(ctx context.Context, certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) +} + // Options specifies values that are needed to check revocation type Options struct { // OCSPHTTPClient is a required HTTP client for OCSP request OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain - CertChainPurpose revX509.Purpose + CertChainPurpose x509.ExtKeyUsage } // revocation is an internal struct used for revocation checking type revocation struct { httpClient *http.Client - certChainPurpose revX509.Purpose + certChainPurpose x509.ExtKeyUsage } // New constructs a revocation object for code signing certificate chain @@ -57,18 +65,18 @@ func New(httpClient *http.Client) (Revocation, error) { } return &revocation{ httpClient: httpClient, - certChainPurpose: revX509.PurposeCodeSigning, + certChainPurpose: x509.ExtKeyUsageTimeStamping, }, nil } // NewWithOptions constructs a revocation object with the specified options -func NewWithOptions(opts *Options) (Revocation, error) { +func NewWithOptions(opts *Options) (ContextRevocation, error) { if opts.OCSPHTTPClient == nil { return nil, errors.New("invalid input: a non-nil OCSPHTTPClient must be specified") } switch opts.CertChainPurpose { - case revX509.PurposeCodeSigning, revX509.PurposeTimestamping: + case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: default: return nil, fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose) } @@ -86,6 +94,16 @@ func NewWithOptions(opts *Options) (Revocation, error) { // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { + return r.ValidateContext(context.Background(), certChain, signingTime) +} + +// ValidateContext checks the revocation status for a certificate chain using OCSP and +// returns an array of CertRevocationResults that contain the results and any +// errors that are encountered during the process +// +// TODO: add CRL support +// https://github.com/notaryproject/notation-core-go/issues/125 +func (r *revocation) ValidateContext(ctx context.Context, certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { return ocsp.CheckStatus(ocsp.Options{ CertChain: certChain, CertChainPurpose: r.certChainPurpose, diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index eeb9f836..dd4ff40a 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -14,6 +14,7 @@ package revocation import ( + "context" "crypto/x509" "errors" "fmt" @@ -23,7 +24,6 @@ import ( revocationocsp "github.com/notaryproject/notation-core-go/revocation/ocsp" "github.com/notaryproject/notation-core-go/revocation/result" - revX509 "github.com/notaryproject/notation-core-go/revocation/x509" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" ) @@ -492,12 +492,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { r, err := NewWithOptions(&Options{ OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate([]*x509.Certificate{}, time.Now()) + certResults, err := r.ValidateContext(context.Background(), []*x509.Certificate{}, time.Now()) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { t.Errorf("Expected CheckStatus to fail with %v, but got: %v", expectedErr, err) @@ -510,12 +510,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -534,12 +534,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -563,12 +563,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -592,12 +592,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -627,12 +627,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -652,12 +652,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -681,12 +681,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now().Add(time.Hour)) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now().Add(time.Hour)) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -714,12 +714,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } r, err := NewWithOptions(&Options{ OCSPHTTPClient: client, - CertChainPurpose: revX509.PurposeTimestamping, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.Validate(revokableChain, time.Now().Add(time.Hour)) + certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now().Add(time.Hour)) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } diff --git a/revocation/x509/cert.go b/revocation/x509/cert.go deleted file mode 100644 index 3be7479c..00000000 --- a/revocation/x509/cert.go +++ /dev/null @@ -1,27 +0,0 @@ -// 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 x509 provides common usage related to x509 certificates -package x509 - -// Purpose is an enum for purpose of the certificate chain whose revocation -// status is checked -type Purpose int - -const ( - // PurposeCodeSigning means the certificate chain is a code signing chain - PurposeCodeSigning Purpose = iota - - // PurposeTimestamping means the certificate chain is a timestamping chain - PurposeTimestamping -) From dc24a75c41f61c6541ec59d6b6b22fc8cc9295b2 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 29 Jul 2024 18:43:14 +0800 Subject: [PATCH 04/25] fix Signed-off-by: Patrick Zheng --- revocation/revocation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index cc3f27ad..319e8983 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -65,7 +65,7 @@ func New(httpClient *http.Client) (Revocation, error) { } return &revocation{ httpClient: httpClient, - certChainPurpose: x509.ExtKeyUsageTimeStamping, + certChainPurpose: x509.ExtKeyUsageCodeSigning, }, nil } From dadec5fa788aa01d693ce5bb027e3de61f37d9e1 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 29 Jul 2024 19:00:10 +0800 Subject: [PATCH 05/25] fix Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 2 +- revocation/ocsp/ocsp_test.go | 179 ++++++++++++++++++++--------------- revocation/revocation.go | 16 ++-- 3 files changed, 112 insertions(+), 85 deletions(-) diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index 86974073..05902714 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -39,7 +39,7 @@ import ( // Options specifies values that are needed to check OCSP revocation type Options struct { CertChain []*x509.Certificate - CertChainPurpose x509.ExtKeyUsage // default value is `PurposeCodeSigning` + CertChainPurpose x509.ExtKeyUsage SigningTime time.Time HTTPClient *http.Client } diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index 9317ecf8..bf8ee412 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -88,9 +88,10 @@ func TestCheckStatus(t *testing.T) { t.Run("check non-revoked cert", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -100,9 +101,10 @@ func TestCheckStatus(t *testing.T) { t.Run("check cert with Unknown OCSP response", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Unknown}, nil, true) opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -117,9 +119,10 @@ func TestCheckStatus(t *testing.T) { t.Run("check OCSP revoked cert", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Revoked}, nil, true) opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -135,9 +138,10 @@ func TestCheckStatus(t *testing.T) { revokedTime := time.Now().Add(time.Hour) client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Revoked}, &revokedTime, true) opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -150,9 +154,10 @@ func TestCheckStatusForSelfSignedCert(t *testing.T) { selfSignedTuple := testhelper.GetRSASelfSignedSigningCertTuple("Notation revocation test self-signed cert") client := testhelper.MockClient([]testhelper.RSACertTuple{selfSignedTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{selfSignedTuple.Cert}, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{selfSignedTuple.Cert}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -167,9 +172,10 @@ func TestCheckStatusForRootCert(t *testing.T) { rootTuple := testhelper.GetRSARootCertificate() client := testhelper.MockClient([]testhelper.RSACertTuple{rootTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{rootTuple.Cert}, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{rootTuple.Cert}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -186,9 +192,10 @@ func TestCheckStatusForNonSelfSignedSingleCert(t *testing.T) { certTuple := testhelper.GetRSALeafCertificate() client := testhelper.MockClient([]testhelper.RSACertTuple{certTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{certTuple.Cert}, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{certTuple.Cert}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -212,9 +219,10 @@ func TestCheckStatusForChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { opts := Options{ - CertChain: []*x509.Certificate{}, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: []*x509.Certificate{}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} @@ -228,9 +236,10 @@ func TestCheckStatusForChain(t *testing.T) { t.Run("check non-revoked chain", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -251,9 +260,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) // 3rd cert will be unknown, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -279,9 +289,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -307,9 +318,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be unknown, 5th will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -341,9 +353,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -365,9 +378,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be unknown, 5th will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -393,9 +407,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: time.Now().Add(time.Hour), - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now().Add(time.Hour), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -422,9 +437,10 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - SigningTime: zeroTime, - HTTPClient: client, + CertChain: revokableChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: zeroTime, + HTTPClient: client, } if !zeroTime.IsZero() { @@ -482,9 +498,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("no OCSPServer specified", func(t *testing.T) { opts := Options{ - CertChain: noOCSPChain, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: noOCSPChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err != nil { @@ -504,9 +521,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("chain missing root", func(t *testing.T) { opts := Options{ - CertChain: noRootChain, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: noRootChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != chainRootErr.Error() { @@ -519,9 +537,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("backwards chain", func(t *testing.T) { opts := Options{ - CertChain: backwardsChain, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: backwardsChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != backwardsChainErr.Error() { @@ -551,12 +570,12 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("check with unknwon CertChainPurpose", func(t *testing.T) { opts := Options{ CertChain: okChain, - CertChainPurpose: 2, + CertChainPurpose: -1, SigningTime: time.Now(), HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) - if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unknown certificate chain purpose 2" { + if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unknown certificate chain purpose -1" { t.Errorf("Expected CheckStatus to fail with %v, but got: %v", timestampSigningCertErr, err) } if certResults != nil { @@ -567,9 +586,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("timeout", func(t *testing.T) { timeoutClient := &http.Client{Timeout: 1 * time.Nanosecond} opts := Options{ - CertChain: okChain, - SigningTime: time.Now(), - HTTPClient: timeoutClient, + CertChain: okChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: timeoutClient, } certResults, err := CheckStatus(opts) if err != nil { @@ -596,9 +616,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("expired ocsp response", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: expiredChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: expiredChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err != nil { @@ -620,9 +641,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("pkixNoCheck missing", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, false) opts := Options{ - CertChain: okChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: okChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -640,9 +662,10 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("non-HTTP URI error", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: noHTTPChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: noHTTPChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err != nil { @@ -687,9 +710,10 @@ func TestCheckOCSPInvalidChain(t *testing.T) { t.Run("chain missing intermediate", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: missingIntermediateChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: missingIntermediateChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != missingIntermediateErr.Error() { @@ -703,9 +727,10 @@ func TestCheckOCSPInvalidChain(t *testing.T) { t.Run("chain out of order", func(t *testing.T) { client := testhelper.MockClient(misorderedIntermediateTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: misorderedIntermediateChain, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: misorderedIntermediateChain, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != misorderedChainErr.Error() { diff --git a/revocation/revocation.go b/revocation/revocation.go index 319e8983..27afbe7f 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -35,9 +35,10 @@ type Revocation interface { Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } -// ContextRevocation is an interface that specifies methods used for revocation checking +// ContextRevocation is an interface that specifies methods used for revocation +// checking with context type ContextRevocation interface { - // ValidateContext checks the revocation status for a certificate chain using OCSP + // ValidateContext checks the revocation status for a certificate chain // and returns an array of CertRevocationResults that contain the results // and any errors that are encountered during the process ValidateContext(ctx context.Context, certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) @@ -48,7 +49,8 @@ type Options struct { // OCSPHTTPClient is a required HTTP client for OCSP request OCSPHTTPClient *http.Client - // CertChainPurpose is the purpose of the certificate chain + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. CertChainPurpose x509.ExtKeyUsage } @@ -69,7 +71,7 @@ func New(httpClient *http.Client) (Revocation, error) { }, nil } -// NewWithOptions constructs a revocation object with the specified options +// NewWithOptions constructs a ContextRevocation with the specified options func NewWithOptions(opts *Options) (ContextRevocation, error) { if opts.OCSPHTTPClient == nil { return nil, errors.New("invalid input: a non-nil OCSPHTTPClient must be specified") @@ -97,9 +99,9 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti return r.ValidateContext(context.Background(), certChain, signingTime) } -// ValidateContext checks the revocation status for a certificate chain using OCSP and -// returns an array of CertRevocationResults that contain the results and any -// errors that are encountered during the process +// ValidateContext checks the revocation status for a certificate chain using +// OCSP and returns an array of CertRevocationResults that contain the results +// and any errors that are encountered during the process // // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 From 73d64c8c104863fe17e2ef923b0107bc07d56ca5 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 10:52:27 +0800 Subject: [PATCH 06/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 52 +++++++++++++++++---------- revocation/revocation_test.go | 67 ++++++++++++++++++++++++----------- 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 27afbe7f..d7def29c 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -35,23 +35,24 @@ type Revocation interface { Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } -// ContextRevocation is an interface that specifies methods used for revocation -// checking with context +// ValidateContextOptions provides configuration options for revocation checks +type ValidateContextOptions struct { + // CertChain denotes the certificate chain whose revocation status is + // been validated. + CertChain []*x509.Certificate + + // AuthenticSigningTime denotes the authentic signing time of the signature. + // It is solely used under signing scheme `notary.x509.signingAuthority`. + AuthenticSigningTime time.Time +} + +// ContextRevocation is an interface that provides revocation checking with +// context type ContextRevocation interface { - // ValidateContext checks the revocation status for a certificate chain + // ValidateContext checks the revocation status given caller provided options // and returns an array of CertRevocationResults that contain the results // and any errors that are encountered during the process - ValidateContext(ctx context.Context, certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) -} - -// Options specifies values that are needed to check revocation -type Options struct { - // OCSPHTTPClient is a required HTTP client for OCSP request - OCSPHTTPClient *http.Client - - // CertChainPurpose is the purpose of the certificate chain. Supported - // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - CertChainPurpose x509.ExtKeyUsage + ValidateContext(ctx context.Context, validateContextOpts ValidateContextOptions) ([]*result.CertRevocationResult, error) } // revocation is an internal struct used for revocation checking @@ -71,8 +72,18 @@ func New(httpClient *http.Client) (Revocation, error) { }, nil } +// Options specifies values that are needed to check revocation +type Options struct { + // OCSPHTTPClient is a required HTTP client for OCSP request + OCSPHTTPClient *http.Client + + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + CertChainPurpose x509.ExtKeyUsage +} + // NewWithOptions constructs a ContextRevocation with the specified options -func NewWithOptions(opts *Options) (ContextRevocation, error) { +func NewWithOptions(opts Options) (ContextRevocation, error) { if opts.OCSPHTTPClient == nil { return nil, errors.New("invalid input: a non-nil OCSPHTTPClient must be specified") } @@ -96,7 +107,10 @@ func NewWithOptions(opts *Options) (ContextRevocation, error) { // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { - return r.ValidateContext(context.Background(), certChain, signingTime) + return r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: certChain, + AuthenticSigningTime: signingTime, + }) } // ValidateContext checks the revocation status for a certificate chain using @@ -105,11 +119,11 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti // // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 -func (r *revocation) ValidateContext(ctx context.Context, certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { +func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts ValidateContextOptions) ([]*result.CertRevocationResult, error) { return ocsp.CheckStatus(ocsp.Options{ - CertChain: certChain, + CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, - SigningTime: signingTime, + SigningTime: validateContextOpts.AuthenticSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index dd4ff40a..741a073c 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -102,14 +102,14 @@ func TestNew(t *testing.T) { func TestNewWithOptions(t *testing.T) { t.Run("nil OCSP HTTP Client", func(t *testing.T) { - _, err := NewWithOptions(&Options{}) + _, err := NewWithOptions(Options{}) if err == nil { t.Error("Expected NewWithOptions to fail with an error, but it succeeded") } }) t.Run("invalid CertChainPurpose", func(t *testing.T) { - _, err := NewWithOptions(&Options{ + _, err := NewWithOptions(Options{ OCSPHTTPClient: &http.Client{}, CertChainPurpose: -1, }) @@ -490,14 +490,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } t.Run("empty chain", func(t *testing.T) { - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), []*x509.Certificate{}, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: []*x509.Certificate{}, + AuthenticSigningTime: time.Now(), + }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { t.Errorf("Expected CheckStatus to fail with %v, but got: %v", expectedErr, err) @@ -508,14 +511,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { }) t.Run("check non-revoked chain", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -532,14 +538,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check chain with 1 Unknown cert", func(t *testing.T) { // 3rd cert will be unknown, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -561,14 +570,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 revoked cert", func(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -590,14 +602,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 unknown and 1 revoked cert", func(t *testing.T) { // 3rd cert will be unknown, 5th will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -625,14 +640,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { revokedTime := time.Now().Add(time.Hour) // 3rd cert will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -650,14 +668,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { revokedTime := time.Now().Add(time.Hour) // 3rd cert will be unknown, 5th will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now()) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -679,14 +700,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check OCSP with 1 revoked cert before signing time", func(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now().Add(time.Hour)) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -712,14 +736,17 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { if !zeroTime.IsZero() { t.Errorf("exected zeroTime.IsZero() to be true") } - r, err := NewWithOptions(&Options{ + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } - certResults, err := r.ValidateContext(context.Background(), revokableChain, time.Now().Add(time.Hour)) + certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } From 56447bdb7bc5af7421c418f4aa5161baed7a9b6c Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 11:14:41 +0800 Subject: [PATCH 07/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index d7def29c..c64ef99f 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -38,11 +38,12 @@ type Revocation interface { // ValidateContextOptions provides configuration options for revocation checks type ValidateContextOptions struct { // CertChain denotes the certificate chain whose revocation status is - // been validated. + // been validated. REQUIRED. CertChain []*x509.Certificate // AuthenticSigningTime denotes the authentic signing time of the signature. // It is solely used under signing scheme `notary.x509.signingAuthority`. + // OPTIONAL. AuthenticSigningTime time.Time } @@ -74,11 +75,12 @@ func New(httpClient *http.Client) (Revocation, error) { // Options specifies values that are needed to check revocation type Options struct { - // OCSPHTTPClient is a required HTTP client for OCSP request + // OCSPHTTPClient is the HTTP client for OCSP request. REQUIRED. OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain. Supported // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + // REQUIRED. CertChainPurpose x509.ExtKeyUsage } From 254c50c7135856cc63c463e8be23805be5b4b673 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 11:31:42 +0800 Subject: [PATCH 08/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 4 ++++ revocation/revocation_test.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/revocation/revocation.go b/revocation/revocation.go index c64ef99f..998eef92 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -122,6 +122,10 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts ValidateContextOptions) ([]*result.CertRevocationResult, error) { + if len(validateContextOpts.CertChain) == 0 { + return nil, result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} + } + return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 741a073c..407b5d92 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -984,3 +984,18 @@ func TestCheckRevocationInvalidChain(t *testing.T) { } }) } + +func TestValidateContext(t *testing.T) { + r, err := NewWithOptions(Options{ + OCSPHTTPClient: &http.Client{}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + }) + if err != nil { + t.Fatal(err) + } + expectedErrMsg := "invalid chain: expected chain to be correct and complete: chain does not contain any certificates" + _, err = r.ValidateContext(context.Background(), ValidateContextOptions{}) + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err) + } +} From e19d36b91b8df42638200a1543d287de9e611e94 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 12:22:26 +0800 Subject: [PATCH 09/25] fix Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 19 +++++++++++-------- revocation/ocsp/ocsp_test.go | 18 +++++++----------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index 05902714..b51e5526 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -38,10 +38,15 @@ import ( // Options specifies values that are needed to check OCSP revocation type Options struct { - CertChain []*x509.Certificate + CertChain []*x509.Certificate + + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + // When not provided, x509.ExtKeyUsageCodeSigning is used as default. CertChainPurpose x509.ExtKeyUsage - SigningTime time.Time - HTTPClient *http.Client + + SigningTime time.Time + HTTPClient *http.Client } const ( @@ -66,16 +71,14 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { // Thus, it is better to pass nil here than fail for a cert's NotBefore // being after zero time switch opts.CertChainPurpose { - case x509.ExtKeyUsageCodeSigning: - if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { - return nil, result.InvalidChainError{Err: err} - } case x509.ExtKeyUsageTimeStamping: if err := coreX509.ValidateTimestampingCertChain(opts.CertChain); err != nil { return nil, result.InvalidChainError{Err: err} } default: - return nil, result.InvalidChainError{Err: fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose)} + if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { + return nil, result.InvalidChainError{Err: err} + } } certResults := make([]*result.CertRevocationResult, len(opts.CertChain)) diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index bf8ee412..e852d099 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -567,19 +567,15 @@ func TestCheckStatusErrors(t *testing.T) { } }) - t.Run("check with unknwon CertChainPurpose", func(t *testing.T) { + t.Run("check with default CertChainPurpose", func(t *testing.T) { opts := Options{ - CertChain: okChain, - CertChainPurpose: -1, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: okChain, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } - certResults, err := CheckStatus(opts) - if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unknown certificate chain purpose -1" { - t.Errorf("Expected CheckStatus to fail with %v, but got: %v", timestampSigningCertErr, err) - } - if certResults != nil { - t.Error("Expected certResults to be nil when there is an error") + _, err := CheckStatus(opts) + if err != nil { + t.Fatal(err) } }) From 442f5106a34f6d80198c24417b1fb7da1c42b572 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 14:55:34 +0800 Subject: [PATCH 10/25] update Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 10 ++++++---- revocation/ocsp/ocsp_test.go | 16 ++++++++++++++++ revocation/revocation.go | 6 ++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index b51e5526..bf45abe8 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -42,7 +42,7 @@ type Options struct { // CertChainPurpose is the purpose of the certificate chain. Supported // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - // When not provided, x509.ExtKeyUsageCodeSigning is used as default. + // When not provided, the chain is taken as a code signing certificate chain. CertChainPurpose x509.ExtKeyUsage SigningTime time.Time @@ -71,14 +71,16 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { // Thus, it is better to pass nil here than fail for a cert's NotBefore // being after zero time switch opts.CertChainPurpose { + case x509.ExtKeyUsageAny, x509.ExtKeyUsageCodeSigning: + if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { + return nil, result.InvalidChainError{Err: err} + } case x509.ExtKeyUsageTimeStamping: if err := coreX509.ValidateTimestampingCertChain(opts.CertChain); err != nil { return nil, result.InvalidChainError{Err: err} } default: - if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { - return nil, result.InvalidChainError{Err: err} - } + return nil, result.InvalidChainError{Err: fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose)} } certResults := make([]*result.CertRevocationResult, len(opts.CertChain)) diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index e852d099..599c6067 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -579,6 +579,22 @@ func TestCheckStatusErrors(t *testing.T) { } }) + t.Run("check with unknwon CertChainPurpose", func(t *testing.T) { + opts := Options{ + CertChain: okChain, + CertChainPurpose: -1, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, + } + certResults, err := CheckStatus(opts) + if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unknown certificate chain purpose -1" { + t.Errorf("Expected CheckStatus to fail with %v, but got: %v", timestampSigningCertErr, err) + } + if certResults != nil { + t.Error("Expected certResults to be nil when there is an error") + } + }) + t.Run("timeout", func(t *testing.T) { timeoutClient := &http.Client{Timeout: 1 * time.Nanosecond} opts := Options{ diff --git a/revocation/revocation.go b/revocation/revocation.go index 998eef92..570ec391 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -75,7 +75,9 @@ func New(httpClient *http.Client) (Revocation, error) { // Options specifies values that are needed to check revocation type Options struct { - // OCSPHTTPClient is the HTTP client for OCSP request. REQUIRED. + // OCSPHTTPClient is the HTTP client for OCSP request. If not provided, + // the default http.Client will be used. + // OPTIONAL. OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain. Supported @@ -87,7 +89,7 @@ type Options struct { // NewWithOptions constructs a ContextRevocation with the specified options func NewWithOptions(opts Options) (ContextRevocation, error) { if opts.OCSPHTTPClient == nil { - return nil, errors.New("invalid input: a non-nil OCSPHTTPClient must be specified") + opts.OCSPHTTPClient = &http.Client{} } switch opts.CertChainPurpose { From 0e7da15390747611e8ac73494eaa448d36c08a6e Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 15:02:44 +0800 Subject: [PATCH 11/25] deprecate Signed-off-by: Patrick Zheng --- revocation/revocation.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 570ec391..40590c88 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -27,7 +27,10 @@ import ( "github.com/notaryproject/notation-core-go/revocation/result" ) -// Revocation is an interface that specifies methods used for revocation checking +// Revocation is an interface that specifies methods used for revocation checking. +// +// Deprecated: Revocation exists for backwards compatibility and should not be used. +// To perform revocation check, use ContextRevocation. type Revocation interface { // Validate checks the revocation status for a certificate chain using OCSP // and returns an array of CertRevocationResults that contain the results @@ -62,7 +65,10 @@ type revocation struct { certChainPurpose x509.ExtKeyUsage } -// New constructs a revocation object for code signing certificate chain +// New constructs a revocation object for code signing certificate chain. +// +// Deprecated: New exists for backwards compatibility and should not be used. +// To create a revocation object, use NewWithOptions. func New(httpClient *http.Client) (Revocation, error) { if httpClient == nil { return nil, errors.New("invalid input: a non-nil httpClient must be specified") From 5d5043a8f8124382ac9c92f27fee04f9bc59c564 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 30 Jul 2024 15:36:16 +0800 Subject: [PATCH 12/25] updated per code review Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 5 +++-- revocation/ocsp/ocsp_test.go | 2 +- revocation/revocation.go | 10 +++++----- revocation/revocation_test.go | 13 ++++++++----- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index bf45abe8..2ebc794b 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -42,7 +42,8 @@ type Options struct { // CertChainPurpose is the purpose of the certificate chain. Supported // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - // When not provided, the chain is taken as a code signing certificate chain. + // When not provided, the default value x509.ExtKeyUsageAny is also taken as + // a code signing certificate chain. CertChainPurpose x509.ExtKeyUsage SigningTime time.Time @@ -80,7 +81,7 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { return nil, result.InvalidChainError{Err: err} } default: - return nil, result.InvalidChainError{Err: fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose)} + return nil, result.InvalidChainError{Err: fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose)} } certResults := make([]*result.CertRevocationResult, len(opts.CertChain)) diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index 599c6067..a4fc07ac 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -587,7 +587,7 @@ func TestCheckStatusErrors(t *testing.T) { HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) - if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unknown certificate chain purpose -1" { + if err == nil || err.Error() != "invalid chain: expected chain to be correct and complete: unsupported certificate chain purpose -1" { t.Errorf("Expected CheckStatus to fail with %v, but got: %v", timestampSigningCertErr, err) } if certResults != nil { diff --git a/revocation/revocation.go b/revocation/revocation.go index 40590c88..f23aabe4 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -30,7 +30,7 @@ import ( // Revocation is an interface that specifies methods used for revocation checking. // // Deprecated: Revocation exists for backwards compatibility and should not be used. -// To perform revocation check, use ContextRevocation. +// To perform revocation check, use [ContextRevocation]. type Revocation interface { // Validate checks the revocation status for a certificate chain using OCSP // and returns an array of CertRevocationResults that contain the results @@ -68,7 +68,7 @@ type revocation struct { // New constructs a revocation object for code signing certificate chain. // // Deprecated: New exists for backwards compatibility and should not be used. -// To create a revocation object, use NewWithOptions. +// To create a revocation object, use [NewWithOptions]. func New(httpClient *http.Client) (Revocation, error) { if httpClient == nil { return nil, errors.New("invalid input: a non-nil httpClient must be specified") @@ -82,7 +82,7 @@ func New(httpClient *http.Client) (Revocation, error) { // Options specifies values that are needed to check revocation type Options struct { // OCSPHTTPClient is the HTTP client for OCSP request. If not provided, - // the default http.Client will be used. + // http.DefaultClient will be used. // OPTIONAL. OCSPHTTPClient *http.Client @@ -95,13 +95,13 @@ type Options struct { // NewWithOptions constructs a ContextRevocation with the specified options func NewWithOptions(opts Options) (ContextRevocation, error) { if opts.OCSPHTTPClient == nil { - opts.OCSPHTTPClient = &http.Client{} + opts.OCSPHTTPClient = http.DefaultClient } switch opts.CertChainPurpose { case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: default: - return nil, fmt.Errorf("unknown certificate chain purpose %v", opts.CertChainPurpose) + return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) } return &revocation{ diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 407b5d92..8d6b310f 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -102,9 +102,11 @@ func TestNew(t *testing.T) { func TestNewWithOptions(t *testing.T) { t.Run("nil OCSP HTTP Client", func(t *testing.T) { - _, err := NewWithOptions(Options{}) - if err == nil { - t.Error("Expected NewWithOptions to fail with an error, but it succeeded") + _, err := NewWithOptions(Options{ + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + }) + if err != nil { + t.Fatal(err) } }) @@ -113,8 +115,9 @@ func TestNewWithOptions(t *testing.T) { OCSPHTTPClient: &http.Client{}, CertChainPurpose: -1, }) - if err == nil { - t.Error("Expected NewWithOptions to fail with an error, but it succeeded") + expectedErrMsg := "unsupported certificate chain purpose -1" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err.Error()) } }) From 8f09e5daddc05f056e4be8adab1a8a9a7e9c6cdf Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 31 Jul 2024 13:41:22 +0800 Subject: [PATCH 13/25] updated per code review Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 8 ++++---- revocation/revocation.go | 20 +++++++++---------- revocation/revocation_test.go | 36 +++++++++++++++++------------------ 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index 2ebc794b..c88aa3a2 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -67,12 +67,12 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { return nil, result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} } - // Validate cert chain structure - // Since this is using authentic signing time, signing time may be zero. - // Thus, it is better to pass nil here than fail for a cert's NotBefore - // being after zero time switch opts.CertChainPurpose { case x509.ExtKeyUsageAny, x509.ExtKeyUsageCodeSigning: + // Since ValidateCodeSigningCertChain is using authentic signing time, + // signing time may be zero. + // Thus, it is better to pass nil here than fail for a cert's NotBefore + // being after zero time if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { return nil, result.InvalidChainError{Err: err} } diff --git a/revocation/revocation.go b/revocation/revocation.go index f23aabe4..8877cfc9 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -30,7 +30,7 @@ import ( // Revocation is an interface that specifies methods used for revocation checking. // // Deprecated: Revocation exists for backwards compatibility and should not be used. -// To perform revocation check, use [ContextRevocation]. +// To perform revocation check, use [Validator]. type Revocation interface { // Validate checks the revocation status for a certificate chain using OCSP // and returns an array of CertRevocationResults that contain the results @@ -44,15 +44,15 @@ type ValidateContextOptions struct { // been validated. REQUIRED. CertChain []*x509.Certificate - // AuthenticSigningTime denotes the authentic signing time of the signature. + // AuthenticdSigningTime denotes the authentic signing time of the signature. // It is solely used under signing scheme `notary.x509.signingAuthority`. // OPTIONAL. - AuthenticSigningTime time.Time + AuthenticdSigningTime time.Time } -// ContextRevocation is an interface that provides revocation checking with +// Validator is an interface that provides revocation checking with // context -type ContextRevocation interface { +type Validator interface { // ValidateContext checks the revocation status given caller provided options // and returns an array of CertRevocationResults that contain the results // and any errors that are encountered during the process @@ -93,9 +93,9 @@ type Options struct { } // NewWithOptions constructs a ContextRevocation with the specified options -func NewWithOptions(opts Options) (ContextRevocation, error) { +func NewWithOptions(opts Options) (Validator, error) { if opts.OCSPHTTPClient == nil { - opts.OCSPHTTPClient = http.DefaultClient + opts.OCSPHTTPClient = &http.Client{Timeout: 2 * time.Second} } switch opts.CertChainPurpose { @@ -118,8 +118,8 @@ func NewWithOptions(opts Options) (ContextRevocation, error) { // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { return r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: certChain, - AuthenticSigningTime: signingTime, + CertChain: certChain, + AuthenticdSigningTime: signingTime, }) } @@ -137,7 +137,7 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, - SigningTime: validateContextOpts.AuthenticSigningTime, + SigningTime: validateContextOpts.AuthenticdSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 8d6b310f..39b9f844 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -501,8 +501,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: []*x509.Certificate{}, - AuthenticSigningTime: time.Now(), + CertChain: []*x509.Certificate{}, + AuthenticdSigningTime: time.Now(), }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -522,8 +522,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -549,8 +549,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -581,8 +581,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -613,8 +613,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -651,8 +651,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -679,8 +679,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -711,8 +711,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -747,8 +747,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticdSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) From fb94266b9dfe796d2cbeb5c9b8f8e2c42e31fea3 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 31 Jul 2024 13:57:48 +0800 Subject: [PATCH 14/25] updated per code review Signed-off-by: Patrick Zheng --- revocation/revocation.go | 10 +++++----- revocation/revocation_test.go | 36 +++++++++++++++++------------------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 8877cfc9..705a4caf 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -44,10 +44,10 @@ type ValidateContextOptions struct { // been validated. REQUIRED. CertChain []*x509.Certificate - // AuthenticdSigningTime denotes the authentic signing time of the signature. + // AuthenticSigningTime denotes the authentic signing time of the signature. // It is solely used under signing scheme `notary.x509.signingAuthority`. // OPTIONAL. - AuthenticdSigningTime time.Time + AuthenticSigningTime time.Time } // Validator is an interface that provides revocation checking with @@ -118,8 +118,8 @@ func NewWithOptions(opts Options) (Validator, error) { // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { return r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: certChain, - AuthenticdSigningTime: signingTime, + CertChain: certChain, + AuthenticSigningTime: signingTime, }) } @@ -137,7 +137,7 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, - SigningTime: validateContextOpts.AuthenticdSigningTime, + SigningTime: validateContextOpts.AuthenticSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 39b9f844..8d6b310f 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -501,8 +501,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: []*x509.Certificate{}, - AuthenticdSigningTime: time.Now(), + CertChain: []*x509.Certificate{}, + AuthenticSigningTime: time.Now(), }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -522,8 +522,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -549,8 +549,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -581,8 +581,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -613,8 +613,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -651,8 +651,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -679,8 +679,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -711,8 +711,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -747,8 +747,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticdSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) From ef895d8535beda42abf07f728c7ec5a59c5fc4b2 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 31 Jul 2024 14:07:55 +0800 Subject: [PATCH 15/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 705a4caf..1e6b5d73 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -82,7 +82,7 @@ func New(httpClient *http.Client) (Revocation, error) { // Options specifies values that are needed to check revocation type Options struct { // OCSPHTTPClient is the HTTP client for OCSP request. If not provided, - // http.DefaultClient will be used. + // a default *http.Client with timeout of 2 seconds will be used. // OPTIONAL. OCSPHTTPClient *http.Client @@ -92,7 +92,7 @@ type Options struct { CertChainPurpose x509.ExtKeyUsage } -// NewWithOptions constructs a ContextRevocation with the specified options +// NewWithOptions constructs a Validator with the specified options func NewWithOptions(opts Options) (Validator, error) { if opts.OCSPHTTPClient == nil { opts.OCSPHTTPClient = &http.Client{Timeout: 2 * time.Second} From 74357d9016128eea6b9c028650ef36009c7671aa Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 31 Jul 2024 14:31:03 +0800 Subject: [PATCH 16/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 1e6b5d73..b940ef95 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -45,7 +45,7 @@ type ValidateContextOptions struct { CertChain []*x509.Certificate // AuthenticSigningTime denotes the authentic signing time of the signature. - // It is solely used under signing scheme `notary.x509.signingAuthority`. + // It can be used only when signing scheme is `notary.x509.signingAuthority`. // OPTIONAL. AuthenticSigningTime time.Time } From 17703be86fe19885474f2d8a070ad103b5cd96c8 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Thu, 1 Aug 2024 08:56:28 +0800 Subject: [PATCH 17/25] updated per code review Signed-off-by: Patrick Zheng --- revocation/revocation.go | 11 +++++------ revocation/revocation_test.go | 36 +++++++++++++++++------------------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index b940ef95..51fa0e20 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -44,10 +44,9 @@ type ValidateContextOptions struct { // been validated. REQUIRED. CertChain []*x509.Certificate - // AuthenticSigningTime denotes the authentic signing time of the signature. - // It can be used only when signing scheme is `notary.x509.signingAuthority`. + // SigningTime denotes the signing time of the signature. // OPTIONAL. - AuthenticSigningTime time.Time + SigningTime time.Time } // Validator is an interface that provides revocation checking with @@ -118,8 +117,8 @@ func NewWithOptions(opts Options) (Validator, error) { // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { return r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: certChain, - AuthenticSigningTime: signingTime, + CertChain: certChain, + SigningTime: signingTime, }) } @@ -137,7 +136,7 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, - SigningTime: validateContextOpts.AuthenticSigningTime, + SigningTime: validateContextOpts.SigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 8d6b310f..d7b2133d 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -501,8 +501,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: []*x509.Certificate{}, - AuthenticSigningTime: time.Now(), + CertChain: []*x509.Certificate{}, + SigningTime: time.Now(), }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -522,8 +522,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -549,8 +549,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -581,8 +581,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -613,8 +613,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -651,8 +651,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -679,8 +679,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), + CertChain: revokableChain, + SigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -711,8 +711,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + SigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -747,8 +747,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + SigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) From 2272d07a1ba7c3c02d809c52c43ed635a75045f5 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 6 Aug 2024 07:58:12 +0800 Subject: [PATCH 18/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 10 +++++----- revocation/revocation_test.go | 36 +++++++++++++++++------------------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 51fa0e20..963ea49e 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -44,9 +44,9 @@ type ValidateContextOptions struct { // been validated. REQUIRED. CertChain []*x509.Certificate - // SigningTime denotes the signing time of the signature. + // AuthenticSigningTime denotes the authentic signing time of the signature. // OPTIONAL. - SigningTime time.Time + AuthenticSigningTime time.Time } // Validator is an interface that provides revocation checking with @@ -117,8 +117,8 @@ func NewWithOptions(opts Options) (Validator, error) { // https://github.com/notaryproject/notation-core-go/issues/125 func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) { return r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: certChain, - SigningTime: signingTime, + CertChain: certChain, + AuthenticSigningTime: signingTime, }) } @@ -136,7 +136,7 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, CertChainPurpose: r.certChainPurpose, - SigningTime: validateContextOpts.SigningTime, + SigningTime: validateContextOpts.AuthenticSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index d7b2133d..8d6b310f 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -501,8 +501,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: []*x509.Certificate{}, - SigningTime: time.Now(), + CertChain: []*x509.Certificate{}, + AuthenticSigningTime: time.Now(), }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -522,8 +522,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -549,8 +549,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -581,8 +581,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -613,8 +613,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -651,8 +651,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -679,8 +679,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now(), + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -711,8 +711,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -747,8 +747,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("Expected successful creation of revocation, but received error: %v", err) } certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - SigningTime: time.Now().Add(time.Hour), + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) From ac2e6499c36d9aea848ed8f86ce44fb37695177c Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 6 Aug 2024 09:37:37 +0800 Subject: [PATCH 19/25] added more comments Signed-off-by: Patrick Zheng --- revocation/revocation.go | 1 + 1 file changed, 1 insertion(+) diff --git a/revocation/revocation.go b/revocation/revocation.go index 963ea49e..b15aeabb 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -45,6 +45,7 @@ type ValidateContextOptions struct { CertChain []*x509.Certificate // AuthenticSigningTime denotes the authentic signing time of the signature. + // It is solely used under signing scheme notary.x509.signingAuthority. // OPTIONAL. AuthenticSigningTime time.Time } From da3704d36861f53e43ee932d1beca6f2ca4271c9 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 6 Aug 2024 09:39:31 +0800 Subject: [PATCH 20/25] added more comments Signed-off-by: Patrick Zheng --- revocation/revocation.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index b15aeabb..a9e75b3c 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -45,8 +45,10 @@ type ValidateContextOptions struct { CertChain []*x509.Certificate // AuthenticSigningTime denotes the authentic signing time of the signature. - // It is solely used under signing scheme notary.x509.signingAuthority. + // It is used to compare with the InvalidityDate during revocation check. // OPTIONAL. + // + // Reference: https://github.com/notaryproject/specifications/blob/v1.0.0/specs/trust-store-trust-policy.md#revocation-checking-with-ocsp AuthenticSigningTime time.Time } From 3ca469df3b26e092f6942581f84202c4dcd95b83 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 6 Aug 2024 13:49:42 +0800 Subject: [PATCH 21/25] update Signed-off-by: Patrick Zheng --- revocation/revocation.go | 46 ++++++++++----------- revocation/revocation_test.go | 78 +++++++++++++++++++---------------- 2 files changed, 64 insertions(+), 60 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index a9e75b3c..35bcd36c 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -32,9 +32,9 @@ import ( // Deprecated: Revocation exists for backwards compatibility and should not be used. // To perform revocation check, use [Validator]. type Revocation interface { - // Validate checks the revocation status for a certificate chain using OCSP - // and returns an array of CertRevocationResults that contain the results - // and any errors that are encountered during the process + // Validate checks the revocation status for a code signing certificate chain + // using OCSP and returns an array of CertRevocationResults that contain + // the results and any errors that are encountered during the process Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } @@ -50,6 +50,11 @@ type ValidateContextOptions struct { // // Reference: https://github.com/notaryproject/specifications/blob/v1.0.0/specs/trust-store-trust-policy.md#revocation-checking-with-ocsp AuthenticSigningTime time.Time + + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + // REQUIRED. + CertChainPurpose x509.ExtKeyUsage } // Validator is an interface that provides revocation checking with @@ -63,8 +68,7 @@ type Validator interface { // revocation is an internal struct used for revocation checking type revocation struct { - httpClient *http.Client - certChainPurpose x509.ExtKeyUsage + httpClient *http.Client } // New constructs a revocation object for code signing certificate chain. @@ -76,8 +80,7 @@ func New(httpClient *http.Client) (Revocation, error) { return nil, errors.New("invalid input: a non-nil httpClient must be specified") } return &revocation{ - httpClient: httpClient, - certChainPurpose: x509.ExtKeyUsageCodeSigning, + httpClient: httpClient, }, nil } @@ -87,11 +90,6 @@ type Options struct { // a default *http.Client with timeout of 2 seconds will be used. // OPTIONAL. OCSPHTTPClient *http.Client - - // CertChainPurpose is the purpose of the certificate chain. Supported - // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - // REQUIRED. - CertChainPurpose x509.ExtKeyUsage } // NewWithOptions constructs a Validator with the specified options @@ -100,21 +98,14 @@ func NewWithOptions(opts Options) (Validator, error) { opts.OCSPHTTPClient = &http.Client{Timeout: 2 * time.Second} } - switch opts.CertChainPurpose { - case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: - default: - return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) - } - return &revocation{ - httpClient: opts.OCSPHTTPClient, - certChainPurpose: opts.CertChainPurpose, + httpClient: opts.OCSPHTTPClient, }, nil } -// Validate checks the revocation status for a certificate chain using OCSP and -// returns an array of CertRevocationResults that contain the results and any -// errors that are encountered during the process +// Validate checks the revocation status for a code signing certificate chain +// using OCSP and returns an array of CertRevocationResults that contain the +// results and any errors that are encountered during the process. // // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 @@ -122,6 +113,7 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti return r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: certChain, AuthenticSigningTime: signingTime, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, }) } @@ -136,9 +128,15 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return nil, result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} } + switch validateContextOpts.CertChainPurpose { + case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: + default: + return nil, fmt.Errorf("unsupported certificate chain purpose %v", validateContextOpts.CertChainPurpose) + } + return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, - CertChainPurpose: r.certChainPurpose, + CertChainPurpose: validateContextOpts.CertChainPurpose, SigningTime: validateContextOpts.AuthenticSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 8d6b310f..30aac184 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -102,25 +102,11 @@ func TestNew(t *testing.T) { func TestNewWithOptions(t *testing.T) { t.Run("nil OCSP HTTP Client", func(t *testing.T) { - _, err := NewWithOptions(Options{ - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - }) + _, err := NewWithOptions(Options{}) if err != nil { t.Fatal(err) } }) - - t.Run("invalid CertChainPurpose", func(t *testing.T) { - _, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{}, - CertChainPurpose: -1, - }) - expectedErrMsg := "unsupported certificate chain purpose -1" - if err == nil || err.Error() != expectedErrMsg { - t.Fatalf("expected %s, but got %s", expectedErrMsg, err.Error()) - } - }) - } func TestCheckRevocationStatusForSingleCert(t *testing.T) { @@ -494,8 +480,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -503,6 +488,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: []*x509.Certificate{}, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -515,8 +501,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check non-revoked chain", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -524,6 +509,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -538,12 +524,30 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } validateEquivalentCertResults(certResults, expectedCertResults, t) }) + + t.Run("invalid CertChainPurpose", func(t *testing.T) { + r, err := NewWithOptions(Options{ + OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, + }) + if err != nil { + t.Errorf("Expected successful creation of revocation, but received error: %v", err) + } + _, err = r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + CertChainPurpose: -1, + }) + expectedErrMsg := "unsupported certificate chain purpose -1" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err.Error()) + } + }) + t.Run("check chain with 1 Unknown cert", func(t *testing.T) { // 3rd cert will be unknown, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -551,6 +555,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -574,8 +579,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -583,6 +587,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -606,8 +611,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be unknown, 5th will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -615,6 +619,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -644,8 +649,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -653,6 +657,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -672,8 +677,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be unknown, 5th will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -681,6 +685,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -704,8 +709,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -713,6 +717,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now().Add(time.Hour), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -740,8 +745,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("exected zeroTime.IsZero() to be true") } r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + OCSPHTTPClient: client, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -749,6 +753,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now().Add(time.Hour), + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -990,14 +995,15 @@ func TestCheckRevocationInvalidChain(t *testing.T) { func TestValidateContext(t *testing.T) { r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, + OCSPHTTPClient: &http.Client{}, }) if err != nil { t.Fatal(err) } expectedErrMsg := "invalid chain: expected chain to be correct and complete: chain does not contain any certificates" - _, err = r.ValidateContext(context.Background(), ValidateContextOptions{}) + _, err = r.ValidateContext(context.Background(), ValidateContextOptions{ + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + }) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } From 8a09715b8a0bc07517bc1802195ef6ef9608c506 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 6 Aug 2024 14:07:25 +0800 Subject: [PATCH 22/25] Revert "update" This reverts commit 3ca469df3b26e092f6942581f84202c4dcd95b83. Signed-off-by: Patrick Zheng --- revocation/revocation.go | 46 +++++++++++---------- revocation/revocation_test.go | 78 ++++++++++++++++------------------- 2 files changed, 60 insertions(+), 64 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 35bcd36c..a9e75b3c 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -32,9 +32,9 @@ import ( // Deprecated: Revocation exists for backwards compatibility and should not be used. // To perform revocation check, use [Validator]. type Revocation interface { - // Validate checks the revocation status for a code signing certificate chain - // using OCSP and returns an array of CertRevocationResults that contain - // the results and any errors that are encountered during the process + // Validate checks the revocation status for a certificate chain using OCSP + // and returns an array of CertRevocationResults that contain the results + // and any errors that are encountered during the process Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } @@ -50,11 +50,6 @@ type ValidateContextOptions struct { // // Reference: https://github.com/notaryproject/specifications/blob/v1.0.0/specs/trust-store-trust-policy.md#revocation-checking-with-ocsp AuthenticSigningTime time.Time - - // CertChainPurpose is the purpose of the certificate chain. Supported - // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - // REQUIRED. - CertChainPurpose x509.ExtKeyUsage } // Validator is an interface that provides revocation checking with @@ -68,7 +63,8 @@ type Validator interface { // revocation is an internal struct used for revocation checking type revocation struct { - httpClient *http.Client + httpClient *http.Client + certChainPurpose x509.ExtKeyUsage } // New constructs a revocation object for code signing certificate chain. @@ -80,7 +76,8 @@ func New(httpClient *http.Client) (Revocation, error) { return nil, errors.New("invalid input: a non-nil httpClient must be specified") } return &revocation{ - httpClient: httpClient, + httpClient: httpClient, + certChainPurpose: x509.ExtKeyUsageCodeSigning, }, nil } @@ -90,6 +87,11 @@ type Options struct { // a default *http.Client with timeout of 2 seconds will be used. // OPTIONAL. OCSPHTTPClient *http.Client + + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + // REQUIRED. + CertChainPurpose x509.ExtKeyUsage } // NewWithOptions constructs a Validator with the specified options @@ -98,14 +100,21 @@ func NewWithOptions(opts Options) (Validator, error) { opts.OCSPHTTPClient = &http.Client{Timeout: 2 * time.Second} } + switch opts.CertChainPurpose { + case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: + default: + return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) + } + return &revocation{ - httpClient: opts.OCSPHTTPClient, + httpClient: opts.OCSPHTTPClient, + certChainPurpose: opts.CertChainPurpose, }, nil } -// Validate checks the revocation status for a code signing certificate chain -// using OCSP and returns an array of CertRevocationResults that contain the -// results and any errors that are encountered during the process. +// Validate checks the revocation status for a certificate chain using OCSP and +// returns an array of CertRevocationResults that contain the results and any +// errors that are encountered during the process // // TODO: add CRL support // https://github.com/notaryproject/notation-core-go/issues/125 @@ -113,7 +122,6 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti return r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: certChain, AuthenticSigningTime: signingTime, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, }) } @@ -128,15 +136,9 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va return nil, result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} } - switch validateContextOpts.CertChainPurpose { - case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: - default: - return nil, fmt.Errorf("unsupported certificate chain purpose %v", validateContextOpts.CertChainPurpose) - } - return ocsp.CheckStatus(ocsp.Options{ CertChain: validateContextOpts.CertChain, - CertChainPurpose: validateContextOpts.CertChainPurpose, + CertChainPurpose: r.certChainPurpose, SigningTime: validateContextOpts.AuthenticSigningTime, HTTPClient: r.httpClient, }) diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 30aac184..8d6b310f 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -102,11 +102,25 @@ func TestNew(t *testing.T) { func TestNewWithOptions(t *testing.T) { t.Run("nil OCSP HTTP Client", func(t *testing.T) { - _, err := NewWithOptions(Options{}) + _, err := NewWithOptions(Options{ + CertChainPurpose: x509.ExtKeyUsageCodeSigning, + }) if err != nil { t.Fatal(err) } }) + + t.Run("invalid CertChainPurpose", func(t *testing.T) { + _, err := NewWithOptions(Options{ + OCSPHTTPClient: &http.Client{}, + CertChainPurpose: -1, + }) + expectedErrMsg := "unsupported certificate chain purpose -1" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err.Error()) + } + }) + } func TestCheckRevocationStatusForSingleCert(t *testing.T) { @@ -480,7 +494,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, + OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -488,7 +503,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: []*x509.Certificate{}, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} if err == nil || err.Error() != expectedErr.Error() { @@ -501,7 +515,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("check non-revoked chain", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -509,7 +524,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -524,30 +538,12 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } validateEquivalentCertResults(certResults, expectedCertResults, t) }) - - t.Run("invalid CertChainPurpose", func(t *testing.T) { - r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, - }) - if err != nil { - t.Errorf("Expected successful creation of revocation, but received error: %v", err) - } - _, err = r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChain: revokableChain, - AuthenticSigningTime: time.Now(), - CertChainPurpose: -1, - }) - expectedErrMsg := "unsupported certificate chain purpose -1" - if err == nil || err.Error() != expectedErrMsg { - t.Fatalf("expected %s, but got %s", expectedErrMsg, err.Error()) - } - }) - t.Run("check chain with 1 Unknown cert", func(t *testing.T) { // 3rd cert will be unknown, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -555,7 +551,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -579,7 +574,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -587,7 +583,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -611,7 +606,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be unknown, 5th will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -619,7 +615,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -649,7 +644,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -657,7 +653,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -677,7 +672,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be unknown, 5th will be future revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -685,7 +681,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now(), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -709,7 +704,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { // 3rd cert will be revoked, the rest will be good client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -717,7 +713,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now().Add(time.Hour), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -745,7 +740,8 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Errorf("exected zeroTime.IsZero() to be true") } r, err := NewWithOptions(Options{ - OCSPHTTPClient: client, + OCSPHTTPClient: client, + CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -753,7 +749,6 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { certResults, err := r.ValidateContext(context.Background(), ValidateContextOptions{ CertChain: revokableChain, AuthenticSigningTime: time.Now().Add(time.Hour), - CertChainPurpose: x509.ExtKeyUsageTimeStamping, }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) @@ -995,15 +990,14 @@ func TestCheckRevocationInvalidChain(t *testing.T) { func TestValidateContext(t *testing.T) { r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{}, + OCSPHTTPClient: &http.Client{}, + CertChainPurpose: x509.ExtKeyUsageCodeSigning, }) if err != nil { t.Fatal(err) } expectedErrMsg := "invalid chain: expected chain to be correct and complete: chain does not contain any certificates" - _, err = r.ValidateContext(context.Background(), ValidateContextOptions{ - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - }) + _, err = r.ValidateContext(context.Background(), ValidateContextOptions{}) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } From 6166ff290ce256c9ee1ca3c244ff17c517197172 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 7 Aug 2024 11:38:37 +0800 Subject: [PATCH 23/25] updated per discussions Signed-off-by: Patrick Zheng --- revocation/ocsp/ocsp.go | 12 +-- revocation/ocsp/ocsp_test.go | 173 +++++++++++++++------------------- revocation/purpose/purpose.go | 28 ++++++ revocation/revocation.go | 11 ++- revocation/revocation_test.go | 24 ++--- 5 files changed, 127 insertions(+), 121 deletions(-) create mode 100644 revocation/purpose/purpose.go diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index c88aa3a2..cb9e97cc 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -31,6 +31,7 @@ import ( "sync" "time" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/revocation/result" coreX509 "github.com/notaryproject/notation-core-go/x509" "golang.org/x/crypto/ocsp" @@ -41,10 +42,9 @@ type Options struct { CertChain []*x509.Certificate // CertChainPurpose is the purpose of the certificate chain. Supported - // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. - // When not provided, the default value x509.ExtKeyUsageAny is also taken as - // a code signing certificate chain. - CertChainPurpose x509.ExtKeyUsage + // values are CodeSigning and Timestamping. + // When not provided, the default value is CodeSigning. + CertChainPurpose purpose.Purpose SigningTime time.Time HTTPClient *http.Client @@ -68,7 +68,7 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { } switch opts.CertChainPurpose { - case x509.ExtKeyUsageAny, x509.ExtKeyUsageCodeSigning: + case purpose.CodeSigning: // Since ValidateCodeSigningCertChain is using authentic signing time, // signing time may be zero. // Thus, it is better to pass nil here than fail for a cert's NotBefore @@ -76,7 +76,7 @@ func CheckStatus(opts Options) ([]*result.CertRevocationResult, error) { if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { return nil, result.InvalidChainError{Err: err} } - case x509.ExtKeyUsageTimeStamping: + case purpose.Timestamping: if err := coreX509.ValidateTimestampingCertChain(opts.CertChain); err != nil { return nil, result.InvalidChainError{Err: err} } diff --git a/revocation/ocsp/ocsp_test.go b/revocation/ocsp/ocsp_test.go index a4fc07ac..8a1479da 100644 --- a/revocation/ocsp/ocsp_test.go +++ b/revocation/ocsp/ocsp_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" @@ -88,10 +89,9 @@ func TestCheckStatus(t *testing.T) { t.Run("check non-revoked cert", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -101,10 +101,9 @@ func TestCheckStatus(t *testing.T) { t.Run("check cert with Unknown OCSP response", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Unknown}, nil, true) opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -119,10 +118,9 @@ func TestCheckStatus(t *testing.T) { t.Run("check OCSP revoked cert", func(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Revoked}, nil, true) opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -138,10 +136,9 @@ func TestCheckStatus(t *testing.T) { revokedTime := time.Now().Add(time.Hour) client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Revoked}, &revokedTime, true) opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResult := certCheckStatus(revokableChain[0], revokableChain[1], opts) @@ -154,10 +151,9 @@ func TestCheckStatusForSelfSignedCert(t *testing.T) { selfSignedTuple := testhelper.GetRSASelfSignedSigningCertTuple("Notation revocation test self-signed cert") client := testhelper.MockClient([]testhelper.RSACertTuple{selfSignedTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{selfSignedTuple.Cert}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{selfSignedTuple.Cert}, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -172,10 +168,9 @@ func TestCheckStatusForRootCert(t *testing.T) { rootTuple := testhelper.GetRSARootCertificate() client := testhelper.MockClient([]testhelper.RSACertTuple{rootTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{rootTuple.Cert}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{rootTuple.Cert}, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -192,10 +187,9 @@ func TestCheckStatusForNonSelfSignedSingleCert(t *testing.T) { certTuple := testhelper.GetRSALeafCertificate() client := testhelper.MockClient([]testhelper.RSACertTuple{certTuple}, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: []*x509.Certificate{certTuple.Cert}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: []*x509.Certificate{certTuple.Cert}, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -219,10 +213,9 @@ func TestCheckStatusForChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { opts := Options{ - CertChain: []*x509.Certificate{}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: []*x509.Certificate{}, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) expectedErr := result.InvalidChainError{Err: errors.New("chain does not contain any certificates")} @@ -237,7 +230,7 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, + CertChainPurpose: purpose.CodeSigning, SigningTime: time.Now(), HTTPClient: client, } @@ -260,10 +253,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) // 3rd cert will be unknown, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -289,10 +281,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -318,10 +309,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be unknown, 5th will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -353,10 +343,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -378,10 +367,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be unknown, 5th will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -407,10 +395,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now().Add(time.Hour), - HTTPClient: client, + CertChain: revokableChain, + SigningTime: time.Now().Add(time.Hour), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -437,10 +424,9 @@ func TestCheckStatusForChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) // 3rd cert will be revoked, the rest will be good opts := Options{ - CertChain: revokableChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: zeroTime, - HTTPClient: client, + CertChain: revokableChain, + SigningTime: zeroTime, + HTTPClient: client, } if !zeroTime.IsZero() { @@ -498,10 +484,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("no OCSPServer specified", func(t *testing.T) { opts := Options{ - CertChain: noOCSPChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: noOCSPChain, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err != nil { @@ -521,10 +506,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("chain missing root", func(t *testing.T) { opts := Options{ - CertChain: noRootChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: noRootChain, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != chainRootErr.Error() { @@ -537,10 +521,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("backwards chain", func(t *testing.T) { opts := Options{ - CertChain: backwardsChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: http.DefaultClient, + CertChain: backwardsChain, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != backwardsChainErr.Error() { @@ -554,7 +537,7 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("check codesigning cert with PurposeTimestamping", func(t *testing.T) { opts := Options{ CertChain: okChain, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, SigningTime: time.Now(), HTTPClient: http.DefaultClient, } @@ -598,10 +581,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("timeout", func(t *testing.T) { timeoutClient := &http.Client{Timeout: 1 * time.Nanosecond} opts := Options{ - CertChain: okChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: timeoutClient, + CertChain: okChain, + SigningTime: time.Now(), + HTTPClient: timeoutClient, } certResults, err := CheckStatus(opts) if err != nil { @@ -628,10 +610,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("expired ocsp response", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: expiredChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: expiredChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err != nil { @@ -653,10 +634,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("pkixNoCheck missing", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, false) opts := Options{ - CertChain: okChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: okChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -674,10 +654,9 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("non-HTTP URI error", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: noHTTPChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: noHTTPChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err != nil { @@ -722,10 +701,9 @@ func TestCheckOCSPInvalidChain(t *testing.T) { t.Run("chain missing intermediate", func(t *testing.T) { client := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: missingIntermediateChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: missingIntermediateChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != missingIntermediateErr.Error() { @@ -739,10 +717,9 @@ func TestCheckOCSPInvalidChain(t *testing.T) { t.Run("chain out of order", func(t *testing.T) { client := testhelper.MockClient(misorderedIntermediateTuples, []ocsp.ResponseStatus{ocsp.Good}, nil, true) opts := Options{ - CertChain: misorderedIntermediateChain, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, - SigningTime: time.Now(), - HTTPClient: client, + CertChain: misorderedIntermediateChain, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) if err == nil || err.Error() != misorderedChainErr.Error() { diff --git a/revocation/purpose/purpose.go b/revocation/purpose/purpose.go new file mode 100644 index 00000000..7d1f55c0 --- /dev/null +++ b/revocation/purpose/purpose.go @@ -0,0 +1,28 @@ +// 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 purpose provides purposes of the certificate chain whose revocation +// status is checked +package purpose + +// Purpose is an enum for purpose of the certificate chain whose revocation +// status is checked +type Purpose int + +const ( + // CodeSigning means the certificate chain is a code signing chain + CodeSigning Purpose = iota + + // Timestamping means the certificate chain is a timestamping chain + Timestamping +) diff --git a/revocation/revocation.go b/revocation/revocation.go index a9e75b3c..78932a64 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -24,6 +24,7 @@ import ( "time" "github.com/notaryproject/notation-core-go/revocation/ocsp" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/revocation/result" ) @@ -64,7 +65,7 @@ type Validator interface { // revocation is an internal struct used for revocation checking type revocation struct { httpClient *http.Client - certChainPurpose x509.ExtKeyUsage + certChainPurpose purpose.Purpose } // New constructs a revocation object for code signing certificate chain. @@ -77,7 +78,7 @@ func New(httpClient *http.Client) (Revocation, error) { } return &revocation{ httpClient: httpClient, - certChainPurpose: x509.ExtKeyUsageCodeSigning, + certChainPurpose: purpose.CodeSigning, }, nil } @@ -89,9 +90,9 @@ type Options struct { OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain. Supported - // values are x509.ExtKeyUsageCodeSigning and x509.ExtKeyUsageTimeStamping. + // values are CodeSigning and TimeStamping. // REQUIRED. - CertChainPurpose x509.ExtKeyUsage + CertChainPurpose purpose.Purpose } // NewWithOptions constructs a Validator with the specified options @@ -101,7 +102,7 @@ func NewWithOptions(opts Options) (Validator, error) { } switch opts.CertChainPurpose { - case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageTimeStamping: + case purpose.CodeSigning, purpose.Timestamping: default: return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) } diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 8d6b310f..f663880c 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -23,6 +23,7 @@ import ( "time" revocationocsp "github.com/notaryproject/notation-core-go/revocation/ocsp" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/testhelper" "golang.org/x/crypto/ocsp" @@ -103,7 +104,7 @@ func TestNew(t *testing.T) { func TestNewWithOptions(t *testing.T) { t.Run("nil OCSP HTTP Client", func(t *testing.T) { _, err := NewWithOptions(Options{ - CertChainPurpose: x509.ExtKeyUsageCodeSigning, + CertChainPurpose: purpose.CodeSigning, }) if err != nil { t.Fatal(err) @@ -495,7 +496,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { t.Run("empty chain", func(t *testing.T) { r, err := NewWithOptions(Options{ OCSPHTTPClient: &http.Client{Timeout: 5 * time.Second}, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -516,7 +517,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -543,7 +544,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -575,7 +576,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -607,7 +608,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -645,7 +646,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -673,7 +674,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Unknown, ocsp.Good, ocsp.Revoked, ocsp.Good}, &revokedTime, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -705,7 +706,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { client := testhelper.MockClient(testChain, []ocsp.ResponseStatus{ocsp.Good, ocsp.Good, ocsp.Revoked, ocsp.Good}, nil, true) r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -741,7 +742,7 @@ func TestCheckRevocationStatusForTimestampChain(t *testing.T) { } r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CertChainPurpose: x509.ExtKeyUsageTimeStamping, + CertChainPurpose: purpose.Timestamping, }) if err != nil { t.Errorf("Expected successful creation of revocation, but received error: %v", err) @@ -990,8 +991,7 @@ func TestCheckRevocationInvalidChain(t *testing.T) { func TestValidateContext(t *testing.T) { r, err := NewWithOptions(Options{ - OCSPHTTPClient: &http.Client{}, - CertChainPurpose: x509.ExtKeyUsageCodeSigning, + OCSPHTTPClient: &http.Client{}, }) if err != nil { t.Fatal(err) From 67f085ad129f88e417a950cba611610c56e37232 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 7 Aug 2024 11:46:08 +0800 Subject: [PATCH 24/25] updated per discussions Signed-off-by: Patrick Zheng --- revocation/revocation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 78932a64..567054d7 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -90,7 +90,7 @@ type Options struct { OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain. Supported - // values are CodeSigning and TimeStamping. + // values are CodeSigning and Timestamping. // REQUIRED. CertChainPurpose purpose.Purpose } From 02ab652946b15ff401b19de95e6a65b68ab35df8 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 7 Aug 2024 11:49:40 +0800 Subject: [PATCH 25/25] updated per discussions Signed-off-by: Patrick Zheng --- revocation/revocation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index 567054d7..25ac11da 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -90,8 +90,8 @@ type Options struct { OCSPHTTPClient *http.Client // CertChainPurpose is the purpose of the certificate chain. Supported - // values are CodeSigning and Timestamping. - // REQUIRED. + // values are CodeSigning and Timestamping. Default value is CodeSigning. + // OPTIONAL. CertChainPurpose purpose.Purpose }