ensure fallback logic executes if attestation key is empty when fetching attestation

.github/workflows/main.yml

@@ -112,3 +112,25 @@ jobs:
         with:
           name: Sharding E2E Docker Compose logs
           path: /tmp/docker-compose.log
+
+  issue-872-e2e:
+    runs-on: ubuntu-20.04
+    needs: build
+
+    steps:
+      - uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2
+      - name: Docker Build
+        run: docker-compose build
+      - name: Extract version of Go to use
+        run: echo "GOVERSION=$(cat Dockerfile|grep golang | awk ' { print $2 } ' | cut -d '@' -f 1 | cut -d ':' -f 2 | uniq)" >> $GITHUB_ENV
+      - uses: actions/setup-go@b22fbbc2921299758641fab08929b4ac52b32923 # v3.1.0
+        with:
+          go-version: ${{ env.GOVERSION }}
+      - name: Test for Attestation begin returned that was previously persisted in tlog
+        run: ./tests/issue-872-e2e-test.sh
+      - name: Upload logs if they exist
+        uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v3
+        if: failure()
+        with:
+          name: Docker Compose logs
+          path: /tmp/*docker-compose.log

pkg/api/entries.go

@@ -106,27 +106,32 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC
 		if err != nil {
 			return nil, err
 		}
+
+		var att []byte
+		var fetchErr error
 		attKey := eimpl.AttestationKey()
+		// if we're given a key by the type logic, let's try that first
 		if attKey != "" {
-			att, err := storageClient.FetchAttestation(ctx, attKey)
+			att, fetchErr = storageClient.FetchAttestation(ctx, attKey)
+			if fetchErr != nil {
+				log.Logger.Errorf("error fetching attestation by key, trying by UUID: %s %w", attKey, fetchErr)
+			}
+		}
+		// if looking up by key failed or we weren't able to generate a key, try looking up by uuid
+		if attKey == "" || fetchErr != nil {
+			activeTree := fmt.Sprintf("%x", tc.logID)
+			entryIDstruct, err := sharding.CreateEntryIDFromParts(activeTree, uuid)
 			if err != nil {
-				log.Logger.Errorf("error fetching attestation by key, trying by UUID: %s %s", attKey, err)
-				// the original attestation implementation stored this by uuid instead of by digest
-				activeTree := fmt.Sprintf("%x", tc.logID)
-				entryIDstruct, err := sharding.CreateEntryIDFromParts(activeTree, uuid)
-				if err != nil {
-					err := fmt.Errorf("error creating EntryID from active treeID %v and uuid %v: %w", activeTree, uuid, err)
-					return nil, err
-				}
-				att, err = storageClient.FetchAttestation(ctx, entryIDstruct.UUID)
-				if err != nil {
-					log.Logger.Errorf("error fetching attestation by uuid: %s %s", entryIDstruct.UUID, err)
-				}
+				return nil, fmt.Errorf("error creating EntryID from active treeID %v and uuid %v: %w", activeTree, uuid, err)
 			}
-			if err == nil {
-				logEntryAnon.Attestation = &models.LogEntryAnonAttestation{
-					Data: att,
-				}
+			att, fetchErr = storageClient.FetchAttestation(ctx, entryIDstruct.UUID)
+			if fetchErr != nil {
+				log.Logger.Errorf("error fetching attestation by uuid: %s %v", entryIDstruct.UUID, fetchErr)
+			}
+		}
+		if fetchErr == nil {
+			logEntryAnon.Attestation = &models.LogEntryAnonAttestation{
+				Data: att,
 			}
 		}
 	}

pkg/storage/storage.go

@@ -18,6 +18,7 @@ package storage
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/sigstore/rekor/pkg/log"
 
@@ -72,7 +73,7 @@ func (b *Blob) FetchAttestation(ctx context.Context, key string) ([]byte, error)
 		return nil, err
 	}
 	if !exists {
-		return nil, nil
+		return nil, fmt.Errorf("attestation %v does not exist", key)
 	}
 
 	data, err := b.bucket.ReadAll(ctx, key)

tests/intoto_dsse.json

@@ -0,0 +1 @@
+{"payloadType":"application/vnd.in-toto+json","payload":"eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjAuMSIsInByZWRpY2F0ZVR5cGUiOiJodHRwczovL3Nsc2EuZGV2L3Byb3ZlbmFuY2UvdjAuMiIsInN1YmplY3QiOlt7Im5hbWUiOiJmb29iYXIiLCJkaWdlc3QiOnsiZm9vIjoiYmFyIn19XSwicHJlZGljYXRlIjp7ImJ1aWxkZXIiOnsiaWQiOiJmb29ISzFiZ2Y1WC8xckNxZz09In0sImJ1aWxkVHlwZSI6IiIsImludm9jYXRpb24iOnsiY29uZmlnU291cmNlIjp7fX19fQ==","signatures":[{"keyid":"","sig":"MEQCIAIlnxHC3eU4jmUsqJExxfzqyy8bk+61btgnRiGcRDxgAiBwmdnJ/GX1yCYhYAvwAtkuYN0yFlVPQVAx9R6JpUUBiA=="}]}

tests/intoto_dsse.pem

@@ -0,0 +1,4 @@
+-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEx+ikqUxXurlxZltajRBV2ju31j32
+baT2ax2dXBcpInWaFESqGF35KISflP1EmMvEnfG+AzHecQ0WQp5QzNId+w==
+-----END PUBLIC KEY-----

tests/issue-872-e2e-test.sh

@@ -0,0 +1,201 @@
+#!/bin/bash
+#
+# 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.
+
+set -e
+testdir=$(dirname "$0")
+
+echo "* starting services"
+docker-compose up -d
+
+echo "* building CLI"
+go build -o rekor-cli ./cmd/rekor-cli
+REKOR_CLI=$(pwd)/rekor-cli
+
+function waitForRekorServer () {
+  echo -n "* waiting up to 60 sec for system to start"
+  count=0
+
+  until [ $(docker ps -a | grep -c "(healthy)") == 3 ];
+  do
+      if [ $count -eq 6 ]; then
+        echo "! timeout reached"
+        exit 1
+      else
+        echo -n "."
+        sleep 10
+        let 'count+=1'
+      fi
+  done
+
+  echo
+}
+
+REKORTMPDIR="$(mktemp -d -t rekor_test.XXXXXX)"
+touch $REKORTMPDIR.rekor.yaml
+trap "rm -rf $REKORTMPDIR" EXIT
+
+waitForRekorServer
+
+echo "* stopping rekor to test issue #872"
+docker-compose stop rekor-server
+
+docker volume rm -f issue872_attestations || true
+ATT_VOLUME=$(docker volume create --name issue872_attestations)
+# set permissions on docker volume to be friendly to non-root since v0.6.0 container is based on distroless
+docker run --rm -v $ATT_VOLUME:/att:z busybox /bin/sh -c 'touch /att/.initialized && chown -R 65532:65532 /att && chmod 777 /att'
+
+V060_COMPOSE_FILE=$REKORTMPDIR/docker-compose-issue872-v060.yaml
+cat << EOF > $V060_COMPOSE_FILE
+version: '3.4'
+services:
+  rekor-server-issue-872-v060:
+    # this container image is built on v0.6.0 with the fix for issue #800
+    image: gcr.io/projectsigstore/rekor/ci/rekor/rekor-server@sha256:568aee99574e6d796d70b7b1fd59438bd54b3b9f44cc2c9a086629597c66d324
+    user: "65532:65532"
+    command: [
+      "serve",
+      "--trillian_log_server.address=trillian-log-server",
+      "--trillian_log_server.port=8090",
+      "--redis_server.address=redis-server",
+      "--redis_server.port=6379",
+      "--rekor_server.address=0.0.0.0",
+      "--rekor_server.signer=memory",
+      "--enable_attestation_storage",
+      "--attestation_storage_bucket=file:///ko-app/attestations",
+      # Uncomment this for production logging
+      # "--log_type=prod",
+      ]
+    volumes:
+    - "$ATT_VOLUME:/ko-app/attestations:z"
+    restart: always # keep the server running
+    ports:
+      - "0.0.0.0:3000:3000"
+      - "0.0.0.0:2112:2112"
+volumes:
+  $ATT_VOLUME:
+    external: true
+EOF
+
+echo "* starting rekor v0.6.0 to test issue #872"
+docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD up -d rekor-server-issue-872-v060
+sleep 5
+
+# this rekor-cli image is based on v0.6.0 and has the fix for issue #800
+ISSUE800_CONTAINER=gcr.io/projectsigstore/rekor/ci/rekor/rekor-cli@sha256:34f6ec6324a6f32f118dc14d33e5cc081fb8b49a5026d388f782a3566afa2ca8
+ISSUE800_CONTAINER_ID=$(docker create $ISSUE800_CONTAINER)
+ISSUE800_CLI=$REKORTMPDIR/rekor-cli-issue-800
+docker cp "$ISSUE800_CONTAINER_ID:/ko-app/rekor-cli" $ISSUE800_CLI
+docker rm $ISSUE800_CONTAINER_ID >/dev/null
+
+V060_UPLOAD_OUTPUT=$REKORTMPDIR/issue-872-upload-output
+echo "* inserting intoto entry into Rekor v0.6.0"
+if ! $ISSUE800_CLI upload --type intoto --artifact tests/intoto_dsse.json --public-key tests/intoto_dsse.pem --format=json --rekor_server=http://localhost:3000 > $V060_UPLOAD_OUTPUT; then
+   echo "* failed to insert intoto entry to test issue #872, exiting"
+   docker-compose logs --no-color > /tmp/docker-compose.log
+   docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD logs rekor-server-issue-872-v060 > /tmp/post-insert-docker-compose.log
+   exit 1
+fi
+
+ISSUE872_UPLOAD_INDEX=$(jq -r .Index $V060_UPLOAD_OUTPUT)
+V060_GET_OUTPUT=$REKORTMPDIR/issue-872-get-output
+echo "* read back entry from Rekor v0.6.0"
+if ! $ISSUE800_CLI get --log-index=$ISSUE872_UPLOAD_INDEX  --format=json --rekor_server=http://localhost:3000 > $V060_GET_OUTPUT; then
+   echo "* failed to retrieve entry from rekor v0.6.0 to test issue #872, exiting"
+   docker-compose logs --no-color > /tmp/docker-compose.log
+   docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD logs rekor-server-issue-872-v060 > /tmp/post-insert-docker-compose.log
+   exit 1
+fi
+
+echo "* checking to ensure attestation is successfully returned from rekor v0.6.0"
+V060_ATT_LENGTH=$(jq -r '.Attestation | length' $V060_GET_OUTPUT)
+if [ $V060_ATT_LENGTH -eq 0 ]; then
+   echo "* failed to read back attestation while testing issue #872 against rekor v0.6.0, exiting"
+   cat $V060_GET_OUTPUT
+   docker-compose logs --no-color > /tmp/docker-compose.log
+   docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD logs rekor-server-issue-872-v060 > /tmp/post-insert-docker-compose.log
+   exit 1
+fi
+
+echo "* grabbing TreeID to use when starting older version"
+REKOR_TRILLIAN_LOG_SERVER_TLOG_ID=$($ISSUE800_CLI loginfo --rekor_server=http://localhost:3000 --format=json | jq -r .TreeID)
+echo "* stopping rekor v0.6.0 to test issue #872"
+docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD logs rekor-server-issue-872-v060 > /tmp/post-insert-docker-compose.log
+docker-compose -f $V060_COMPOSE_FILE --project-directory=$PWD stop rekor-server-issue-872-v060
+
+COMPOSE_FILE=$REKORTMPDIR/docker-compose-issue872.yaml
+cat << EOF > $COMPOSE_FILE
+version: '3.4'
+services:
+  rekor-server:
+    build:
+      context: .
+      target: "deploy"
+    command: [
+      "rekor-server",
+      "serve",
+      "--trillian_log_server.address=trillian-log-server",
+      "--trillian_log_server.port=8090",
+      "--redis_server.address=redis-server",
+      "--redis_server.port=6379",
+      "--rekor_server.address=0.0.0.0",
+      "--rekor_server.signer=memory",
+      "--enable_attestation_storage",
+      "--attestation_storage_bucket=file:///var/run/attestations",
+      "--trillian_log_server.tlog_id=$REKOR_TRILLIAN_LOG_SERVER_TLOG_ID",
+      # Uncomment this for production logging
+      # "--log_type=prod",
+      ]
+    volumes:
+    - "$ATT_VOLUME:/var/run/attestations:z"
+    restart: always # keep the server running
+    ports:
+      - "3000:3000"
+      - "2112:2112"
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:3000/ping"]
+      interval: 10s
+      timeout: 3s
+      retries: 3
+      start_period: 5s
+volumes:
+  $ATT_VOLUME:
+    external: true
+EOF
+
+docker network prune -f
+echo "* starting rekor under test to ensure attestation inserted in old version is successfully returned"
+docker-compose -f $COMPOSE_FILE --project-directory=$PWD up -d
+waitForRekorServer
+
+ISSUE872_GET_ENTRY=$REKORTMPDIR/issue-872-get-entry
+echo "* fetching previous entry made under v0.6.0"
+if ! $REKOR_CLI get --log-index=$ISSUE872_UPLOAD_INDEX --rekor_server=http://localhost:3000 --format=json > $ISSUE872_GET_ENTRY; then
+   echo "* failed to read back intoto entry while testing issue #872, exiting"
+   docker-compose logs --no-color > /tmp/docker-compose.log
+   exit 1
+fi
+
+#ensure attestation of len() > 0 returned
+echo "* checking to ensure attestation is successfully returned"
+ATT_LENGTH=$(jq -r '.Attestation | length' $ISSUE872_GET_ENTRY)
+if [ $ATT_LENGTH -eq 0 ]; then
+   echo "* failed to read back attestation while testing issue #872, exiting"
+   cat $ISSUE872_GET_ENTRY
+   docker-compose logs --no-color > /tmp/docker-compose.log
+   exit 1
+else
+   echo "* tests succeeded!"
+fi