updated per code review

Signed-off-by: Patrick Zheng <[email protected]>

conformance_test.go

@@ -113,7 +113,7 @@ func TestTSATimestampGranted(t *testing.T) {
 	if err != nil {
 		t.Fatal("SignedToken.Info() error =", err)
 	}
-	ts, accuracy, err := info.ExtractGenTime(message)
+	ts, accuracy, err := info.Validate(message)
 	if err != nil {
 		t.Errorf("TSTInfo.Timestamp() error = %v", err)
 	}

http_test.go

@@ -122,7 +122,7 @@ func TestHTTPTimestampGranted(t *testing.T) {
 	if err != nil {
 		t.Fatal("SignedToken.Info() error =", err)
 	}
-	timestamp, accuracy, err := info.ExtractGenTime(message)
+	timestamp, accuracy, err := info.Validate(message)
 	if err != nil {
 		t.Errorf("TSTInfo.Timestamp() error = %v", err)
 	}

pki/pki.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
+	"strings"
 )
 
 // ErrUnknownStatus is used when PKIStatus is not supported
@@ -132,12 +133,17 @@ type StatusInfo struct {
 //
 // Otherwise, Err returns an error with FailInfo if any.
 func (si StatusInfo) Err() error {
+	var err error
 	if si.Status != StatusGranted && si.Status != StatusGrantedWithMods {
 		for _, fi := range failureInfos {
 			if si.FailInfo.At(int(fi)) != 0 {
-				return fmt.Errorf("invalid response with status code %d: %s. Failure info: %w", si.Status, si.Status.String(), fi.Error())
+				err = errors.Join(err, fi.Error())
 			}
 		}
+		if err != nil { // there is FailInfo
+			errMsg := strings.ReplaceAll(err.Error(), "\n", "; ")
+			return fmt.Errorf("invalid response with status code %d: %s. Failure info: %s", si.Status, si.Status.String(), errMsg)
+		}
 		return fmt.Errorf("invalid response with status code %d: %s", si.Status, si.Status.String())
 	}
 	return nil

pki/pki_test.go

@@ -50,6 +50,20 @@ func TestStatusInfo(t *testing.T) {
 		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
 	}
 
+	statusInfo = StatusInfo{
+		Status: StatusRejection,
+		FailInfo: asn1.BitString{
+			// FailureInfoBadRequest and FailureInfoBadDataFormat
+			Bytes:     []byte{0x24},
+			BitLength: 8,
+		},
+	}
+	err = statusInfo.Err()
+	expectedErrMsg = "invalid response with status code 2: rejected. Failure info: transaction not permitted or supported; the data submitted has the wrong format"
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
+	}
+
 	statusInfo = StatusInfo{
 		Status: StatusRejection,
 		FailInfo: asn1.BitString{

response.go

@@ -24,24 +24,6 @@ import (
 	"github.com/notaryproject/tspclient-go/pki"
 )
 
-// signingCertificate contains certificate hash and identifier of the
-// TSA signing certificate.
-//
-// Reference: RFC 2634 5.4 signingCertificate
-//
-//	signingCertificate ::=  SEQUENCE {
-//	 certs        SEQUENCE OF ESSCertID,
-//	 policies     SEQUENCE OF PolicyInformation OPTIONAL }
-type signingCertificate struct {
-	// Certificates contains the list of certificates. The first certificate
-	// MUST be the signing certificate used to verify the timestamp token.
-	Certificates []eSSCertID
-
-	// Policies suggests policy values to be used in the certification path
-	// validation.
-	Policies asn1.RawValue `asn1:"optional"`
-}
-
 // signingCertificateV2 contains certificate hash and identifier of the
 // TSA signing certificate.
 //
@@ -60,25 +42,6 @@ type signingCertificateV2 struct {
 	Policies asn1.RawValue `asn1:"optional"`
 }
 
-// eSSCertID uniquely identifies a certificate.
-//
-// Reference: RFC 2634 5.4.1
-//
-//	eSSCertID ::=  SEQUENCE {
-//	 certHash                 Hash,
-//	 issuerSerial             IssuerSerial OPTIONAL }
-type eSSCertID struct {
-	// CertHash is the certificate hash using algorithm SHA1.
-	// It is computed over the entire DER-encoded certificate
-	// (including the signature)
-	CertHash []byte
-
-	// IssuerSerial holds the issuer and serialNumber of the certificate.
-	// When it is not present, the SignerIdentifier field in the SignerInfo
-	// will be used.
-	IssuerSerial issuerAndSerial `asn1:"optional"`
-}
-
 // eSSCertIDv2 uniquely identifies a certificate.
 //
 // Reference: RFC 5035 4
@@ -210,7 +173,8 @@ func (resp *Response) Validate(req *Request) error {
 	if !info.MessageImprint.Equal(req.MessageImprint) {
 		return &InvalidResponseError{Msg: fmt.Sprintf("message imprint in response %+v does not match with request %+v", info.MessageImprint, req.MessageImprint)}
 	}
-	// check gen time
+	// check gen time to be UTC
+	// reference: https://datatracker.ietf.org/doc/html/rfc3161#section-2.4.2
 	genTime := info.GenTime
 	if genTime.Location() != time.UTC {
 		return &InvalidResponseError{Msg: "TSTInfo genTime must be in UTC"}

token.go

@@ -105,47 +105,23 @@ func (t *SignedToken) Verify(ctx context.Context, opts x509.VerifyOptions) ([]*x
 }
 
 // SigningCertificate gets the signing certificate identified by SignedToken
-// SignerInfo's SigningCertificate or SigningCertificateV2 attribute.
+// SignerInfo's SigningCertificateV2 attribute.
 // If the IssuerSerial field of signing certificate is missing,
 // use signerInfo's sid instead.
-// The identified signing certificate MUST match the hash in SigningCertificate
-// or SigningCertificateV2.
-// When both are present, SigningCertificateV2 is used because it uses hash
-// algorithm beyond SHA1. If both are missing, an error will be returned.
+// The identified signing certificate MUST match the hash in SigningCertificateV2.
 //
-// References: RFC 3161 2.4.1 & 2.4.2; RFC 5816; RFC 5035 4; RFC 2634 5.4
+// References: RFC 3161 2.4.1 & 2.4.2; RFC 5816
 func (t *SignedToken) SigningCertificate(signerInfo *cms.SignerInfo) (*x509.Certificate, error) {
-	var signingCertificate signingCertificate
-	var expectSigningCertificateV1 bool
 	var signingCertificateV2 signingCertificateV2
 	if err := signerInfo.SignedAttributes.Get(oid.SigningCertificateV2, &signingCertificateV2); err != nil {
-		if !errors.Is(err, cms.ErrAttributeNotFound) {
-			return nil, fmt.Errorf("failed to get SigningCertificateV2 from signed attributes: %w", err)
-		}
-		// signingCertificateV2 is missing, try signingCertificate v1 instead
-		expectSigningCertificateV1 = true
-		if err := signerInfo.SignedAttributes.Get(oid.SigningCertificate, &signingCertificate); err != nil {
-			if !errors.Is(err, cms.ErrAttributeNotFound) {
-				return nil, fmt.Errorf("failed to get SigningCertificate from signed attributes: %w", err)
-			}
-			// signingCertificate v1 is missing as well
-			return nil, errors.New("invalid timestamp token: both signingCertificate and signingCertificateV2 fields are missing")
-		}
+		return nil, fmt.Errorf("failed to get SigningCertificateV2 from signed attributes: %w", err)
 	}
 	// get candidate signing certificate
-	var candidateSigningCert *x509.Certificate
-	var issuerSerial issuerAndSerial
-	if expectSigningCertificateV1 {
-		if len(signingCertificate.Certificates) == 0 {
-			return nil, errors.New("signingCertificate does not contain any certificate")
-		}
-		issuerSerial = signingCertificate.Certificates[0].IssuerSerial
-	} else {
-		if len(signingCertificateV2.Certificates) == 0 {
-			return nil, errors.New("signingCertificateV2 does not contain any certificate")
-		}
-		issuerSerial = signingCertificateV2.Certificates[0].IssuerSerial
+	if len(signingCertificateV2.Certificates) == 0 {
+		return nil, errors.New("signingCertificateV2 does not contain any certificate")
 	}
+	issuerSerial := signingCertificateV2.Certificates[0].IssuerSerial
+	var candidateSigningCert *x509.Certificate
 	signed := (*cms.ParsedSignedData)(t)
 	if issuerSerial.SerialNumber != nil {
 		// use IssuerSerial from signed attribute
@@ -169,25 +145,17 @@ func (t *SignedToken) SigningCertificate(signerInfo *cms.SignerInfo) (*x509.Cert
 		return nil, CertificateNotFoundError(errors.New("signing certificate not found in the timestamp token"))
 	}
 	// validate hash of candidate signing certificate
-	var hashFunc crypto.Hash
-	var expectedCertHash []byte
-	if expectSigningCertificateV1 {
-		// Reference: https://datatracker.ietf.org/doc/html/rfc2634#section-5.4.1
-		hashFunc = crypto.SHA1
-		expectedCertHash = signingCertificate.Certificates[0].CertHash
-	} else {
-		// Reference: https://datatracker.ietf.org/doc/html/rfc5035#section-4
-		hashFunc = crypto.SHA256 // default hash algorithm for signingCertificateV2 is id-sha256
-		if signingCertificateV2.Certificates[0].HashAlgorithm.Algorithm != nil {
-			// use hash algorithm from SigningCertificateV2 signed attribute
-			var ok bool
-			hashFunc, ok = oid.ToHash(signingCertificateV2.Certificates[0].HashAlgorithm.Algorithm)
-			if !ok {
-				return nil, errors.New("unsupported certificate hash algorithm in SigningCertificateV2 attribute")
-			}
+	// Reference: https://datatracker.ietf.org/doc/html/rfc5035#section-4
+	hashFunc := crypto.SHA256 // default hash algorithm for signingCertificateV2 is id-sha256
+	if signingCertificateV2.Certificates[0].HashAlgorithm.Algorithm != nil {
+		// use hash algorithm from SigningCertificateV2 signed attribute
+		var ok bool
+		hashFunc, ok = oid.ToHash(signingCertificateV2.Certificates[0].HashAlgorithm.Algorithm)
+		if !ok {
+			return nil, errors.New("unsupported certificate hash algorithm in SigningCertificateV2 attribute")
 		}
-		expectedCertHash = signingCertificateV2.Certificates[0].CertHash
 	}
+	expectedCertHash := signingCertificateV2.Certificates[0].CertHash
 	certHash, err := hashutil.ComputeHash(hashFunc, candidateSigningCert.Raw)
 	if err != nil {
 		return nil, err
@@ -243,9 +211,9 @@ type TSTInfo struct {
 	Extensions     []pkix.Extension `asn1:"optional,tag:1"`
 }
 
-// ExtractGenTime returns the timestamp by TSA and its accuracy.
+// Validate validates tst and returns the GenTime and Accuracy.
 // tst MUST be valid and the time stamped datum MUST match message.
-func (tst *TSTInfo) ExtractGenTime(message []byte) (time.Time, time.Duration, error) {
+func (tst *TSTInfo) Validate(message []byte) (time.Time, time.Duration, error) {
 	if err := tst.validate(message); err != nil {
 		return time.Time{}, 0, err
 	}

token_test.go

@@ -102,11 +102,11 @@ func TestInfo(t *testing.T) {
 }
 
 func TestGetSigningCertificate(t *testing.T) {
-	timestampToken, err := getTimestampTokenFromPath("testdata/TimeStampTokenWithoutSigningCertificateV1orV2.p7s")
+	timestampToken, err := getTimestampTokenFromPath("testdata/TimeStampTokenWithoutSigningCertificate.p7s")
 	if err != nil {
 		t.Fatal(err)
 	}
-	expectedErrMsg := "invalid timestamp token: both signingCertificate and signingCertificateV2 fields are missing"
+	expectedErrMsg := "failed to get SigningCertificateV2 from signed attributes: attribute not found"
 	if _, err := timestampToken.SigningCertificate(&timestampToken.SignerInfos[0]); err == nil || err.Error() != expectedErrMsg {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
@@ -155,15 +155,6 @@ func TestGetSigningCertificate(t *testing.T) {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
 
-	timestampToken, err = getTimestampTokenFromPath("testdata/TimeStampTokenWithInvalidSigningCertificateV1.p7s")
-	if err != nil {
-		t.Fatal(err)
-	}
-	expectedErrMsg = "failed to get SigningCertificate from signed attributes: asn1: syntax error: sequence truncated"
-	if _, err := timestampToken.SigningCertificate(&timestampToken.SignerInfos[0]); err == nil || err.Error() != expectedErrMsg {
-		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
-	}
-
 	timestampToken, err = getTimestampTokenFromPath("testdata/TimeStampTokenWithInvalidSigningCertificateV2.p7s")
 	if err != nil {
 		t.Fatal(err)
@@ -173,15 +164,6 @@ func TestGetSigningCertificate(t *testing.T) {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
 
-	timestampToken, err = getTimestampTokenFromPath("testdata/TimeStampTokenWithSigningCertificateV1NoCert.p7s")
-	if err != nil {
-		t.Fatal(err)
-	}
-	expectedErrMsg = "signingCertificate does not contain any certificate"
-	if _, err := timestampToken.SigningCertificate(&timestampToken.SignerInfos[0]); err == nil || err.Error() != expectedErrMsg {
-		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
-	}
-
 	timestampToken, err = getTimestampTokenFromPath("testdata/TimeStampTokenWithSigningCertificateV2NoCert.p7s")
 	if err != nil {
 		t.Fatal(err)
@@ -195,8 +177,9 @@ func TestGetSigningCertificate(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := timestampToken.SigningCertificate(&timestampToken.SignerInfos[0]); err != nil {
-		t.Fatalf("expected nil error, but got %v", err)
+	expectedErrMsg = "failed to get SigningCertificateV2 from signed attributes: attribute not found"
+	if _, err := timestampToken.SigningCertificate(&timestampToken.SignerInfos[0]); err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
 }
 
@@ -210,7 +193,7 @@ func TestTimestamp(t *testing.T) {
 		t.Fatal(err)
 	}
 	expectedErrMsg := "invalid TSTInfo: mismatched message"
-	if _, _, err := tstInfo.ExtractGenTime([]byte("invalid")); err == nil || err.Error() != expectedErrMsg {
+	if _, _, err := tstInfo.Validate([]byte("invalid")); err == nil || err.Error() != expectedErrMsg {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
 
@@ -222,7 +205,7 @@ func TestTimestamp(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	timestamp, accuracy, err := tstInfo.ExtractGenTime([]byte("notation"))
+	timestamp, accuracy, err := tstInfo.Validate([]byte("notation"))
 	if err != nil {
 		t.Fatalf("expected nil error, but got %v", err)
 	}