From 1fc1ebbb8184af6e01a9db8c0532b3652eab7791 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Fri, 9 Sep 2022 14:51:43 -0500 Subject: [PATCH] fix retrieve endpoint response code and add testing Signed-off-by: Asra Ali --- pkg/api/entries.go | 32 +++++++++----- tests/canonical_rekor.json | 20 +++++++++ tests/e2e_test.go | 91 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 tests/canonical_rekor.json diff --git a/pkg/api/entries.go b/pkg/api/entries.go index 6fa6f6b01..64251a16a 100644 --- a/pkg/api/entries.go +++ b/pkg/api/entries.go @@ -361,11 +361,15 @@ 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 @@ -373,16 +377,15 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo // 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 { @@ -390,13 +393,12 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo 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) @@ -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 { @@ -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()) } @@ -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 @@ -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 { diff --git a/tests/canonical_rekor.json b/tests/canonical_rekor.json new file mode 100644 index 000000000..d5c6b6398 --- /dev/null +++ b/tests/canonical_rekor.json @@ -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" + } + } + } +} \ No newline at end of file diff --git a/tests/e2e_test.go b/tests/e2e_test.go index 9408855b0..d03b80e81 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -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. @@ -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") @@ -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) @@ -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) } }