From 8b28f05b3247b325f65b20872e9c37bb9f988256 Mon Sep 17 00:00:00 2001
From: Bob Callaway <bobcallaway@users.noreply.github.com>
Date: Fri, 19 Mar 2021 08:31:03 -0400
Subject: [PATCH] Remove gzip processing flow completely from rekor (#221)

* Remove gzip processing flow completely from rekor

Issue #208 reported different handling of gzipped content via fetch vs
direct upload to rekor server. The code should be consistent, regardless
of whether content was compressed or not - by always attempting to
verify the signature against the (unmodified) byte stream.

This patch removes the gzip decoding completely from rekor and verifies
the bytes supplied or referenced.

Also fixes issue in E2E tests where sending SIGKILL to watch process
caused message to be printed to stderr, which fails the test when
running on MacOS.

Fixes #208

Signed-off-by: Bob Callaway <bcallawa@redhat.com>
---
 pkg/api/index.go                 |  2 +-
 pkg/types/rekord/v0.0.1/entry.go |  6 +++---
 pkg/types/rpm/v0.0.1/entry.go    |  4 ++--
 pkg/util/fetch.go                | 21 ++-------------------
 tests/e2e-test.sh                |  6 +++---
 tests/e2e_test.go                |  6 ++++--
 6 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/pkg/api/index.go b/pkg/api/index.go
index 87e2993..0b5fdb0 100644
--- a/pkg/api/index.go
+++ b/pkg/api/index.go
@@ -53,7 +53,7 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder {
 	}
 	if params.Query.PublicKey != nil {
 		af := pki.NewArtifactFactory(swag.StringValue(params.Query.PublicKey.Format))
-		keyReader, err := util.FileOrURLReadCloser(httpReqCtx, params.Query.PublicKey.URL.String(), params.Query.PublicKey.Content, true)
+		keyReader, err := util.FileOrURLReadCloser(httpReqCtx, params.Query.PublicKey.URL.String(), params.Query.PublicKey.Content)
 		if err != nil {
 			return handleRekorAPIError(params, http.StatusBadRequest, err, malformedPublicKey)
 		}
diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go
index 1957829..277a331 100644
--- a/pkg/types/rekord/v0.0.1/entry.go
+++ b/pkg/types/rekord/v0.0.1/entry.go
@@ -195,7 +195,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		defer hashW.Close()
 		defer sigW.Close()
 
-		dataReadCloser, err := util.FileOrURLReadCloser(ctx, v.RekordObj.Data.URL.String(), v.RekordObj.Data.Content, false)
+		dataReadCloser, err := util.FileOrURLReadCloser(ctx, v.RekordObj.Data.URL.String(), v.RekordObj.Data.Content)
 		if err != nil {
 			return closePipesOnError(err)
 		}
@@ -237,7 +237,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		defer close(sigResult)
 
 		sigReadCloser, err := util.FileOrURLReadCloser(ctx, v.RekordObj.Signature.URL.String(),
-			v.RekordObj.Signature.Content, false)
+			v.RekordObj.Signature.Content)
 		if err != nil {
 			return closePipesOnError(err)
 		}
@@ -262,7 +262,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		defer close(keyResult)
 
 		keyReadCloser, err := util.FileOrURLReadCloser(ctx, v.RekordObj.Signature.PublicKey.URL.String(),
-			v.RekordObj.Signature.PublicKey.Content, false)
+			v.RekordObj.Signature.PublicKey.Content)
 		if err != nil {
 			return closePipesOnError(err)
 		}
diff --git a/pkg/types/rpm/v0.0.1/entry.go b/pkg/types/rpm/v0.0.1/entry.go
index 88ded53..62024dc 100644
--- a/pkg/types/rpm/v0.0.1/entry.go
+++ b/pkg/types/rpm/v0.0.1/entry.go
@@ -198,7 +198,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		defer sigW.Close()
 		defer rpmW.Close()
 
-		dataReadCloser, err := util.FileOrURLReadCloser(ctx, v.RPMModel.Package.URL.String(), v.RPMModel.Package.Content, true)
+		dataReadCloser, err := util.FileOrURLReadCloser(ctx, v.RPMModel.Package.URL.String(), v.RPMModel.Package.Content)
 		if err != nil {
 			return closePipesOnError(err)
 		}
@@ -236,7 +236,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 
 	g.Go(func() error {
 		keyReadCloser, err := util.FileOrURLReadCloser(ctx, v.RPMModel.PublicKey.URL.String(),
-			v.RPMModel.PublicKey.Content, false)
+			v.RPMModel.PublicKey.Content)
 		if err != nil {
 			return closePipesOnError(err)
 		}
diff --git a/pkg/util/fetch.go b/pkg/util/fetch.go
index 5b9a00f..b1862a6 100644
--- a/pkg/util/fetch.go
+++ b/pkg/util/fetch.go
@@ -16,9 +16,7 @@ limitations under the License.
 package util
 
 import (
-	"bufio"
 	"bytes"
-	"compress/gzip"
 	"context"
 	"fmt"
 	"io"
@@ -27,7 +25,7 @@ import (
 )
 
 // FileOrURLReadCloser Note: caller is responsible for closing ReadCloser returned from method!
-func FileOrURLReadCloser(ctx context.Context, url string, content []byte, checkGZIP bool) (io.ReadCloser, error) {
+func FileOrURLReadCloser(ctx context.Context, url string, content []byte) (io.ReadCloser, error) {
 	var dataReader io.ReadCloser
 	if url != "" {
 		//TODO: set timeout here, SSL settings?
@@ -44,22 +42,7 @@ func FileOrURLReadCloser(ctx context.Context, url string, content []byte, checkG
 			return nil, fmt.Errorf("error received while fetching artifact: %v", resp.Status)
 		}
 
-		if checkGZIP {
-			// read first 512 bytes to determine if content is gzip compressed
-			bufReader := bufio.NewReaderSize(resp.Body, 512)
-			ctBuf, err := bufReader.Peek(512)
-			if err != nil && err != bufio.ErrBufferFull && err != io.EOF {
-				return nil, err
-			}
-
-			if http.DetectContentType(ctBuf) == "application/x-gzip" {
-				dataReader, _ = gzip.NewReader(io.MultiReader(bufReader, resp.Body))
-			} else {
-				dataReader = ioutil.NopCloser(io.MultiReader(bufReader, resp.Body))
-			}
-		} else {
-			dataReader = resp.Body
-		}
+		dataReader = resp.Body
 	} else {
 		dataReader = ioutil.NopCloser(bytes.NewReader(content))
 	}
diff --git a/tests/e2e-test.sh b/tests/e2e-test.sh
index 389bdee..773281a 100755
--- a/tests/e2e-test.sh
+++ b/tests/e2e-test.sh
@@ -1,5 +1,5 @@
 #!/bin/bash
-#set -ex
+set -e
 testdir=$(dirname "$0")
 
 echo "starting services"
@@ -26,7 +26,7 @@ done
 
 echo
 echo "running tests"
-TMPDIR="$(mktemp -d -t rekor_test)"
+TMPDIR="$(mktemp -d -t rekor_test.XXXXXX)"
 touch $TMPDIR.rekor.yaml
 trap "rm -rf $TMPDIR" EXIT
-TMPDIR=$TMPDIR go test -tags=e2e ./tests/
\ No newline at end of file
+TMPDIR=$TMPDIR go test -tags=e2e ./tests/
diff --git a/tests/e2e_test.go b/tests/e2e_test.go
index 54d8814..e15ccfd 100644
--- a/tests/e2e_test.go
+++ b/tests/e2e_test.go
@@ -303,8 +303,10 @@ func TestWatch(t *testing.T) {
 	go func() {
 		b, err := cmd.CombinedOutput()
 		t.Log(string(b))
-		if err != nil {
-			t.Fatal(err)
+		if cmd.ProcessState.Exited() && cmd.ProcessState.ExitCode() != 0 {
+			if err != nil {
+				t.Fatal(err)
+			}
 		}
 	}()
 
-- 
GitLab