Drop support for fetching public keys by URL in the search index (#2731)

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>

Author: Hayden-IO

cmd/rekor-cli/app/search.go

@@ -115,13 +115,12 @@ var searchCmd = &cobra.Command{
 			hasher := sha256.New()
 			var tee io.Reader
 			if isURL(artifactStr) {
-				/* #nosec G107 */
-				resp, err := http.Get(artifactStr)
+				r, err := util.FileOrURLReadCloser(cmd.Context(), artifactStr, nil)
 				if err != nil {
 					return nil, fmt.Errorf("error fetching '%v': %w", artifactStr, err)
 				}
-				defer resp.Body.Close()
-				tee = io.TeeReader(resp.Body, hasher)
+				defer r.Close()
+				tee = io.TeeReader(r, hasher)
 			} else {
 				file, err := os.Open(filepath.Clean(artifactStr))
 				if err != nil {
@@ -167,7 +166,16 @@ var searchCmd = &cobra.Command{
 			splitPubKeyString := strings.Split(publicKeyStr, ",")
 			if len(splitPubKeyString) == 1 {
 				if isURL(splitPubKeyString[0]) {
-					params.Query.PublicKey.URL = strfmt.URI(splitPubKeyString[0])
+					r, err := util.FileOrURLReadCloser(cmd.Context(), splitPubKeyString[0], nil)
+					if err != nil {
+						return nil, fmt.Errorf("error fetching '%v': %w", splitPubKeyString[0], err)
+					}
+					defer r.Close()
+					c, err := io.ReadAll(r)
+					if err != nil {
+						return nil, fmt.Errorf("error reading public key from '%v': %w", splitPubKeyString[0], err)
+					}
+					params.Query.PublicKey.Content = c
 				} else {
 					keyBytes, err := os.ReadFile(filepath.Clean(splitPubKeyString[0]))
 					if err != nil {

openapi.yaml

@@ -140,7 +140,6 @@ paths:
       summary: Creates an entry in the transparency log
       description: >
         Creates an entry in the transparency log for a detached signature, public key, and content.
-        Items can be included in the request or fetched by the server when URLs are specified.
       operationId: createLogEntry
       tags:
         - entries
@@ -496,9 +495,6 @@ definitions:
           content:
             type: string
             format: byte
-          url:
-            type: string
-            format: uri
         required:
           - "format"
       hash:

pkg/api/index.go

@@ -16,6 +16,7 @@
 package api
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -62,11 +63,7 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder {
 		if err != nil {
 			return handleRekorAPIError(params, http.StatusBadRequest, err, unsupportedPKIFormat)
 		}
-		keyReader, err := util.FileOrURLReadCloser(httpReqCtx, params.Query.PublicKey.URL.String(), params.Query.PublicKey.Content)
-		if err != nil {
-			return handleRekorAPIError(params, http.StatusBadRequest, err, malformedPublicKey)
-		}
-		defer keyReader.Close()
+		keyReader := bytes.NewReader(params.Query.PublicKey.Content)
 
 		key, err := af.NewPublicKey(keyReader)
 		if err != nil {

pkg/generated/client/entries/entries_client.go

@@ -21,14 +21,18 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"time"
 )
 
-// FileOrURLReadCloser Note: caller is responsible for closing ReadCloser returned from method!
+// FileOrURLReadCloser reads content either from a URL or a byte slice
+// Note: Caller is responsible for closing the returned ReadCloser
+// Note: This must never be called from any server codepath to prevent SSRF
 func FileOrURLReadCloser(ctx context.Context, url string, content []byte) (io.ReadCloser, error) {
 	var dataReader io.ReadCloser
 	if url != "" {
-		//TODO: set timeout here, SSL settings?
-		client := &http.Client{}
+		client := &http.Client{
+			Timeout: 30 * time.Second,
+		}
 		req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 		if err != nil {
 			return nil, err