Repository: sigstore/sigstore-go (go)

Issue:
Certificate time is always valid when no timestamp is provided
Following up from https://github.com/sigstore/sigstore-go/issues/193#issuecomment-2297158885, we are using the [certificate's timestamp to verify itself](https://github.com/sigstore/sigstore-go/blob/d19dc5bd33f20e43bf49eaafcaeded0173c6b526/pkg/verify/signed_entity.go#L733-L738). This will always pass the time verification because a certificate's "not before" time always falls in the range of "not before" to "not after".

Instead, we should only use current time to verify a certificate's validity. I've started making the change, but some tests are failing, so I wanted to check if I'm missing something and this behavior is what you had expected from the policy flag?

@codysoyland @steiza @cmurphy 

Pull request:
Verify certificate validity with only current time, bump conformance tests
Using a certificate's NBF will always pass the time verification. We should be using only the current time to try to verify a certificate's validity. This is likely to only work with long-lived certificates or where verification happens immediately after signing.

Updated conformance testing to drop detached testing. 

Fixes #276

<!--
Thanks for opening a pull request! Please do not just delete this text.  The three fields below are mandatory.

Please remember to:
- This PR requires an issue. If it is a new feature, the issue should proceed the PR and will have allowed sufficent time for discussions to take place. Please use
issue tags such as "Closes #XYZ" or "Resolves sigstore/repo-name#XYZ".
  [documentation](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
- ensure your commits are signed-off, as sigstore uses the [DCO](https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin) using `git commit -s`, or `git commit -s --amend` if you want to amend already existing commits
- lastly, ensure there are no merge commits!

Thank you :)
-->

#### Summary
<!--
 Explain the **motivation** for making this change. What existing problem does the pull request solve? How can reviewers test this PR?
-->

#### Release Note
<!--
Add a release note for each of the following conditions:

* Config changes (additions, deletions, updates)
* API additions—new endpoint, new response fields, or newly accepted request parameters
* Database changes (any)
* Websocket additions or changes
* Anything noteworthy to an administrator running private sigstore instances (err on the side of over-communicating)
* New features and improvements, including behavioural changes, UI changes and CLI changes
* Bug fixes and fixes of previous known issues
* Deprecation warnings, breaking changes, or compatibility notes

If no release notes are required write NONE. Use past-tense.

-->

#### Documentation
<!--

Does this change require an update to documentation? How will users implement your new feature?

Please reference a PR within https://docs.sigstore.dev

-->


Patch:
diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml
index 553fe20..3c9c0eb 100644
--- a/.github/workflows/conformance.yml
+++ b/.github/workflows/conformance.yml
@@ -42,11 +42,11 @@ jobs:
 
       - run: go build -o conformance cmd/conformance/main.go
 
-      - uses: sigstore/sigstore-conformance@6bd1c54e236c9517da56f7344ad16cc00439fe19 # v0.0.13
+      - uses: sigstore/sigstore-conformance@b0635d4101f11dbd18a50936568a1f7f55b17760 # v0.0.14
         with:
           entrypoint: ${{ github.workspace }}/conformance
 
-      - uses: sigstore/sigstore-conformance@6bd1c54e236c9517da56f7344ad16cc00439fe19 # v0.0.13
+      - uses: sigstore/sigstore-conformance@b0635d4101f11dbd18a50936568a1f7f55b17760 # v0.0.14
         with:
           entrypoint: ${{ github.workspace }}/conformance
           environment: staging
diff --git a/cmd/conformance/main.go b/cmd/conformance/main.go
index 0a085e9..2f22dd6 100644
--- a/cmd/conformance/main.go
+++ b/cmd/conformance/main.go
@@ -15,10 +15,7 @@
 package main
 
 import (
-	"crypto/sha256"
-	"encoding/base64"
 	"encoding/hex"
-	"encoding/pem"
 	"fmt"
 	"log"
 	"os"
@@ -26,7 +23,6 @@ import (
 	"time"
 
 	protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
-	protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1"
 	"github.com/theupdateframework/go-tuf/v2/metadata/fetcher"
 	"google.golang.org/protobuf/encoding/protojson"
 
@@ -39,19 +35,15 @@ import (
 )
 
 var bundlePath *string
-var certPath *string
 var certOIDC *string
 var certSAN *string
 var identityToken *string
-var signaturePath *string
 var staging = false
 var trustedRootPath *string
 
 func usage() {
 	fmt.Println("Usage:")
-	fmt.Printf("\t%s sign --identity-token TOKEN --signature FILE --certificate FILE [--staging] FILE", os.Args[0])
 	fmt.Printf("\t%s sign-bundle --identity-token TOKEN --bundle FILE [--staging] FILE", os.Args[0])
-	fmt.Printf("\t%s verify --signature FILE --certificate FILE --certificate-identity IDENTITY --certificate-oidc-issuer URL [--trusted-root FILE] [--staging] FILE\n", os.Args[0])
 	fmt.Printf("\t%s verify-bundle --bundle FILE --certificate-identity IDENTITY --certificate-oidc-issuer URL [--trusted-root FILE] [--staging] FILE\n", os.Args[0])
 }
 
@@ -100,9 +92,6 @@ func parseArgs() {
 		case "--bundle":
 			bundlePath = &os.Args[i+1]
 			i += 2
-		case "--certificate":
-			certPath = &os.Args[i+1]
-			i += 2
 		case "--certificate-oidc-issuer":
 			certOIDC = &os.Args[i+1]
 			i += 2
@@ -112,9 +101,6 @@ func parseArgs() {
 		case "--identity-token":
 			identityToken = &os.Args[i+1]
 			i += 2
-		case "--signature":
-			signaturePath = &os.Args[i+1]
-			i += 2
 		case "--staging":
 			staging = true
 			i++
@@ -186,36 +172,6 @@ func main() {
 	parseArgs()
 
 	switch os.Args[1] {
-	case "sign":
-		// We don't need Rekor entries for detached materials
-		bundle, err := signBundle(false)
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		messageSignature := bundle.GetMessageSignature()
-		if messageSignature == nil {
-			log.Fatal("unable to load signature")
-		}
-		b64MessageSignature := base64.StdEncoding.EncodeToString(messageSignature.Signature)
-		err = os.WriteFile(*signaturePath, []byte(b64MessageSignature), 0600)
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		verificationMaterial := bundle.GetVerificationMaterial()
-		if verificationMaterial == nil {
-			log.Fatal("unable to load verification material")
-		}
-		certificate := verificationMaterial.GetCertificate()
-		if certificate == nil {
-			log.Fatal("unable to load certificate")
-		}
-		pemBlock := &pem.Block{Type: "certificate", Bytes: certificate.RawBytes}
-		err = os.WriteFile(*certPath, pem.EncodeToMemory(pemBlock), 0600)
-		if err != nil {
-			log.Fatal(err)
-		}
 	case "sign-bundle":
 		bundle, err := signBundle(true)
 		if err != nil {
@@ -230,92 +186,6 @@ func main() {
 		if err != nil {
 			log.Fatal(err)
 		}
-	case "verify":
-		// Load certificate
-		cert, err := os.ReadFile(*certPath)
-		if err != nil {
-			log.Fatalf("unable to open certificate file %s", *certPath)
-		}
-
-		pemCert, _ := pem.Decode(cert)
-		if pemCert == nil {
-			log.Fatalf("unable to load cerficate from %s", *certPath)
-		}
-
-		// Load signature
-		sig, err := os.ReadFile(*signaturePath)
-		if err != nil {
-			log.Fatalf("unable to open signature file %s", *signaturePath)
-		}
-		sigBytes, err := base64.StdEncoding.DecodeString(string(sig))
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		fileBytes, err := os.ReadFile(os.Args[len(os.Args)-1])
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		fileDigest := sha256.Sum256(fileBytes)
-
-		// Construct bundle
-		signingCert := protocommon.X509Certificate{
-			RawBytes: pemCert.Bytes,
-		}
-
-		pb := protobundle.Bundle{
-			MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.1",
-			VerificationMaterial: &protobundle.VerificationMaterial{
-				Content: &protobundle.VerificationMaterial_X509CertificateChain{
-					X509CertificateChain: &protocommon.X509CertificateChain{
-						Certificates: []*protocommon.X509Certificate{&signingCert},
-					},
-				},
-			},
-			Content: &protobundle.Bundle_MessageSignature{
-				MessageSignature: &protocommon.MessageSignature{
-					MessageDigest: &protocommon.HashOutput{
-						Algorithm: protocommon.HashAlgorithm_SHA2_256,
-						Digest:    fileDigest[:],
-					},
-					Signature: sigBytes,
-				},
-			},
-		}
-
-		identityPolicies := []verify.PolicyOption{}
-		if *certOIDC != "" || *certSAN != "" {
-			certID, err := verify.NewShortCertificateIdentity(*certOIDC, "", *certSAN, "")
-			if err != nil {
-				fmt.Println(err)
-				os.Exit(1)
-			}
-
-			identityPolicies = append(identityPolicies, verify.WithCertificateIdentity(certID))
-		}
-
-		// Load trust root
-		tr := getTrustedRoot(staging)
-
-		verifierConfig := []verify.VerifierOption{}
-		verifierConfig = append(verifierConfig, verify.WithoutAnyObserverTimestampsUnsafe(), verify.WithSignedCertificateTimestamps(1))
-
-		// Verify bundle
-		sev, err := verify.NewSignedEntityVerifier(tr, verifierConfig...)
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		bun, err := bundle.NewBundle(&pb)
-		if err != nil {
-			log.Fatal(err)
-		}
-
-		_, err = sev.Verify(bun, verify.NewPolicy(verify.WithArtifactDigest("sha256", fileDigest[:]), identityPolicies...))
-		if err != nil {
-			log.Fatal(err)
-		}
 	case "verify-bundle":
 		// Load bundle
 		b, err := bundle.LoadJSONFromPath(*bundlePath)
diff --git a/pkg/verify/signed_entity.go b/pkg/verify/signed_entity.go
index ec4aa5b..fcdda92 100644
--- a/pkg/verify/signed_entity.go
+++ b/pkg/verify/signed_entity.go
@@ -38,9 +38,9 @@ type SignedEntityVerifier struct {
 }
 
 type VerifierConfig struct { // nolint: revive
-	// weExpectSignedTimestamps requires RFC3161 timestamps to verify
+	// requireSignedTimestamps requires RFC3161 timestamps to verify
 	// short-lived certificates
-	weExpectSignedTimestamps bool
+	requireSignedTimestamps bool
 	// signedTimestampThreshold is the minimum number of verified
 	// RFC3161 timestamps in a bundle
 	signedTimestampThreshold int
@@ -56,20 +56,19 @@ type VerifierConfig struct { // nolint: revive
 	// observerTimestampThreshold is the minimum number of verified
 	// RFC3161 timestamps and/or log integrated timestamps in a bundle
 	observerTimestampThreshold int
-	// weExpectTlogEntries requires log inclusion proofs in a bundle
-	weExpectTlogEntries bool
+	// requireTlogEntries requires log inclusion proofs in a bundle
+	requireTlogEntries bool
 	// tlogEntriesThreshold is the minimum number of verified inclusion
 	// proofs in a bundle
 	tlogEntriesThreshold int
-	// weExpectSCTs requires SCTs in Fulcio certificates
-	weExpectSCTs bool
+	// requireSCTs requires SCTs in Fulcio certificates
+	requireSCTs bool
 	// ctlogEntriesTreshold is the minimum number of verified SCTs in
 	// a Fulcio certificate
 	ctlogEntriesThreshold int
-	// weDoNotExpectAnyObserverTimestamps uses the certificate's lifetime
-	// rather than a provided signed or log timestamp. Most workflows will
-	// not use this option
-	weDoNotExpectAnyObserverTimestamps bool
+	// useCurrentTime uses the current time rather than a provided signed
+	// or log timestamp. Most workflows will not use this option
+	useCurrentTime bool
 }
 
 type VerifierOption func(*VerifierConfig) error
@@ -115,7 +114,7 @@ func WithSignedTimestamps(threshold int) VerifierOption {
 		if threshold < 1 {
 			return errors.New("signed timestamp threshold must be at least 1")
 		}
-		c.weExpectSignedTimestamps = true
+		c.requireSignedTimestamps = true
 		c.signedTimestampThreshold = threshold
 		return nil
 	}
@@ -145,7 +144,7 @@ func WithTransparencyLog(threshold int) VerifierOption {
 		if threshold < 1 {
 			return errors.New("transparency log entry threshold must be at least 1")
 		}
-		c.weExpectTlogEntries = true
+		c.requireTlogEntries = true
 		c.tlogEntriesThreshold = threshold
 		return nil
 	}
@@ -170,30 +169,26 @@ func WithSignedCertificateTimestamps(threshold int) VerifierOption {
 		if threshold < 1 {
 			return errors.New("ctlog entry threshold must be at least 1")
 		}
-		c.weExpectSCTs = true
+		c.requireSCTs = true
 		c.ctlogEntriesThreshold = threshold
 		return nil
 	}
 }
 
-// WithoutAnyObserverTimestampsUnsafe configures the SignedEntityVerifier to not expect
+// WithCurrentTime configures the SignedEntityVerifier to not expect
 // any timestamps from either a Timestamp Authority or a Transparency Log.
-//
-// A SignedEntity without a trusted "observer" timestamp to verify the attached
-// Fulcio certificate can't provide the same kind of integrity guarantee.
-//
-// Do not enable this if you don't know what you are doing; as the name implies,
-// using it defeats part of the security guarantees offered by Sigstore. This
-// option is only useful for testing.
-func WithoutAnyObserverTimestampsUnsafe() VerifierOption {
+// This option should not be enabled when verifying short-lived certificates,
+// as an observer timestamp is needed. This option is useful primarily for
+// private deployments with long-lived code signing certificates.
+func WithCurrentTime() VerifierOption {
 	return func(c *VerifierConfig) error {
-		c.weDoNotExpectAnyObserverTimestamps = true
+		c.useCurrentTime = true
 		return nil
 	}
 }
 
 func (c *VerifierConfig) Validate() error {
-	if !c.requireObserverTimestamps && !c.weExpectSignedTimestamps && !c.requireIntegratedTimestamps && !c.weDoNotExpectAnyObserverTimestamps {
+	if !c.requireObserverTimestamps && !c.requireSignedTimestamps && !c.requireIntegratedTimestamps && !c.useCurrentTime {
 		return errors.New("when initializing a new SignedEntityVerifier, you must specify at least one of " +
 			"WithObserverTimestamps(), WithSignedTimestamps(), WithIntegratedTimestamps(), or WithoutAnyObserverTimestampsUnsafe()")
 	}
@@ -552,7 +547,7 @@ func (v *SignedEntityVerifier) Verify(entity SignedEntity, pb PolicyBuilder) (*V
 		// From spec:
 		// > Unless performing online verification (see §Alternative Workflows), the Verifier MUST extract the  SignedCertificateTimestamp embedded in the leaf certificate, and verify it as in RFC 9162 §8.1.3, using the verification key from the Certificate Transparency Log.
 
-		if v.config.weExpectSCTs {
+		if v.config.requireSCTs {
 			err = VerifySignedCertificateTimestamp(chains, v.config.ctlogEntriesThreshold, v.trustedMaterial)
 			if err != nil {
 				return nil, fmt.Errorf("failed to verify signed certificate timestamp: %w", err)
@@ -647,7 +642,7 @@ func (v *SignedEntityVerifier) Verify(entity SignedEntity, pb PolicyBuilder) (*V
 func (v *SignedEntityVerifier) VerifyTransparencyLogInclusion(entity SignedEntity) ([]TimestampVerificationResult, error) {
 	verifiedTimestamps := []TimestampVerificationResult{}
 
-	if v.config.weExpectTlogEntries {
+	if v.config.requireTlogEntries {
 		// log timestamps should be verified if with WithIntegratedTimestamps or WithObserverTimestamps is used
 		verifiedTlogTimestamps, err := VerifyArtifactTransparencyLog(entity, v.trustedMaterial, v.config.tlogEntriesThreshold,
 			v.config.requireIntegratedTimestamps || v.config.requireObserverTimestamps)
@@ -675,7 +670,7 @@ func (v *SignedEntityVerifier) VerifyObserverTimestamps(entity SignedEntity, log
 
 	// From spec:
 	// > … if verification or timestamp parsing fails, the Verifier MUST abort
-	if v.config.weExpectSignedTimestamps {
+	if v.config.requireSignedTimestamps {
 		verifiedSignedTimestamps, err := VerifyTimestampAuthorityWithThreshold(entity, v.trustedMaterial, v.config.signedTimestampThreshold)
 		if err != nil {
 			return nil, err
@@ -712,19 +707,9 @@ func (v *SignedEntityVerifier) VerifyObserverTimestamps(entity SignedEntity, log
 		}
 	}
 
-	if v.config.weDoNotExpectAnyObserverTimestamps {
-		// if we have a cert, let's pop the leafcert's NotBefore
-		verificationContent, err := entity.VerificationContent()
-		if err != nil {
-			return nil, err
-		}
-
-		if leafCert := verificationContent.Certificate(); leafCert != nil {
-			verifiedTimestamps = append(verifiedTimestamps, TimestampVerificationResult{Type: "LeafCert.NotBefore", URI: "", Timestamp: leafCert.NotBefore})
-		} else {
-			// no cert? use current time
-			verifiedTimestamps = append(verifiedTimestamps, TimestampVerificationResult{Type: "CurrentTime", URI: "", Timestamp: time.Now()})
-		}
+	if v.config.useCurrentTime {
+		// use current time to verify certificate if no signed timestamps are provided
+		verifiedTimestamps = append(verifiedTimestamps, TimestampVerificationResult{Type: "CurrentTime", URI: "", Timestamp: time.Now()})
 	}
 
 	if len(verifiedTimestamps) == 0 {


Test patch:
diff --git a/pkg/verify/signed_entity_test.go b/pkg/verify/signed_entity_test.go
index 98ea657..70ca15a 100644
--- a/pkg/verify/signed_entity_test.go
+++ b/pkg/verify/signed_entity_test.go
@@ -40,7 +40,7 @@ func TestSignedEntityVerifierInitialization(t *testing.T) {
 	assert.Nil(t, err)
 
 	// unless we are really sure we want a verifier without either tlog or tsa
-	_, err = verify.NewSignedEntityVerifier(tr, verify.WithoutAnyObserverTimestampsUnsafe())
+	_, err = verify.NewSignedEntityVerifier(tr, verify.WithCurrentTime())
 	assert.Nil(t, err)
 
 	// can configure the verifiers with thresholds
@@ -68,7 +68,7 @@ func TestSignedEntityVerifierInitRequiresTimestamp(t *testing.T) {
 	assert.NoError(t, err)
 	_, err = verify.NewSignedEntityVerifier(tr, verify.WithTransparencyLog(1), verify.WithObserverTimestamps(1))
 	assert.NoError(t, err)
-	_, err = verify.NewSignedEntityVerifier(tr, verify.WithTransparencyLog(1), verify.WithoutAnyObserverTimestampsUnsafe())
+	_, err = verify.NewSignedEntityVerifier(tr, verify.WithTransparencyLog(1), verify.WithCurrentTime())
 	assert.NoError(t, err)
 }
 
@@ -108,7 +108,7 @@ func TestEntitySignedByPublicGoodWithoutTimestampsVerifiesSuccessfully(t *testin
 	tr := data.PublicGoodTrustedMaterialRoot(t)
 	entity := data.SigstoreJS200ProvenanceBundle(t)
 
-	v, err := verify.NewSignedEntityVerifier(tr, verify.WithoutAnyObserverTimestampsUnsafe())
+	v, err := verify.NewSignedEntityVerifier(tr, verify.WithTransparencyLog(1), verify.WithIntegratedTimestamps(1))
 	assert.NoError(t, err)
 
 	res, err := v.Verify(entity, SkipArtifactAndIdentitiesPolicy)
@@ -205,7 +205,7 @@ func TestEntityWithOthernameSan(t *testing.T) {
 	tr := data.ScaffoldingTrustedMaterialRoot(t)
 	entity := data.OthernameBundle(t)
 
-	v, err := verify.NewSignedEntityVerifier(tr, verify.WithoutAnyObserverTimestampsUnsafe())
+	v, err := verify.NewSignedEntityVerifier(tr, verify.WithTransparencyLog(1), verify.WithIntegratedTimestamps(1))
 	assert.NoError(t, err)
 
 	digest, err := hex.DecodeString("bc103b4a84971ef6459b294a2b98568a2bfb72cded09d4acd1e16366a401f95b")