From 36dfbda0c8ee2de4bd9dc01fc6314a02ee8ba23b Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 29 Aug 2022 11:08:00 -0400 Subject: [PATCH 1/2] Add bounds on number of elements in api/v1/log/entries/retrieve Currently set the limit to 10 entries, we can increase it if users request it. Signed-off-by: Priya Wadhwa --- pkg/api/entries.go | 9 ++++++++ pkg/api/error.go | 1 + tests/e2e_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/pkg/api/entries.go b/pkg/api/entries.go index 5be6a6202..19c39aa9b 100644 --- a/pkg/api/entries.go +++ b/pkg/api/entries.go @@ -46,6 +46,10 @@ import ( "github.com/sigstore/sigstore/pkg/signature/options" ) +const ( + maxSearchQueries = 10 +) + func signEntry(ctx context.Context, signer signature.Signer, entry models.LogEntryAnon) ([]byte, error) { payload, err := entry.MarshalBinary() if err != nil { @@ -316,6 +320,11 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo resultPayload := []models.LogEntry{} tc := NewTrillianClient(httpReqCtx) + totalQueries := len(params.Entry.EntryUUIDs) + len(params.Entry.Entries()) + len(params.Entry.LogIndexes) + if totalQueries > maxSearchQueries { + return handleRekorAPIError(params, http.StatusBadRequest, fmt.Errorf(maxSearchQueryLimit, maxSearchQueries), maxSearchQueryLimit, maxSearchQueries) + } + if len(params.Entry.EntryUUIDs) > 0 || len(params.Entry.Entries()) > 0 { g, _ := errgroup.WithContext(httpReqCtx) diff --git a/pkg/api/error.go b/pkg/api/error.go index 5e2eb75e8..765ddcd8f 100644 --- a/pkg/api/error.go +++ b/pkg/api/error.go @@ -48,6 +48,7 @@ const ( sthGenerateError = "Error generating signed tree head" unsupportedPKIFormat = "The PKI format requested is not supported by this server" unexpectedInactiveShardError = "Unexpected error communicating with inactive shard" + maxSearchQueryLimit = "more than max allowed %d entries in request" ) func errorMsg(message string, code int) *models.Error { diff --git a/tests/e2e_test.go b/tests/e2e_test.go index 327c04762..5222beba8 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -30,7 +30,6 @@ import ( "encoding/json" "encoding/pem" "fmt" - "golang.org/x/sync/errgroup" "io/ioutil" "net/http" "os" @@ -42,6 +41,8 @@ import ( "testing" "time" + "golang.org/x/sync/errgroup" + "github.com/cyberphone/json-canonicalization/go/src/webpki.org/jsoncanonicalizer" "github.com/go-openapi/strfmt" "github.com/go-openapi/swag" @@ -921,3 +922,54 @@ func TestHostnameInSTH(t *testing.T) { t.Errorf("logInfo contains rekor.sigstore.dev which should not be set by default") } } + +func TestSearchQueryLimit(t *testing.T) { + tests := []struct { + description string + limit int + shouldErr bool + }{ + { + description: "request 6 entries", + limit: 6, + }, { + description: "request 10 entries", + limit: 10, + }, { + description: "request more than max", + limit: 12, + shouldErr: true, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + b := bytes.NewReader(getBody(t, test.limit)) + resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve", "application/json", b) + if err != nil { + t.Fatal(err) + } + c, _ := ioutil.ReadAll(resp.Body) + t.Log(string(c)) + if resp.StatusCode != 200 && !test.shouldErr { + t.Fatalf("expected test to pass but it failed") + } + if resp.StatusCode != 400 && test.shouldErr { + t.Fatal("expected test to fail but it passed") + } + if test.shouldErr && !strings.Contains(string(c), "more than max allowed") { + t.Fatal("expected max limit error but didn't get it") + } + }) + } +} + +func getBody(t *testing.T, limit int) []byte { + t.Helper() + s := fmt.Sprintf("{\"logIndexes\": [%d", limit) + for i := 1; i < limit; i++ { + s = fmt.Sprintf("%s, %d", s, i) + } + s += "]}" + return []byte(s) +} From b02320aace7a8dd0fcb7435c4f86b8a0736670b0 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 29 Aug 2022 12:03:40 -0400 Subject: [PATCH 2/2] Add maxItems:10 for SearchLogQuery Signed-off-by: Priya Wadhwa --- openapi.yaml | 7 ++++-- pkg/api/entries.go | 2 +- pkg/generated/models/search_log_query.go | 27 ++++++++++++++++++++++++ pkg/generated/restapi/embedded_spec.go | 14 +++++++++--- tests/e2e_test.go | 4 ++-- 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index 0946c93fb..93af089b6 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -519,21 +519,24 @@ definitions: properties: entryUUIDs: type: array + minItems: 1 + maxItems: 10 items: type: string - minItems: 1 pattern: '^([0-9a-fA-F]{64}|[0-9a-fA-F]{80})$' logIndexes: type: array minItems: 1 + maxItems: 10 items: type: integer minimum: 0 entries: type: array + minItems: 1 + maxItems: 10 items: $ref: '#/definitions/ProposedEntry' - minItems: 1 LogInfo: type: object diff --git a/pkg/api/entries.go b/pkg/api/entries.go index 19c39aa9b..abc3cefad 100644 --- a/pkg/api/entries.go +++ b/pkg/api/entries.go @@ -322,7 +322,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo totalQueries := len(params.Entry.EntryUUIDs) + len(params.Entry.Entries()) + len(params.Entry.LogIndexes) if totalQueries > maxSearchQueries { - return handleRekorAPIError(params, http.StatusBadRequest, fmt.Errorf(maxSearchQueryLimit, maxSearchQueries), maxSearchQueryLimit, maxSearchQueries) + return handleRekorAPIError(params, http.StatusUnprocessableEntity, fmt.Errorf(maxSearchQueryLimit, maxSearchQueries), fmt.Sprintf(maxSearchQueryLimit, maxSearchQueries)) } if len(params.Entry.EntryUUIDs) > 0 || len(params.Entry.Entries()) > 0 { diff --git a/pkg/generated/models/search_log_query.go b/pkg/generated/models/search_log_query.go index 37beafab7..6838b8a76 100644 --- a/pkg/generated/models/search_log_query.go +++ b/pkg/generated/models/search_log_query.go @@ -42,9 +42,12 @@ type SearchLogQuery struct { entriesField []ProposedEntry // entry u UI ds + // Max Items: 10 + // Min Items: 1 EntryUUIDs []string `json:"entryUUIDs"` // log indexes + // Max Items: 10 // Min Items: 1 LogIndexes []*int64 `json:"logIndexes"` } @@ -158,6 +161,16 @@ func (m *SearchLogQuery) validateEntries(formats strfmt.Registry) error { return nil } + iEntriesSize := int64(len(m.Entries())) + + if err := validate.MinItems("entries", "body", iEntriesSize, 1); err != nil { + return err + } + + if err := validate.MaxItems("entries", "body", iEntriesSize, 10); err != nil { + return err + } + for i := 0; i < len(m.Entries()); i++ { if err := m.entriesField[i].Validate(formats); err != nil { @@ -179,6 +192,16 @@ func (m *SearchLogQuery) validateEntryUUIDs(formats strfmt.Registry) error { return nil } + iEntryUUIDsSize := int64(len(m.EntryUUIDs)) + + if err := validate.MinItems("entryUUIDs", "body", iEntryUUIDsSize, 1); err != nil { + return err + } + + if err := validate.MaxItems("entryUUIDs", "body", iEntryUUIDsSize, 10); err != nil { + return err + } + for i := 0; i < len(m.EntryUUIDs); i++ { if err := validate.Pattern("entryUUIDs"+"."+strconv.Itoa(i), "body", m.EntryUUIDs[i], `^([0-9a-fA-F]{64}|[0-9a-fA-F]{80})$`); err != nil { @@ -201,6 +224,10 @@ func (m *SearchLogQuery) validateLogIndexes(formats strfmt.Registry) error { return err } + if err := validate.MaxItems("logIndexes", "body", iLogIndexesSize, 10); err != nil { + return err + } + for i := 0; i < len(m.LogIndexes); i++ { if swag.IsZero(m.LogIndexes[i]) { // not required continue diff --git a/pkg/generated/restapi/embedded_spec.go b/pkg/generated/restapi/embedded_spec.go index ef89e89cb..792cfaade 100644 --- a/pkg/generated/restapi/embedded_spec.go +++ b/pkg/generated/restapi/embedded_spec.go @@ -634,21 +634,24 @@ func init() { "properties": { "entries": { "type": "array", + "maxItems": 10, + "minItems": 1, "items": { - "minItems": 1, "$ref": "#/definitions/ProposedEntry" } }, "entryUUIDs": { "type": "array", + "maxItems": 10, + "minItems": 1, "items": { "type": "string", - "pattern": "^([0-9a-fA-F]{64}|[0-9a-fA-F]{80})$", - "minItems": 1 + "pattern": "^([0-9a-fA-F]{64}|[0-9a-fA-F]{80})$" } }, "logIndexes": { "type": "array", + "maxItems": 10, "minItems": 1, "items": { "type": "integer" @@ -2517,12 +2520,16 @@ func init() { "properties": { "entries": { "type": "array", + "maxItems": 10, + "minItems": 1, "items": { "$ref": "#/definitions/ProposedEntry" } }, "entryUUIDs": { "type": "array", + "maxItems": 10, + "minItems": 1, "items": { "type": "string", "pattern": "^([0-9a-fA-F]{64}|[0-9a-fA-F]{80})$" @@ -2530,6 +2537,7 @@ func init() { }, "logIndexes": { "type": "array", + "maxItems": 10, "minItems": 1, "items": { "type": "integer", diff --git a/tests/e2e_test.go b/tests/e2e_test.go index 5222beba8..e28c4bd1d 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -954,10 +954,10 @@ func TestSearchQueryLimit(t *testing.T) { if resp.StatusCode != 200 && !test.shouldErr { t.Fatalf("expected test to pass but it failed") } - if resp.StatusCode != 400 && test.shouldErr { + if resp.StatusCode != 422 && test.shouldErr { t.Fatal("expected test to fail but it passed") } - if test.shouldErr && !strings.Contains(string(c), "more than max allowed") { + if test.shouldErr && !strings.Contains(string(c), "logIndexes in body should have at most 10 items") { t.Fatal("expected max limit error but didn't get it") } })