Merge branch 'notaryproject:main' into nonce

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

@@ -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",

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)