From f45197cbd53b9cbaf867cfe5152e3b8cf4ea9766 Mon Sep 17 00:00:00 2001
From: Patrick Zheng <patrickzheng@microsoft.com>
Date: Tue, 16 Jul 2024 08:13:20 +0800
Subject: [PATCH] fix: fix `signerInfo.authenticSigningTime` according to spec
 (#211)

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
---
 signature/types.go      | 23 ++++++++++++++---------
 signature/types_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/signature/types.go b/signature/types.go
index ab53bee8..df69236c 100644
--- a/signature/types.go
+++ b/signature/types.go
@@ -17,6 +17,7 @@ import (
 	"context"
 	"crypto/x509"
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/notaryproject/tspclient-go"
@@ -197,17 +198,21 @@ func (signerInfo *SignerInfo) ExtendedAttribute(key string) (Attribute, error) {
 	return Attribute{}, errors.New("key not in ExtendedAttributes")
 }
 
-// AuthenticSigningTime returns the authentic signing time
+// AuthenticSigningTime returns the authentic signing time under signing scheme
+// notary.x509.signingAuthority.
+// For signing scheme notary.x509, since it only supports authentic timestamp,
+// an error is returned.
+//
+// Reference: https://github.com/notaryproject/specifications/blob/3b0743cd9bb99faee60600dc31d706149775fd49/specs/signature-specification.md#signing-time--authentic-signing-time
 func (signerInfo *SignerInfo) AuthenticSigningTime() (time.Time, error) {
-	switch signerInfo.SignedAttributes.SigningScheme {
+	switch signingScheme := signerInfo.SignedAttributes.SigningScheme; signingScheme {
 	case SigningSchemeX509SigningAuthority:
-		return signerInfo.SignedAttributes.SigningTime, nil
-	case SigningSchemeX509:
-		if len(signerInfo.UnsignedAttributes.TimestampSignature) > 0 {
-			// TODO: Add TSA support for AutheticSigningTime
-			// https://github.com/notaryproject/notation-core-go/issues/38
-			return time.Time{}, errors.New("TSA checking has not been implemented")
+		signingTime := signerInfo.SignedAttributes.SigningTime
+		if signingTime.IsZero() {
+			return time.Time{}, fmt.Errorf("authentic signing time must be present under signing scheme %q", signingScheme)
 		}
+		return signingTime, nil
+	default:
+		return time.Time{}, fmt.Errorf("authentic signing time not supported under signing scheme %q", signingScheme)
 	}
-	return time.Time{}, errors.New("authenticSigningTime not found")
 }
diff --git a/signature/types_test.go b/signature/types_test.go
index f8ff5625..74fe4ef3 100644
--- a/signature/types_test.go
+++ b/signature/types_test.go
@@ -17,6 +17,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 )
 
 func TestSignRequestContext(t *testing.T) {
@@ -51,3 +52,42 @@ func TestSignRequestWithContext(t *testing.T) {
 	}()
 	r.WithContext(nil) // should panic
 }
+
+func TestAuthenticSigningTime(t *testing.T) {
+	testTime := time.Now()
+	signerInfo := SignerInfo{
+		SignedAttributes: SignedAttributes{
+			SigningScheme: "notary.x509.signingAuthority",
+			SigningTime:   testTime,
+		},
+	}
+	authenticSigningTime, err := signerInfo.AuthenticSigningTime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !authenticSigningTime.Equal(testTime) {
+		t.Fatalf("expected %s, but got %s", testTime, authenticSigningTime)
+	}
+
+	signerInfo = SignerInfo{
+		SignedAttributes: SignedAttributes{
+			SigningScheme: "notary.x509.signingAuthority",
+		},
+	}
+	expectedErrMsg := "authentic signing time must be present under signing scheme \"notary.x509.signingAuthority\""
+	_, err = signerInfo.AuthenticSigningTime()
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
+	}
+
+	signerInfo = SignerInfo{
+		SignedAttributes: SignedAttributes{
+			SigningScheme: "notary.x509",
+		},
+	}
+	expectedErrMsg = "authentic signing time not supported under signing scheme \"notary.x509\""
+	_, err = signerInfo.AuthenticSigningTime()
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
+	}
+}