chore: update maintainer list: Junjie Gao retired (#46)

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

.github/.codecov.yml

@@ -14,5 +14,8 @@
 coverage:
   status:
     project:
+      default:
+        target: 80%
+    patch:
       default:
         target: 80%

.github/ISSUE_TEMPLATE/bug-or-issue.yaml

@@ -0,0 +1,60 @@
+# 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.
+
+name: 🐛 Bug or Issue
+description: Something is not working as expected or not working at all! Report it here!
+labels: [bug, triage]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thank you for taking the time to fill out this issue report. 🛑 Please check existing issues first before continuing: https://github.com/notaryproject/tspclient-go/issues
+  - type: textarea
+    id: verbatim
+    validations:
+      required: true
+    attributes:
+      label: "What is not working as expected?"
+      description: "In your own words, describe what the issue is."
+  - type: textarea
+    id: expect
+    validations:
+      required: true
+    attributes:
+      label: "What did you expect to happen?"
+      description: "A clear and concise description of what you expected to happen."
+  - type: textarea
+    id: reproduce
+    validations:
+      required: true
+    attributes:
+      label: "How can we reproduce it?"
+      description: "Detailed steps to reproduce the behavior, code snippets are welcome."
+  - type: textarea
+    id: environment
+    validations:
+      required: true
+    attributes:
+      label: Describe your environment 
+      description: "OS and Golang version"
+  - type: textarea
+    id: version
+    validations:
+      required: true
+    attributes:
+      label: What is the version of your tspclient-go Library?
+      description: "Check the `go.mod` file for the library version."
+  - type: markdown
+    attributes:
+      value: |
+        If you want to contribute to this project, we will be happy to guide you through the contribution process especially when you already have a good proposal or understanding of how to fix this issue. Join us at https://slack.cncf.io/ and choose #notary-project channel.

.github/ISSUE_TEMPLATE/config.yml

@@ -0,0 +1,18 @@
+# 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.
+
+blank_issues_enabled: false
+contact_links:
+  - name: Ask a question
+    url: https://slack.cncf.io/
+    about: "Join #notary-project channel on CNCF Slack"

.github/ISSUE_TEMPLATE/feature-request.yaml

@@ -0,0 +1,53 @@
+# 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.
+
+name: 🚀 Feature Request
+description: Suggest an idea for this project.
+labels: [enhancement, triage]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thank you for taking the time to suggest a useful feature for the project!
+  - type: textarea
+    id: problem
+    validations:
+      required: true
+    attributes:
+      label: "Is your feature request related to a problem?"
+      description: "A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]"
+  - type: textarea
+    id: solution
+    validations:
+      required: true
+    attributes:
+      label: "What solution do you propose?"
+      description: "A clear and concise description of what you want to happen."
+  - type: textarea
+    id: alternatives
+    validations:
+      required: true
+    attributes:
+      label: "What alternatives have you considered?"
+      description: "A clear and concise description of any alternative solutions or features you've considered."
+  - type: textarea
+    id: context
+    validations:
+      required: false
+    attributes:
+      label: "Any additional context?"
+      description: "Add any other context or screenshots about the feature request here."
+  - type: markdown
+    attributes:
+      value: |
+        If you want to contribute to this project, we will be happy to guide you through the contribution process especially when you already have a good proposal or understanding of how to improve the functionality. Join us at https://slack.cncf.io/ and choose #notary-project channel.

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,11 @@
+**What this PR does / why we need it**:
+
+**Which issue(s) this PR resolves** *(optional, in `resolves #<issue number>(, resolves #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
+Resolves #
+
+**Please check the following list**:
+- [ ]  Does the affected code have corresponding tests, e.g. unit test?
+- [ ]  Does this introduce breaking changes that would require an announcement or bumping the major version?
+- [ ]  Do all new files have an appropriate license header?
+
+<!-- If this is a security issue, please do not discuss on GitHub. Please report any suspected or confirmed security issues directly to https://github.com/notaryproject/tspclient-go/blob/main/CODEOWNERS. -->

CODEOWNERS

@@ -1,3 +1,3 @@
 # Repo-Level Owners (in alphabetical order)
 # Note: This is only for the notaryproject/tspclient-go repo
-* @gokarnm @JeyJeyGao @niazfk @priteshbandi @shizhMSFT @toddysm @Two-Hearts @vaninrao10 @yizha1
+* @gokarnm @niazfk @priteshbandi @shizhMSFT @toddysm @Two-Hearts @vaninrao10 @yizha1

MAINTAINERS

@@ -10,10 +10,12 @@ Yi Zha <yizha1@microsoft.com> (@yizha1)
 # Repo-Level Maintainers (in alphabetical order)
 # Pattern: [First Name] [Last Name] <[Email Address]> ([GitHub Handle])
 # Note: This is for the notaryproject/tspclient-go repo
-Junjie Gao <junjiegao@microsoft.com> (@JeyJeyGao)
 Patrick Zheng <patrickzheng@microsoft.com> (@Two-Hearts)
 Shiwei Zhang <shizh@microsoft.com> (@shizhMSFT)
 
 # Emeritus Org Maintainers (in alphabetical order)
 Justin Cormack <justin.cormack@docker.com> (@justincormack)
-Steve Lasker <StevenLasker@hotmail.com> (@stevelasker)
+Steve Lasker <StevenLasker@hotmail.com> (@stevelasker)
+
+# Emeritus Repo-Level Maintainers (in alphabetical order)
+Junjie Gao <junjiegao@microsoft.com> (@JeyJeyGao)

conformance_test.go

@@ -73,14 +73,9 @@ func TestTSATimestampGranted(t *testing.T) {
 
 	// do timestamp
 	message := []byte("notation")
-	nonce, err := generateNonce()
-	if err != nil {
-		t.Fatal("failed to create nonce:", err)
-	}
 	requestOpts := RequestOptions{
 		Content:       message,
 		HashAlgorithm: crypto.SHA256,
-		Nonce:         nonce,
 	}
 	req, err := NewRequest(requestOpts)
 	if err != nil {

go.mod

@@ -1,3 +1,3 @@
 module github.com/notaryproject/tspclient-go
 
-go 1.21
+go 1.23.0

http.go

@@ -53,9 +53,16 @@ func NewHTTPTimestamper(httpClient *http.Client, endpoint string) (Timestamper,
 	if httpClient == nil {
 		httpClient = &http.Client{Timeout: 5 * time.Second}
 	}
-	if _, err := url.Parse(endpoint); err != nil {
+	tsaURL, err := url.Parse(endpoint)
+	if err != nil {
 		return nil, err
 	}
+	if tsaURL.Scheme != "http" && tsaURL.Scheme != "https" {
+		return nil, fmt.Errorf("endpoint %q: scheme must be http or https, but got %q", endpoint, tsaURL.Scheme)
+	}
+	if tsaURL.Host == "" {
+		return nil, fmt.Errorf("endpoint %q: host cannot be empty", endpoint)
+	}
 	return &httpTimestamper{
 		httpClient: httpClient,
 		endpoint:   endpoint,

http_test.go

@@ -67,12 +67,12 @@ func TestHTTPTimestampGranted(t *testing.T) {
 			Tag:       5,
 			FullBytes: []byte{5, 0},
 		},
-		NoNonce: true,
 	}
 	req, err := NewRequest(requestOpts)
 	if err != nil {
 		t.Fatalf("NewRequest() error = %v", err)
 	}
+	req.Nonce = nil
 	ctx := context.Background()
 	resp, err := tsa.Timestamp(ctx, req)
 	if err != nil {
@@ -256,6 +256,24 @@ func TestNewHTTPTimestamper(t *testing.T) {
 	if _, err := NewHTTPTimestamper(nil, malformedURL); err == nil || err.Error() != expectedErrMsg {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
+
+	malformedURL = "invalid"
+	expectedErrMsg = `endpoint "invalid": scheme must be http or https, but got ""`
+	if _, err := NewHTTPTimestamper(nil, malformedURL); err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
+	}
+
+	malformedURL = "invalid://"
+	expectedErrMsg = `endpoint "invalid://": scheme must be http or https, but got "invalid"`
+	if _, err := NewHTTPTimestamper(nil, malformedURL); err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
+	}
+
+	malformedURL = "https://"
+	expectedErrMsg = `endpoint "https://": host cannot be empty`
+	if _, err := NewHTTPTimestamper(nil, malformedURL); err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
+	}
 }
 
 func TestHttpTimestamperTimestamp(t *testing.T) {

internal/cms/signed.go

@@ -248,14 +248,19 @@ func (d *ParsedSignedData) verifySignedAttributes(signerInfo *SignerInfo, chains
 
 	// verify attributes if present
 	if len(signerInfo.SignedAttributes) == 0 {
-		if d.ContentType.Equal(oid.Data) {
-			return nil, nil
-		}
-		// signed attributes MUST be present if the content type of the
-		// EncapsulatedContentInfo value being signed is not id-data.
+		// According to RFC 5652, if the Content Type is id-data, signed
+		// attributes can be empty. However, this cms package is designed for
+		// timestamp (RFC 3161) and the content type must be id-ct-TSTInfo,
+		// so we require signed attributes to be present.
 		return nil, VerificationError{Message: "missing signed attributes"}
 	}
 
+	// this CMS package is designed for timestamping (RFC 3161), so checking the
+	// content type to be id-ct-TSTInfo is an optimization for tspclient to
+	// fail fast.
+	if !oid.TSTInfo.Equal(d.ContentType) {
+		return nil, fmt.Errorf("unexpected content type: %v. Expected to be id-ct-TSTInfo (%v)", d.ContentType, oid.TSTInfo)
+	}
 	var contentType asn1.ObjectIdentifier
 	if err := signerInfo.SignedAttributes.Get(oid.ContentType, &contentType); err != nil {
 		return nil, VerificationError{Message: "invalid content type", Detail: err}

internal/cms/signed_test.go

@@ -160,7 +160,12 @@ func TestVerify(t *testing.T) {
 		{
 			name:     "id-data content type without signed attributes",
 			filePath: "testdata/SignedDataWithoutSignedAttributes.p7s",
-			wantErr:  false,
+			wantErr:  true,
+		},
+		{
+			name:     "invalid content type",
+			filePath: "testdata/TimeStampTokenWithInvalidContentType.p7s",
+			wantErr:  true,
 		},
 		{
 			name:     "an invalid and a valid signer info",

internal/encoding/asn1/ber/ber_test.go

@@ -19,7 +19,7 @@ import (
 )
 
 func TestConvertToDER(t *testing.T) {
-	var testBytes = make([]byte, 0xFFFFFFFF+8)
+	var testBytes = make([]byte, 0x7FFFFFFF)
 	// primitive identifier
 	testBytes[0] = 0x1f
 	testBytes[1] = 0xa0

request.go

@@ -94,18 +94,6 @@ type RequestOptions struct {
 	// Reference: https://datatracker.ietf.org/doc/html/rfc3161#section-2.4.1
 	ReqPolicy asn1.ObjectIdentifier
 
-	// NoNonce disables any Nonce usage. When set to true, the Nonce field is
-	// ignored, and no built-in Nonce will be generated. OPTIONAL.
-	NoNonce bool
-
-	// Nonce is a large random number with a high probability that the client
-	// generates it only once. The same nonce is included and validated in the
-	// response. It is only used when NoNonce is not set to true.
-	//
-	// When this field is nil, a built-in Nonce will be generated and sent to
-	// the TSA. OPTIONAL.
-	Nonce *big.Int
-
 	// NoCert tells the TSA to not include any signing certificate in its
 	// response. By default, TSA signing certificate is included in the response.
 	// OPTIONAL.
@@ -133,17 +121,9 @@ func NewRequest(opts RequestOptions) (*Request, error) {
 	if tspclientasn1.EqualRawValue(hashAlgParameter, asn1.RawValue{}) || tspclientasn1.EqualRawValue(hashAlgParameter, asn1.NullRawValue) {
 		hashAlgParameter = asn1NullRawValue
 	}
-	var nonce *big.Int
-	if !opts.NoNonce {
-		if opts.Nonce != nil { // user provided Nonce, use it
-			nonce = opts.Nonce
-		} else { // user ignored Nonce, use built-in Nonce
-			var err error
-			nonce, err = generateNonce()
-			if err != nil {
-				return nil, &MalformedRequestError{Msg: err.Error()}
-			}
-		}
+	nonce, err := generateNonce()
+	if err != nil {
+		return nil, &MalformedRequestError{Msg: err.Error()}
 	}
 	return &Request{
 		Version: 1,
@@ -206,7 +186,7 @@ func generateNonce() (*big.Int, error) {
 	// Pick a random number from 0 to 2^159
 	nonce, err := rand.Int(rand.Reader, (&big.Int{}).Lsh(big.NewInt(1), 159))
 	if err != nil {
-		return nil, errors.New("error generating nonce")
+		return nil, errors.New("failed to generate nonce")
 	}
 	return nonce, nil
 }

request_test.go

@@ -15,6 +15,7 @@ package tspclient
 
 import (
 	"crypto"
+	"crypto/rand"
 	"crypto/x509/pkix"
 	"encoding/asn1"
 	"errors"
@@ -71,6 +72,21 @@ func TestNewRequest(t *testing.T) {
 	if !reflect.DeepEqual(req.MessageImprint.HashAlgorithm.Parameters, asn1NullRawValue) {
 		t.Fatalf("expected %v, but got %v", asn1NullRawValue, req.MessageImprint.HashAlgorithm.Parameters)
 	}
+
+	defaultRandReader := rand.Reader
+	rand.Reader = &dummyRandReader{}
+	defer func() {
+		rand.Reader = defaultRandReader
+	}()
+	opts = RequestOptions{
+		Content:       message,
+		HashAlgorithm: crypto.SHA256,
+	}
+	expectedErrMsg = "malformed timestamping request: failed to generate nonce"
+	_, err = NewRequest(opts)
+	if err == nil || !errors.As(err, &malformedRequest) || err.Error() != expectedErrMsg {
+		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
+	}
 }
 
 func TestRequestMarshalBinary(t *testing.T) {
@@ -155,3 +171,9 @@ func TestValidateRequest(t *testing.T) {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)
 	}
 }
+
+type dummyRandReader struct{}
+
+func (r *dummyRandReader) Read(b []byte) (int, error) {
+	return 0, errors.New("failed to read")
+}

timestamp.go

@@ -13,7 +13,10 @@
 
 package tspclient
 
-import "time"
+import (
+	"fmt"
+	"time"
+)
 
 // Timestamp denotes the time at which the timestamp token was created by the TSA
 //
@@ -43,3 +46,11 @@ func (t *Timestamp) BoundedAfter(u time.Time) bool {
 	timestampLowerLimit := t.Value.Add(-t.Accuracy)
 	return timestampLowerLimit.After(u) || timestampLowerLimit.Equal(u)
 }
+
+// Format returns a string of t in format layout.
+// The output is a timestamp range calculated with its accuracy.
+func (t *Timestamp) Format(layout string) string {
+	lowerBound := t.Value.Add(-t.Accuracy)
+	upperBound := t.Value.Add(t.Accuracy)
+	return fmt.Sprintf("[%s, %s]", lowerBound.Format(layout), upperBound.Format(layout))
+}

timestamp_test.go

@@ -83,3 +83,18 @@ func TestTimestamp(t *testing.T) {
 		t.Fatal("timestamp.BoundedAfter expected false, but got true")
 	}
 }
+
+func TestString(t *testing.T) {
+	// timestamp range:
+	// [time.Date(2021, time.September, 17, 14, 9, 8, 0, time.UTC),
+	//  time.Date(2021, time.September, 17, 14, 9, 12, 0, time.UTC)]
+	timestamp := Timestamp{
+		Value:    time.Date(2021, time.September, 17, 14, 9, 10, 0, time.UTC),
+		Accuracy: 2 * time.Second,
+	}
+
+	expectedStr := "[2021-09-17T14:09:08Z, 2021-09-17T14:09:12Z]"
+	if timestamp.Format(time.RFC3339) != expectedStr {
+		t.Fatalf("expected %s, but got %s", expectedStr, timestamp.Format(time.RFC3339))
+	}
+}

token.go

@@ -45,7 +45,7 @@ func ParseSignedToken(berData []byte) (*SignedToken, error) {
 		return nil, err
 	}
 	if !oid.TSTInfo.Equal(signed.ContentType) {
-		return nil, fmt.Errorf("unexpected content type: %v", signed.ContentType)
+		return nil, fmt.Errorf("unexpected content type: %v. Expected to be id-ct-TSTInfo (%v)", signed.ContentType, oid.TSTInfo)
 	}
 	return (*SignedToken)(signed), nil
 }

token_test.go

@@ -30,7 +30,7 @@ func TestParseSignedToken(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	expectedErrMsg := fmt.Sprintf("unexpected content type: %v", oid.Data)
+	expectedErrMsg := fmt.Sprintf("unexpected content type: %v. Expected to be id-ct-TSTInfo (1.2.840.113549.1.9.16.1.4)", oid.Data)
 	_, err = ParseSignedToken(timestampToken)
 	if err == nil || err.Error() != expectedErrMsg {
 		t.Fatalf("expected error %s, but got %v", expectedErrMsg, err)