Skip to content

Commit c0fe21d

Browse files
committed
Drop support for fetching public keys by URL in the search index
This mitigates blind SSRF. Note that this API was marked as experimental so while this is a breaking change to the API, we offered no guarantee of stability. Fixes GHSA-4c4x-jm2x-pf9j Signed-off-by: Hayden <8418760+Hayden-IO@users.noreply.github.com>
1 parent 812e699 commit c0fe21d

File tree

8 files changed

+24
-49
lines changed

8 files changed

+24
-49
lines changed

cmd/rekor-cli/app/search.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,17 @@ var searchCmd = &cobra.Command{
167167
splitPubKeyString := strings.Split(publicKeyStr, ",")
168168
if len(splitPubKeyString) == 1 {
169169
if isURL(splitPubKeyString[0]) {
170-
params.Query.PublicKey.URL = strfmt.URI(splitPubKeyString[0])
170+
resp, err := http.Get(splitPubKeyString[0])
171+
if err != nil {
172+
return nil, fmt.Errorf("error fetching '%v': %w", splitPubKeyString[0], err)
173+
}
174+
defer resp.Body.Close()
175+
io.ReadAll(resp.Body)
176+
c, err := io.ReadAll(resp.Body)
177+
if err != nil {
178+
return nil, fmt.Errorf("error reading public key from '%v': %w", splitPubKeyString[0], err)
179+
}
180+
params.Query.PublicKey.Content = c
171181
} else {
172182
keyBytes, err := os.ReadFile(filepath.Clean(splitPubKeyString[0]))
173183
if err != nil {

openapi.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ paths:
140140
summary: Creates an entry in the transparency log
141141
description: >
142142
Creates an entry in the transparency log for a detached signature, public key, and content.
143-
Items can be included in the request or fetched by the server when URLs are specified.
144143
operationId: createLogEntry
145144
tags:
146145
- entries
@@ -496,9 +495,6 @@ definitions:
496495
content:
497496
type: string
498497
format: byte
499-
url:
500-
type: string
501-
format: uri
502498
required:
503499
- "format"
504500
hash:

pkg/api/index.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package api
1717

1818
import (
19+
"bytes"
1920
"context"
2021
"crypto/sha256"
2122
"encoding/hex"
@@ -62,11 +63,7 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder {
6263
if err != nil {
6364
return handleRekorAPIError(params, http.StatusBadRequest, err, unsupportedPKIFormat)
6465
}
65-
keyReader, err := util.FileOrURLReadCloser(httpReqCtx, params.Query.PublicKey.URL.String(), params.Query.PublicKey.Content)
66-
if err != nil {
67-
return handleRekorAPIError(params, http.StatusBadRequest, err, malformedPublicKey)
68-
}
69-
defer keyReader.Close()
66+
keyReader := bytes.NewReader(params.Query.PublicKey.Content)
7067

7168
key, err := af.NewPublicKey(keyReader)
7269
if err != nil {

pkg/generated/client/entries/entries_client.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/generated/models/search_index.go

Lines changed: 0 additions & 20 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/generated/restapi/embedded_spec.go

Lines changed: 2 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/generated/restapi/operations/entries/create_log_entry.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/util/fetch.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,18 @@ import (
2121
"fmt"
2222
"io"
2323
"net/http"
24+
"time"
2425
)
2526

26-
// FileOrURLReadCloser Note: caller is responsible for closing ReadCloser returned from method!
27+
// FileOrURLReadCloser reads content either from a URL or a byte slice
28+
// Note: Caller is responsible for closing the returned ReadCloser
29+
// Note: This must never be called from any server codepath to prevent SSRF
2730
func FileOrURLReadCloser(ctx context.Context, url string, content []byte) (io.ReadCloser, error) {
2831
var dataReader io.ReadCloser
2932
if url != "" {
30-
//TODO: set timeout here, SSL settings?
31-
client := &http.Client{}
33+
client := &http.Client{
34+
Timeout: 30 * time.Second,
35+
}
3236
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
3337
if err != nil {
3438
return nil, err

0 commit comments

Comments
 (0)