Fix hashed message verification (#118)

* fix: compare hashed messages

Signed-off-by: Hector Fernandez <[email protected]>

* test: validate verify hashed messages

Signed-off-by: Hector Fernandez <[email protected]>

* refactor verify function and test

Signed-off-by: Hector Fernandez <[email protected]>

* Update cmd/timestamp-cli/app/verify.go

Co-authored-by: Bob Callaway <[email protected]>
Signed-off-by: Hector Fernandez <[email protected]>

Signed-off-by: Hector Fernandez <[email protected]>
Co-authored-by: Bob Callaway <[email protected]>

cmd/timestamp-cli/app/verify.go

@@ -20,6 +20,7 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
+	"hash"
 	"io"
 	"os"
 	"path/filepath"
@@ -125,27 +126,22 @@ func verifyTSRWithPEM(ts *timestamp.Timestamp) error {
 
 func verifyArtifactWithTSR(ts *timestamp.Timestamp) error {
 	artifactPath := viper.GetString("artifact")
-	artifactBytes, err := os.ReadFile(filepath.Clean(artifactPath))
+	artifact, err := os.Open(filepath.Clean(artifactPath))
 	if err != nil {
 		return err
 	}
 
-	h := ts.HashAlgorithm.New()
-	b := make([]byte, h.Size())
-
-	r := bytes.NewReader(artifactBytes)
-	n, err := r.Read(b)
-	if err == io.EOF {
-		return err
-	}
+	return verifyHashedMessages(ts.HashAlgorithm.New(), ts.HashedMessage, artifact)
+}
 
-	_, err = h.Write(b[:n])
-	if err != nil {
-		return fmt.Errorf("Failed to create local message hash")
+func verifyHashedMessages(hashAlg hash.Hash, hashedMessage []byte, artifactReader io.Reader) error {
+	h := hashAlg
+	if _, err := io.Copy(h, artifactReader); err != nil {
+		return fmt.Errorf("failed to create hash %w", err)
 	}
+	localHashedMsg := h.Sum(nil)
 
-	localHashedMessage := h.Sum(nil)
-	if !bytes.Equal(localHashedMessage, ts.HashedMessage) {
+	if !bytes.Equal(localHashedMsg, hashedMessage) {
 		return fmt.Errorf("Hashed messages don't match")
 	}
 

cmd/timestamp-cli/app/verify_test.go

@@ -0,0 +1,96 @@
+//
+// Copyright 2022 The Sigstore 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.
+
+package app
+
+import (
+	"bytes"
+	"crypto"
+	"io"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/digitorus/timestamp"
+	"github.com/sigstore/timestamp-authority/pkg/client"
+	tsatimestamp "github.com/sigstore/timestamp-authority/pkg/generated/client/timestamp"
+	"github.com/sigstore/timestamp-authority/pkg/server"
+	"github.com/spf13/viper"
+)
+
+func TestVerifyArtifactHashedMessages(t *testing.T) {
+	viper.Set("timestamp-signer", "memory")
+	apiServer := server.NewRestAPIServer("localhost", 0, []string{"http"}, 10*time.Second, 10*time.Second)
+	server := httptest.NewServer(apiServer.GetHandler())
+	t.Cleanup(server.Close)
+
+	c, err := client.GetTimestampClient(server.URL)
+	if err != nil {
+		t.Fatalf("unexpected error creating client: %v", err)
+	}
+
+	type test struct {
+		message string
+	}
+
+	tests := []test{
+		{
+			message: "valid local artifact",
+		},
+		{
+			message: "nonexistant local artifact",
+		},
+		{
+			message: "valid local artifact with hash algorithm",
+		},
+		{
+			message: "valid oid",
+		},
+		{
+			message: "MIIEbjADAgEAMIIEZQYJKoZIhvcNAQcCoIIEVjCCBFICAQExDTALBglghkgBZQMEAgEwgdQGCyqGSIb3DQEJEAEEoIHEBIHBMIG+AgEBBgkrBgEEAYO/MAIwMTANBglghkgBZQMEAgEFAAQgN94hMnpq0onyUi7r1zJHNiLT1/spX8MU2GBN9AdMe6wCFQDS6RL1iVlmlkwJzmpS2EH0cuX8sxgTMjAyMjExMDMxNzQyNDIrMDEwMDADAgEBAhRKnQszZjzcgJkpE8LCbmbF0s1jPaA0pDIwMDEOMAwGA1UEChMFbG9jYWwxHjAcBgNVBAMTFVRlc3QgVFNBIFRpbWVzdGFtcGluZ6CCAckwggHFMIIBaqADAgECAhRHCu9dHKS97mFo1cH5neJubRibujAKBggqhkjOPQQDAjAoMQ4wDAYDVQQKEwVsb2NhbDEWMBQGA1UEAxMNVGVzdCBUU0EgUm9vdDAeFw0yMjExMDMxMTUzMThaFw0zMTExMDMxMTU2MThaMDAxDjAMBgNVBAoTBWxvY2FsMR4wHAYDVQQDExVUZXN0IFRTQSBUaW1lc3RhbXBpbmcwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATo3W6NQrpx5D8z5IvgD2DlAgoJMF4KPY9Pj4UfFhfOq029ryszXp3460Z7N+x86bDvyjVrHaeiPnl1HO9Q52zso2owaDAOBgNVHQ8BAf8EBAMCB4AwHQYDVR0OBBYEFHSIhDdTGIsodML/iUOhx7hgo/K7MB8GA1UdIwQYMBaAFBoZYijuouZCvKDtBd0eCyaU2HWoMBYGA1UdJQEB/wQMMAoGCCsGAQUFBwMIMAoGCCqGSM49BAMCA0kAMEYCIQCmPVr5kwYe4Jg9PGO6apgfzSrKAtESgNHpAbE3iIvJhQIhAJIGNxshJcC8LXHRrVWM77no3d3GguSvR01OAPZwE2pqMYIBmDCCAZQCAQEwQDAoMQ4wDAYDVQQKEwVsb2NhbDEWMBQGA1UEAxMNVGVzdCBUU0EgUm9vdAIURwrvXRykve5haNXB+Z3ibm0Ym7owCwYJYIZIAWUDBAIBoIHqMBoGCSqGSIb3DQEJAzENBgsqhkiG9w0BCRABBDAcBgkqhkiG9w0BCQUxDxcNMjIxMTAzMTY0MjQyWjAvBgkqhkiG9w0BCQQxIgQgrKbkOizzGoAudPhAnW5Qny788Kcd++VQwPrCMhg4MTEwfQYLKoZIhvcNAQkQAi8xbjBsMGowaAQgXqxJD0nAgg6en9P1bRrU7+6tzxOMn3YThreg7uR6T7EwRDAspCowKDEOMAwGA1UEChMFbG9jYWwxFjAUBgNVBAMTDVRlc3QgVFNBIFJvb3QCFEcK710cpL3uYWjVwfmd4m5tGJu6MAoGCCqGSM49BAMCBEcwRQIgQkc2BxMjnUMzqBDYzUiw10LoCIZ9Zmp1E0Hl6E+9mzwCIQDp2lD826Du5Ss4pNG/TksDknTUJfKvrLc2ex+x+W3VHg==",
+		},
+	}
+
+	for _, tc := range tests {
+		tsq, err := timestamp.CreateRequest(strings.NewReader(tc.message), &timestamp.RequestOptions{
+			Hash:         crypto.SHA256,
+			Certificates: true,
+		})
+		if err != nil {
+			t.Fatalf("unexpected error creating request: %v", err)
+		}
+
+		params := tsatimestamp.NewGetTimestampResponseParams()
+		params.SetTimeout(10 * time.Second)
+		params.Request = io.NopCloser(bytes.NewReader(tsq))
+
+		var respBytes bytes.Buffer
+		_, err = c.Timestamp.GetTimestampResponse(params, &respBytes)
+		if err != nil {
+			t.Fatalf("unexpected error getting timestamp response: %v", err)
+		}
+
+		tsr, err := timestamp.ParseResponse(respBytes.Bytes())
+		if err != nil {
+			t.Fatalf("unexpected error parsing response: %v", err)
+		}
+
+		if err := verifyHashedMessages(tsr.HashAlgorithm.New(), tsr.HashedMessage, strings.NewReader(tc.message)); err != nil {
+			t.Errorf("verifyHashedMessages failed comparing hashes: %v", err)
+		}
+
+	}
+}

pkg/tests/api_test.go

@@ -81,7 +81,7 @@ func TestGetTimestampResponse(t *testing.T) {
 
 	// create request with nonce and certificate, the typical request structure
 	tsNonce := big.NewInt(1234)
-	tsq, err := ts.CreateRequest(strings.NewReader("blob"), &ts.RequestOptions{
+	tsq, err := ts.CreateRequest(strings.NewReader("blobblobblobblobblobblobblobblobblob"), &ts.RequestOptions{
 		Hash:         crypto.SHA256,
 		Certificates: true,
 		Nonce:        tsNonce,
@@ -120,7 +120,7 @@ func TestGetTimestampResponse(t *testing.T) {
 	if tsr.HashAlgorithm != crypto.SHA256 {
 		t.Fatalf("unexpected hash algorithm")
 	}
-	hashedMessage := sha256.Sum256([]byte("blob"))
+	hashedMessage := sha256.Sum256([]byte("blobblobblobblobblobblobblobblobblob"))
 	if !bytes.Equal(tsr.HashedMessage, hashedMessage[:]) {
 		t.Fatalf("expected hashed messages to be equal: %v %v", tsr.HashedMessage, hashedMessage)
 	}