Skip to content

Commit

Permalink
feat(builder-spec): Correctly reject SSZ Accept/Content-Types headers. (
Browse files Browse the repository at this point in the history
#678)

* feat(builder-spec): Correctly reject SSZ Accept/Content-Types headers.

* fixup: clearer Content-Type comment

* fixup: GHA lint differs from local one.

* Update services/api/service.go

Co-authored-by: Justin Traglia <[email protected]>

* Fix ErrNotAcceptable indentation

* Run go mod tidy

* Add content type check for registerValidators

---------

Co-authored-by: Justin Traglia <[email protected]>
Co-authored-by: Justin Traglia <[email protected]>
  • Loading branch information
3 people authored Feb 19, 2025
1 parent 3a0417e commit a89aea6
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 0 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ linters:
- interfacebloat
- exhaustruct
- tenv
- gocognit

#
# Disabled because of generics:
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.24.0
require (
github.com/NYTimes/gziphandler v1.1.1
github.com/alicebob/miniredis/v2 v2.32.1
github.com/aohorodnyk/mimeheader v0.0.6
github.com/attestantio/go-builder-client v0.5.1-0.20250120215322-c65b220a98eb
github.com/attestantio/go-eth2-client v0.22.1-0.20250106164842-07b6ce39bb43
github.com/bradfitz/gomemcache v0.0.0-20230124162541-5f7a7d875746
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a h1:HbKu58rmZp
github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
github.com/alicebob/miniredis/v2 v2.32.1 h1:Bz7CciDnYSaa0mX5xODh6GUITRSx+cVhjNoOR4JssBo=
github.com/alicebob/miniredis/v2 v2.32.1/go.mod h1:AqkLNAfUm0K07J28hnAyyQKf/x0YkCY/g5DCtuL01Mw=
github.com/aohorodnyk/mimeheader v0.0.6 h1:WCV4NQjtbqnd2N3FT5MEPesan/lfvaLYmt5v4xSaX/M=
github.com/aohorodnyk/mimeheader v0.0.6/go.mod h1:/Gd3t3vszyZYwjNJo2qDxoftZjjVzMdkQZxkiINp3vM=
github.com/attestantio/go-builder-client v0.5.1-0.20250120215322-c65b220a98eb h1:J4Dpih8da/kACBsY104/mKzDv42a3O5TXTElY5ZkN80=
github.com/attestantio/go-builder-client v0.5.1-0.20250120215322-c65b220a98eb/go.mod h1:X31JAUL4q6cY/OGClpBQcwFN7FBixt6Wjrqy7RrlhEc=
github.com/attestantio/go-eth2-client v0.22.1-0.20250106164842-07b6ce39bb43 h1:lORlCOleRXvVt3H7fan64UaYAK4FJDHdy19uYfe7FKQ=
Expand Down
79 changes: 79 additions & 0 deletions services/api/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/NYTimes/gziphandler"
"github.com/aohorodnyk/mimeheader"
builderApi "github.com/attestantio/go-builder-client/api"
builderApiV1 "github.com/attestantio/go-builder-client/api/v1"
"github.com/attestantio/go-eth2-client/spec"
Expand Down Expand Up @@ -944,6 +945,46 @@ func (api *RelayAPI) handleStatus(w http.ResponseWriter, req *http.Request) {
w.WriteHeader(http.StatusOK)
}

const (
ApplicationJSON = "application/json"
ApplicationOctetStream = "application/octet-stream"
)

// RequestAcceptsJSON returns true if the Accept header is empty (defaults to JSON)
// or application/json can be negotiated.
func RequestAcceptsJSON(req *http.Request) bool {
ah := req.Header.Get("Accept")
if ah == "" {
return true
}
mh := mimeheader.ParseAcceptHeader(ah)
_, _, matched := mh.Negotiate(
[]string{ApplicationJSON},
ApplicationJSON,
)
return matched
}

// NegotiateRequestResponseType returns whether the request accepts
// JSON (application/json) or SSZ (application/octet-stream) responses.
// If accepted is false, no mime type could be negotiated and the server
// should respond with http.StatusNotAcceptable.
func NegotiateRequestResponseType(req *http.Request) (mimeType string, err error) {
ah := req.Header.Get("Accept")
if ah == "" {
return ApplicationJSON, nil
}
mh := mimeheader.ParseAcceptHeader(ah)
_, mimeType, matched := mh.Negotiate(
[]string{ApplicationJSON, ApplicationOctetStream},
ApplicationJSON,
)
if !matched {
return "", ErrNotAcceptable
}
return mimeType, nil
}

// ---------------
// PROPOSER APIS
// ---------------
Expand All @@ -963,6 +1004,19 @@ func (api *RelayAPI) handleRegisterValidator(w http.ResponseWriter, req *http.Re
"contentLength": req.ContentLength,
})

// If the Content-Type header is included, for now only allow JSON.
// TODO: support Content-Type: application/octet-stream and allow SSZ
// request bodies.
if ct := req.Header.Get("Content-Type"); ct != "" {
switch ct {
case ApplicationJSON:
break
default:
api.RespondError(w, http.StatusUnsupportedMediaType, "only Content-Type: application/json is currently supported")
return
}
}

start := time.Now().UTC()
registrationTimestampUpperBound := start.Unix() + 10 // 10 seconds from now

Expand Down Expand Up @@ -1219,6 +1273,12 @@ func (api *RelayAPI) handleGetHeader(w http.ResponseWriter, req *http.Request) {
return
}

// TODO: Use NegotiateRequestResponseType, for now we only accept JSON
if !RequestAcceptsJSON(req) {
api.RespondError(w, http.StatusNotAcceptable, "only Accept: application/json is currently supported")
return
}

log.Debug("getHeader request received")
defer func() {
metrics.GetHeaderLatencyHistogram.Record(
Expand Down Expand Up @@ -1328,6 +1388,25 @@ func (api *RelayAPI) handleGetPayload(w http.ResponseWriter, req *http.Request)
)
}()

// TODO: Use NegotiateRequestResponseType, for now we only accept JSON
if !RequestAcceptsJSON(req) {
api.RespondError(w, http.StatusNotAcceptable, "only Accept: application/json is currently supported")
return
}

// If the Content-Type header is included, for now only allow JSON.
// TODO: support Content-Type: application/octet-stream and allow SSZ
// request bodies.
if ct := req.Header.Get("Content-Type"); ct != "" {
switch ct {
case ApplicationJSON:
break
default:
api.RespondError(w, http.StatusUnsupportedMediaType, "only Content-Type: application/json is currently supported")
return
}
}

// Read the body first, so we can decode it later
body, err := io.ReadAll(req.Body)
if err != nil {
Expand Down
58 changes: 58 additions & 0 deletions services/api/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1383,3 +1383,61 @@ func gzipBytes(t *testing.T, b []byte) []byte {
require.NoError(t, zw.Close())
return buf.Bytes()
}

func TestRequestAcceptsJSON(t *testing.T) {
for _, tc := range []struct {
Header string
Expected bool
}{
{Header: "", Expected: true},
{Header: "application/json", Expected: true},
{Header: "application/octet-stream", Expected: false},
{Header: "application/octet-stream;q=1.0,application/json;q=0.9", Expected: true},
{Header: "application/octet-stream;q=1.0,application/something-else;q=0.9", Expected: false},
{Header: "application/octet-stream;q=1.0,application/*;q=0.9", Expected: true},
{Header: "application/octet-stream;q=1.0,*/*;q=0.9", Expected: true},
{Header: "application/*;q=0.9", Expected: true},
{Header: "application/*", Expected: true},
} {
t.Run(tc.Header, func(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "/eth/v1/builder/header/1/0x00/0xaa", nil)
require.NoError(t, err)
req.Header.Set("Accept", tc.Header)
actual := RequestAcceptsJSON(req)
require.Equal(t, tc.Expected, actual)
})
}
}

func TestNegotiateRequestResponseType(t *testing.T) {
for _, tc := range []struct {
Header string
Expected string
Error error
}{
{Header: "", Expected: ApplicationJSON},
{Header: "application/json", Expected: ApplicationJSON},
{Header: "application/octet-stream", Expected: ApplicationOctetStream},
{Header: "application/octet-stream;q=1.0,application/json;q=0.9", Expected: ApplicationOctetStream},
{Header: "application/octet-stream;q=1.0,application/something-else;q=0.9", Expected: ApplicationOctetStream},
{Header: "application/octet-stream;q=1.0,application/*;q=0.9", Expected: ApplicationOctetStream},
{Header: "application/octet-stream;q=1.0,*/*;q=0.9", Expected: ApplicationOctetStream},
{Header: "application/octet-stream;q=0.9,*/*;q=1.0", Expected: ApplicationJSON},
{Header: "application/*;q=0.9", Expected: ApplicationJSON, Error: nil},
{Header: "application/*", Expected: ApplicationJSON, Error: nil},
{Header: "text/html", Error: ErrNotAcceptable},
} {
t.Run(tc.Header, func(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "/eth/v1/builder/header/1/0x00/0xaa", nil)
require.NoError(t, err)
req.Header.Set("Accept", tc.Header)
negotiated, err := NegotiateRequestResponseType(req)
if tc.Error != nil {
require.Equal(t, tc.Error, err)
} else {
require.NoError(t, err)
require.Equal(t, tc.Expected, negotiated)
}
})
}
}
1 change: 1 addition & 0 deletions services/api/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var (
ErrPayloadMismatch = errors.New("beacon-block and payload version mismatch")
ErrHeaderHTRMismatch = errors.New("beacon-block and payload header mismatch")
ErrBlobMismatch = errors.New("beacon-block and payload blob contents mismatch")
ErrNotAcceptable = errors.New("not acceptable")
)

func SanityCheckBuilderBlockSubmission(payload *common.VersionedSubmitBlockRequest) error {
Expand Down

0 comments on commit a89aea6

Please sign in to comment.