diff --git a/revocation/ocsp/ocsp.go b/revocation/ocsp/ocsp.go index 359ce7e9..cb9e97cc 100644 --- a/revocation/ocsp/ocsp.go +++ b/revocation/ocsp/ocsp.go @@ -31,29 +31,23 @@ 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" ) -// 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` - SigningTime time.Time - HTTPClient *http.Client + CertChain []*x509.Certificate + + // CertChainPurpose is the purpose of the certificate chain. Supported + // values are CodeSigning and Timestamping. + // When not provided, the default value is CodeSigning. + CertChainPurpose purpose.Purpose + + SigningTime time.Time + HTTPClient *http.Client } const ( @@ -73,21 +67,21 @@ 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 PurposeCodeSigning: + 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 + // being after zero time if err := coreX509.ValidateCodeSigningCertChain(opts.CertChain, nil); err != nil { return nil, result.InvalidChainError{Err: err} } - case PurposeTimestamping: + case purpose.Timestamping: 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)} + 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 14d43df6..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" @@ -228,9 +229,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: purpose.CodeSigning, + SigningTime: time.Now(), + HTTPClient: client, } certResults, err := CheckStatus(opts) @@ -535,7 +537,7 @@ func TestCheckStatusErrors(t *testing.T) { t.Run("check codesigning cert with PurposeTimestamping", func(t *testing.T) { opts := Options{ CertChain: okChain, - CertChainPurpose: PurposeTimestamping, + CertChainPurpose: purpose.Timestamping, SigningTime: time.Now(), HTTPClient: http.DefaultClient, } @@ -548,15 +550,27 @@ func TestCheckStatusErrors(t *testing.T) { } }) + t.Run("check with default CertChainPurpose", func(t *testing.T) { + opts := Options{ + CertChain: okChain, + SigningTime: time.Now(), + HTTPClient: http.DefaultClient, + } + _, err := CheckStatus(opts) + if err != nil { + t.Fatal(err) + } + }) + 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: 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/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 801a0780..25ac11da 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -16,16 +16,22 @@ package revocation import ( + "context" "crypto/x509" "errors" + "fmt" "net/http" "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" ) -// 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 [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 @@ -33,32 +39,77 @@ type Revocation interface { Validate(certChain []*x509.Certificate, signingTime time.Time) ([]*result.CertRevocationResult, error) } +// ValidateContextOptions provides configuration options for revocation checks +type ValidateContextOptions struct { + // CertChain denotes the certificate chain whose revocation status is + // been validated. REQUIRED. + CertChain []*x509.Certificate + + // AuthenticSigningTime denotes the authentic signing time of the signature. + // 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 +} + +// Validator is an interface that provides revocation checking with +// context +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 + ValidateContext(ctx context.Context, validateContextOpts ValidateContextOptions) ([]*result.CertRevocationResult, error) +} + // revocation is an internal struct used for revocation checking type revocation struct { httpClient *http.Client - certChainPurpose ocsp.Purpose + certChainPurpose purpose.Purpose } -// 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") } return &revocation{ httpClient: httpClient, - certChainPurpose: ocsp.PurposeCodeSigning, + certChainPurpose: purpose.CodeSigning, }, 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") +// Options specifies values that are needed to check revocation +type Options struct { + // OCSPHTTPClient is the HTTP client for OCSP request. If not provided, + // 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 CodeSigning and Timestamping. Default value is CodeSigning. + // OPTIONAL. + CertChainPurpose purpose.Purpose +} + +// 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} + } + + switch opts.CertChainPurpose { + case purpose.CodeSigning, purpose.Timestamping: + default: + return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) } + return &revocation{ - httpClient: httpClient, - certChainPurpose: ocsp.PurposeTimestamping, + httpClient: opts.OCSPHTTPClient, + certChainPurpose: opts.CertChainPurpose, }, nil } @@ -69,10 +120,27 @@ func NewTimestamp(httpClient *http.Client) (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(), ValidateContextOptions{ + CertChain: certChain, + AuthenticSigningTime: 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, 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: 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 f9d4f4e5..f663880c 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -14,6 +14,7 @@ package revocation import ( + "context" "crypto/x509" "errors" "fmt" @@ -22,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" @@ -99,12 +101,27 @@ 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{ + CertChainPurpose: purpose.CodeSigning, + }) + 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) { @@ -477,11 +494,17 @@ 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: purpose.Timestamping, + }) 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(), 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) @@ -492,11 +515,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -513,11 +542,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -539,11 +574,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -565,11 +606,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -597,11 +644,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -619,11 +672,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now(), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -645,11 +704,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 := NewTimestamp(client) + r, err := NewWithOptions(Options{ + OCSPHTTPClient: client, + CertChainPurpose: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -675,11 +740,17 @@ 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: purpose.Timestamping, + }) 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(), ValidateContextOptions{ + CertChain: revokableChain, + AuthenticSigningTime: time.Now().Add(time.Hour), + }) if err != nil { t.Errorf("Expected CheckStatus to succeed, but got error: %v", err) } @@ -917,3 +988,17 @@ func TestCheckRevocationInvalidChain(t *testing.T) { } }) } + +func TestValidateContext(t *testing.T) { + r, err := NewWithOptions(Options{ + 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{}) + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err) + } +}