Skip to content

Commit

Permalink
fix retrieve endpoint response code and add testing (#1043)
Browse files Browse the repository at this point in the history 
Signed-off-by: Asra Ali <asraa@google.com>

Signed-off-by: Asra Ali <asraa@google.com>
  • Loading branch information
@asraa
asraa committed Sep 13, 2022
1 parent d925bc3 commit a5f3b0a
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 14 deletions.
32 changes: 22 additions & 10 deletions pkg/api/entries.go
View file
Expand Up @@ -361,42 +361,44 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
g, _ := errgroup.WithContext(httpReqCtx)

var searchHashes [][]byte
code := http.StatusBadRequest
for _, entryID := range params.Entry.EntryUUIDs {
if sharding.ValidateEntryID(entryID) == nil {
logEntry, err := retrieveLogEntry(httpReqCtx, entryID)
if errors.Is(err, ErrNotFound) {
code = http.StatusNotFound
}
if err != nil {
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("error getting log entry for %s", entryID))
return handleRekorAPIError(params, code, err, fmt.Sprintf("error getting log entry for %s", entryID))
}
resultPayload = append(resultPayload, logEntry)
continue
}
// At this point, check if we got a uuid instead of an EntryID, so search for the hash later
uuid := entryID
if err := sharding.ValidateUUID(uuid); err != nil {
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("validating uuid %s", uuid))
return handleRekorAPIError(params, code, err, fmt.Sprintf("validating uuid %s", uuid))
}
hash, err := hex.DecodeString(uuid)
if err != nil {
return handleRekorAPIError(params, http.StatusBadRequest, err, malformedUUID)
return handleRekorAPIError(params, code, err, malformedUUID)
}
searchHashes = append(searchHashes, hash)
}

code := http.StatusBadRequest
entries := params.Entry.Entries()
searchHashesChan := make(chan []byte, len(entries))
for _, e := range entries {
e := e // https://golang.org/doc/faq#closures_and_goroutines
g.Go(func() error {
entry, err := types.UnmarshalEntry(e)
if err != nil {
return err
return fmt.Errorf("unmarshalling entry: %w", err)
}

leaf, err := types.CanonicalizeEntry(httpReqCtx, entry)
if err != nil {
code = http.StatusInternalServerError
return err
return fmt.Errorf("canonicalizing entry: %w", err)
}
hasher := rfc6962.DefaultHasher
leafHash := hasher.HashLeaf(leaf)
Expand All @@ -420,9 +422,11 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
g.Go(func() error {
resp := tc.getLeafAndProofByHash(hash)
switch resp.status {
case codes.OK, codes.NotFound:
default:
case codes.OK:
case codes.NotFound:
code = http.StatusNotFound
return resp.err
default:
}
leafResult := resp.getLeafAndProofResult
if leafResult != nil && leafResult.Leaf != nil {
Expand All @@ -440,6 +444,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
if leafResp != nil {
logEntry, err := logEntryFromLeaf(httpReqCtx, api.signer, tc, leafResp.Leaf, leafResp.SignedLogRoot, leafResp.Proof, api.logRanges.ActiveTreeID(), api.logRanges)
if err != nil {
code = http.StatusInternalServerError
return handleRekorAPIError(params, code, err, err.Error())
}

Expand All @@ -451,11 +456,18 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
if len(params.Entry.LogIndexes) > 0 {
g, _ := errgroup.WithContext(httpReqCtx)
resultPayloadChan := make(chan models.LogEntry, len(params.Entry.LogIndexes))

code := http.StatusInternalServerError
for _, logIndex := range params.Entry.LogIndexes {
logIndex := logIndex // https://golang.org/doc/faq#closures_and_goroutines
g.Go(func() error {
logEntry, err := retrieveLogEntryByIndex(httpReqCtx, int(swag.Int64Value(logIndex)))
if err != nil {
switch {
case errors.Is(err, ErrNotFound):
code = http.StatusNotFound
default:
}
return err
}
resultPayloadChan <- logEntry
Expand All @@ -464,7 +476,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
}

if err := g.Wait(); err != nil {
return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc error: %w", err), trillianUnexpectedResult)
return handleRekorAPIError(params, code, err, err.Error())
}
close(resultPayloadChan)
for result := range resultPayloadChan {
Expand Down
20 changes: 20 additions & 0 deletions tests/canonical_rekor.json
View file
@@ -0,0 +1,20 @@
{
"kind": "rekord",
"apiVersion": "0.0.1",
"spec": {
"signature": {
"format": "pgp",
"content": "iQHKBAABCAA0FiEEcgCUXG78adj6hGUJJrfBoJ04pHoFAl+86RwWHGxoaW5kc0Bwcm90b25tYWlsLmNvbQAKCRAmt8GgnTikejcHC/9yyGEPh2D+MnNR8I8w0sfWChc6pGAQoS6qk/sfC/9GvF4OC7RIy6OwLr/lxyEZbOP2ngYjh/s5KjKxhZyApwwg13LmcbazGnXc3E76J55LoTfwoRa9fupH/M6HI56VFKwnu+AbMNW1s+DM47r7i5nIN6IX9kMpDe3B9XTUULff/yNUv0XtXU+VAf8ndF1w117YVWxf8TnU/HWvX74URQPN+syuyqK/NO1H1KhBVTzcIYd5H6kJu300jgkDypyyqQpd/pJYVwfeY8fCOaeCpfIPjKQ/4enCsAeBgKsAwfIbor8WiE86KoANYqROaW7uqiN+VPadbWVeN6bMpRIdEq8+NKQGlepSCRqbkVg4VKGOPgB3h5WbY9U1O1FVDnXyt7kWdEPEZjBX+V4DawshvNe5LIyqH5hJ1QNAFd0UStqKQt8EUZ/gAtQiXSGbxM1ACoYL9HblKW5b+kj/onKghekFoCoAfhMwRRqR5g/TS/Pc2/ztwYTIuhpQQfMXziTm64g=",
"publicKey": {
"content":"LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUdOQkYrY0lNMEJEQUNhOEc3UkQydjNtaXdNdHhWYVppM0pCVnVlVkFxSEtDNGVLb01TNUhNQ1JvK0haVlJBCjcwWG1zVHBYMVoxZ1pRdXVDMEdEWTI2aEJoZWpBcTNoeDJydjYvOHE5MEJ2V0dIOXRWZUdwTDFzYUltNTJnRVIKWHlWZ2d6NWtBQzBTNnZNbjdkcjJldEJrV1dQK09qMDVTMDJZWkJUWWd4cE9ieWVjVVNjcUtOVGpzbFpRQkgyZApTSHVrM28yWjdoTTQ5VTBsN3piV3c0b0lUK2xBUmNzYWpRVHdXamxpYVBEL0hSalQyblJPaEloaXRlZC93Z3l6CnlkSXE1ZTZzMThWTGNUNzVxV25yWlhOUFdGd2YyNVJYWTN1dGtXK0dXNW5RZU44MFEya1JFZ2t4RnM1QWQ1V1oKdkU3dDgvaHg1em1zbFo0dGZGNHNpM1FaZUlRQmFjWWJ3eE1QU0RmOW9GR3hkR0ZUODg5d0pMR2dXbXIxVGtQTQpjTjA2d3hBUkd0eE4wejYxRkpUaWpMV1JialczdW5JOWhjUWNVbE4vUSsxNm90SHBlS1ZnNG9XMDBDcnZXT0Q2CnFrTGNNRDQ5eVVEOGZTR0IrUkVuaUJhODlDOWtRVTRTS2Rnc0xML1ErSksrU3k5S21JRHJtMWE0RGZQMXBzZmUKTGphcnpzVlpmS1VIZndjQUVRRUFBYlFpVEhWclpTQklhVzVrY3lBOGJHaHBibVJ6UUhCeWIzUnZibTFoYVd3dQpZMjl0UG9rQjFBUVRBUWdBUGhZaEJISUFsRnh1L0duWStvUmxDU2Ezd2FDZE9LUjZCUUpmbkNETkFoc0RCUWtECndtY0FCUXNKQ0FjQ0JoVUtDUWdMQWdRV0FnTUJBaDRCQWhlQUFBb0pFQ2Ezd2FDZE9LUjZaMWtMLzFJSzB2ZGUKWlg1cjVTZWJOeFRJTlNBQXZZa3JLUnlKNWY3bE9NOWdMR0l1YzJGb05VbmpWUVQwcklHOTAxOWg0OHBDeTkxZgpYakREUk1ZOWd6RldXQ2dHblhoMWhXSTNNN0JKRjZZRTZ1NkRYR3N2dVVwR3JOZVpBRzZra2F6QXVBbm5WMGtDCjA4em9SckFaQ3ZscGFacnlkOGl0YityVitRS3A3QXcybEFJSDFlNmR3TTRSTEZqdmZrOExKWHhqSkFvUG13NmwKTHcxOGM3b1c2UkxPOVFYUThlTTZyMnZISHBtMFR1ZHZaeWFmTnVDMzJHRGxNWTR1MFYxRGI4THN5bVBzQWh1QQoySno0L0tQcTZ1S3dJdG1WSzRwbmRmRUR1NkQxVG9vRFlYaXB0WWFmZHZVMzNwVVF4d0hvZlRUZkU1elp3MlBlCmxIM25aZHNnSFhHUHhKTExNcU9wVzRDL2NNNlpRVmdZU3RWcjBudlU2NitRalF2c2tVWlIwNmRkRXpuQnBHSnMKdHBtajlBZS9HUlk4RU5uTjkvMkdmRXVydHozZEtOVVpvak15MTUzamNHMFUxenpoMTE1V0o3dDh3SEJ1NFM0cAowZ0UrUkFxeXRBY0laRGQyTlNOcno4VnI5RkU5eCtmYXQ5RVJsYm5kQUJFNWlWOHNLMCtGYW5Xd2dia0JqUVJmCm5DRE5BUXdBdEJvdGhmY1J6cjN4cjNQOXA3UUNNd0t1aW9udk1DbThXZ3dOUzRDcGhxbzVOT3IyaU1qa0xQMEoKb21nSkxWWDVOK2Jydjh5NEg4cllQd0tCMTZvL2hBOEliR2JwWXltM0ZjeWtUd2NiV2J0UFRMRXRkQ1VQTFlURApOQzVMR0pwZzNlODZZZlF0QU42L01uWnlZT21sRHgyV0d0dExkbXNBU0dWdXg2QVZKcUl2K3gwNlVLSkVtSzN0CmpsRVZLeWcxMlJFenllNUlUNnFFU0dwT3pvMllsV1VxSVR3L0FhUFEyWnhVYXh2WUZvVU9jd2djZG5Ia2dzaEkKT245aC9OSFVtUDMyV1F2cWtRTXVVYVBJTlJzQzgzS3ZUREdseWZTSFZGek1hNGhETWhFY1h6NGFjaW5kNVdUZQp6eUxnWmhPYjdjTmVDeDR4Y3J0UEI2VTdCUi9GVkx6TEJsQXp1emppRWhZd0pvM0FPTXFGb1I1bUFxaGx1dE5PCnNzeW9mYnFUZ0diU0xkamJYUC9hRXRnejJNVjluL29jMVNCOEhlWk8vMTdKeWduenJ1SUt5Ky9sT1dPenQralYKVkZwVnloMXVlOGxGN3ltS1I0dHNsK2lJVmJxblB2cE1oTE9JQnFYRm4yZ01Da0dvSkx5N09IbzJXQUVKR2x0MwpTd3BicmpqMUFCRUJBQUdKQWJ3RUdBRUlBQ1lXSVFSeUFKUmNidnhwMlBxRVpRa210OEdnblRpa2VnVUNYNXdnCnpRSWJEQVVKQThKbkFBQUtDUkFtdDhHZ25UaWtlaW5pREFDRUFma1pxLzRScDJhTkE0ZGJvSjdVRlhET2FSa1YKOU1Lb0VaRnFUTU5vdkRMNXhoTWxnbFBQdS9sK2RoVGd4ZGVKOUVWSG9lenRiODk2VS9wT3VCUnNuOVZ0VzRZLwpqZWlXN0V5TlhBZC9PcnZuRmJ4KzdpWExxdXBaSkpGVGkvajlSaFZZTnNtbDdzZWJUUGVCbkdEQTkxcWJDNHhICnBRVkRDdWp4NjlWeE81RTFMU29oQ00rTy81dkxCbThpMW8vbmJGbWJ5N1ZDeUtlUkRmaHRmOW5DODRxc0U5R3EKVTcvTFNpazliZnhNV2JwcTh5a250bVMzYTBzemM0YlZGcGV6QnBtTmIwQVZjQitUbTlnV21FemhpTHM2RktBTgpJbnFOdVh1Qkw5UENhYzcrbVUrYzJtQmdHT1JHZDFkWk8zUkM4OXpGM3hCQlluQ09lNWNBTUZsYzFYR3NsbHNJCmR6ZHJkWHZiTkJ6L2o3MXB1TjhvRlltL1hiVmNpZU8wVGZRaURjVHQ4S2lpUjlUQUQ5L1A1OTNSTWxMT0dTOHAKaHZKYmlGb1pmWEhjbHNaRkhtOERRUWE5NElad1RCOG00Z0JWME0yWFN2ZEhvMzBsc3FqdFphWmlTclJoNHJzaApuMTRwYkFhVGRhS0VQY3Z0dWZiVXVXMElqWWQya3BJVC90Zz0KPU9naHIKLS0tLS1FTkQgUEdQIFBVQkxJQyBLRVkgQkxPQ0stLS0tLQo="
}
},
"data": {
"content": "aGVsbG8gcmVrb3IK",
"hash": {
"algorithm": "sha256",
"value": "45c7b11fcbf07dec1694adecd8c5b85770a12a6c8dfdcf2580a2db0c47c31779"
}
}
}
}
91 changes: 87 additions & 4 deletions tests/e2e_test.go
View file
Expand Up @@ -142,10 +142,11 @@ func TestUploadVerifyRekord(t *testing.T) {
}

// Verify should fail initially
runCliErr(t, "verify", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
out := runCliErr(t, "verify", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
outputContains(t, out, "404")

// It should upload successfully.
out := runCli(t, "upload", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
out = runCli(t, "upload", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
outputContains(t, out, "Created entry at")

// Now we should be able to verify it.
Expand Down Expand Up @@ -996,12 +997,45 @@ func TestGetNonExistantIndex(t *testing.T) {
outputContains(t, out, "404")
}

func TestVerifyNonExistantIndex(t *testing.T) {
// this index is extremely likely to not exist
out := runCliErr(t, "verify", "--log-index", "100000000")
outputContains(t, out, "404")
}

func TestGetNonExistantUUID(t *testing.T) {
// this uuid is extremely likely to not exist
out := runCliErr(t, "get", "--uuid", "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
outputContains(t, out, "404")
}

func TestVerifyNonExistantUUID(t *testing.T) {
// this uuid is extremely likely to not exist
out := runCliErr(t, "verify", "--uuid", "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
outputContains(t, out, "404")

// Check response code
tid := getTreeID(t)
h := sha256.Sum256([]byte("123"))
entryID, err := sharding.CreateEntryIDFromParts(fmt.Sprintf("%x", tid),
hex.EncodeToString(h[:]))
if err != nil {
t.Fatal(err)
}
body := fmt.Sprintf("{\"entryUUIDs\":[\"%s\"]}", entryID.ReturnEntryIDString())
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
"application/json",
bytes.NewReader([]byte(body)))
if err != nil {
t.Fatal(err)
}
c, _ := ioutil.ReadAll(resp.Body)
t.Log(string(c))
if resp.StatusCode != 404 {
t.Fatal("expected 404 status")
}
}

func TestEntryUpload(t *testing.T) {
artifactPath := filepath.Join(t.TempDir(), "artifact")
sigPath := filepath.Join(t.TempDir(), "signature.asc")
Expand Down Expand Up @@ -1193,6 +1227,54 @@ func TestSearchQueryLimit(t *testing.T) {
}
}

func TestSearchQueryMalformedEntry(t *testing.T) {
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
b, err := ioutil.ReadFile(filepath.Join(wd, "rekor.json"))
if err != nil {
t.Fatal(err)
}
body := fmt.Sprintf("{\"entries\":[\"%s\"]}", b)
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
"application/json",
bytes.NewBuffer([]byte(body)))
if err != nil {
t.Fatal(err)
}
c, _ := ioutil.ReadAll(resp.Body)
t.Log(string(c))
if resp.StatusCode != 400 {
t.Fatal("expected status 400")
}
}

func TestSearchQueryNonExistentEntry(t *testing.T) {
// Nonexistent but well-formed entry results in 404 not found.
wd, err := os.Getwd()
if err != nil {
t.Fatal(err)
}
b, err := ioutil.ReadFile(filepath.Join(wd, "canonical_rekor.json"))
if err != nil {
t.Fatal(err)
}
body := fmt.Sprintf("{\"entries\":[%s]}", b)
t.Log(string(body))
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
"application/json",
bytes.NewBuffer([]byte(body)))
if err != nil {
t.Fatal(err)
}
c, _ := ioutil.ReadAll(resp.Body)
t.Log(string(c))
if resp.StatusCode != 404 {
t.Fatal("expected 404 status")
}
}

func getBody(t *testing.T, limit int) []byte {
t.Helper()
s := fmt.Sprintf("{\"logIndexes\": [%d", limit)
Expand Down Expand Up @@ -1260,7 +1342,8 @@ func TestSearchValidateTreeID(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if resp.StatusCode != 400 {
t.Fatalf("expected 400 status code but got %d", resp.StatusCode)
// Not Found because currently we don't detect that an unused random tree ID is invalid.
if resp.StatusCode != 404 {
t.Fatalf("expected 404 status code but got %d", resp.StatusCode)
}
}

0 comments on commit a5f3b0a

Please sign in to comment.