From cb363f3b7b8c42ad9f7b55df2bbc0bbe16871f27 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Mon, 10 Feb 2025 14:35:06 -0800 Subject: [PATCH 1/7] feat(builder-spec): Correctly reject SSZ Accept/Content-Types headers. --- go.mod | 1 + go.sum | 2 ++ services/api/service.go | 65 ++++++++++++++++++++++++++++++++++++ services/api/service_test.go | 58 ++++++++++++++++++++++++++++++++ services/api/utils.go | 1 + 5 files changed, 127 insertions(+) diff --git a/go.mod b/go.mod index d5e369cd..e8546008 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.22 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.4.3-0.20240124194555-d44db06f45fa github.com/attestantio/go-eth2-client v0.21.1 github.com/bradfitz/gomemcache v0.0.0-20230124162541-5f7a7d875746 diff --git a/go.sum b/go.sum index 02468b62..80436cac 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,8 @@ github.com/alicebob/miniredis/v2 v2.32.1 h1:Bz7CciDnYSaa0mX5xODh6GUITRSx+cVhjNoO github.com/alicebob/miniredis/v2 v2.32.1/go.mod h1:AqkLNAfUm0K07J28hnAyyQKf/x0YkCY/g5DCtuL01Mw= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= +github.com/aohorodnyk/mimeheader v0.0.6 h1:WCV4NQjtbqnd2N3FT5MEPesan/lfvaLYmt5v4xSaX/M= +github.com/aohorodnyk/mimeheader v0.0.6/go.mod h1:/Gd3t3vszyZYwjNJo2qDxoftZjjVzMdkQZxkiINp3vM= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/attestantio/go-builder-client v0.4.3-0.20240124194555-d44db06f45fa h1:Kj6d1tXAA+EAi7fK8z8NakBEpY4WYzZMuCmLZjwBpTM= github.com/attestantio/go-builder-client v0.4.3-0.20240124194555-d44db06f45fa/go.mod h1:e02i/WO4fjs3/u9oIZEjiC8CK1Qyxy4cpiMMGKx4VqQ= diff --git a/services/api/service.go b/services/api/service.go index d5ac4a0a..49fe2a56 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -20,6 +20,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" @@ -930,6 +931,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 // --------------- @@ -1205,6 +1246,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( @@ -1312,6 +1359,24 @@ 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 + } + + // future updates will allow SSZ but for now we only + // allow JSON request bodies. + if ct := req.Header.Get("Content-Type"); ct != "" { + switch ct { + case "application/json": + 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 { diff --git a/services/api/service_test.go b/services/api/service_test.go index 0e82fa93..36a86e4f 100644 --- a/services/api/service_test.go +++ b/services/api/service_test.go @@ -1379,3 +1379,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) + } + }) + } +} diff --git a/services/api/utils.go b/services/api/utils.go index 55948ee2..cc7e4b14 100644 --- a/services/api/utils.go +++ b/services/api/utils.go @@ -23,6 +23,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 { From 1a32af25ff9758f1136162a1585640ec6b78a82b Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Mon, 10 Feb 2025 14:45:35 -0800 Subject: [PATCH 2/7] fixup: clearer Content-Type comment --- services/api/service.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/api/service.go b/services/api/service.go index 49fe2a56..15b6ca55 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -1358,15 +1358,16 @@ func (api *RelayAPI) handleGetPayload(w http.ResponseWriter, req *http.Request) float64(time.Since(receivedAt).Milliseconds()), ) }() - + // 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 } - // future updates will allow SSZ but for now we only - // allow JSON request bodies. + // 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 "application/json": From 50dba39943b73deabaa288a401debaca29a02a65 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Mon, 10 Feb 2025 15:00:30 -0800 Subject: [PATCH 3/7] fixup: GHA lint differs from local one. --- services/api/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/service.go b/services/api/service.go index 15b6ca55..986aec25 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -1358,7 +1358,7 @@ func (api *RelayAPI) handleGetPayload(w http.ResponseWriter, req *http.Request) float64(time.Since(receivedAt).Milliseconds()), ) }() - + // TODO: Use NegotiateRequestResponseType, for now we only accept JSON if !RequestAcceptsJSON(req) { api.RespondError(w, http.StatusNotAcceptable, "only Accept: application/json is currently supported") From e421b4a9278cd45a2007da8c54a79f358393244c Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Wed, 12 Feb 2025 14:07:43 -0800 Subject: [PATCH 4/7] Update services/api/service.go Co-authored-by: Justin Traglia <95511699+jtraglia@users.noreply.github.com> --- services/api/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/service.go b/services/api/service.go index 986aec25..27ec9dbd 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -1370,7 +1370,7 @@ func (api *RelayAPI) handleGetPayload(w http.ResponseWriter, req *http.Request) // request bodies. if ct := req.Header.Get("Content-Type"); ct != "" { switch ct { - case "application/json": + case ApplicationJSON: break default: api.RespondError(w, http.StatusUnsupportedMediaType, "only Content-Type: application/json is currently supported") From 92f6cc662765d73dfdbffab0fb95a3a9275fc408 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 19 Feb 2025 09:34:48 -0600 Subject: [PATCH 5/7] Fix ErrNotAcceptable indentation --- services/api/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/api/utils.go b/services/api/utils.go index 940493f8..2903169a 100644 --- a/services/api/utils.go +++ b/services/api/utils.go @@ -25,7 +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") + ErrNotAcceptable = errors.New("not acceptable") ) func SanityCheckBuilderBlockSubmission(payload *common.VersionedSubmitBlockRequest) error { From 99062c7ef7f33773a74510ee853c19c712c21e01 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 19 Feb 2025 09:37:30 -0600 Subject: [PATCH 6/7] Run go mod tidy --- go.sum | 4 ---- 1 file changed, 4 deletions(-) diff --git a/go.sum b/go.sum index f64ad242..a3fd86be 100644 --- a/go.sum +++ b/go.sum @@ -9,16 +9,12 @@ 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/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= -github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/aohorodnyk/mimeheader v0.0.6 h1:WCV4NQjtbqnd2N3FT5MEPesan/lfvaLYmt5v4xSaX/M= github.com/aohorodnyk/mimeheader v0.0.6/go.mod h1:/Gd3t3vszyZYwjNJo2qDxoftZjjVzMdkQZxkiINp3vM= -github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= 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= github.com/attestantio/go-eth2-client v0.22.1-0.20250106164842-07b6ce39bb43/go.mod h1:vy5jU/uDZ2+RcVzq5BfnG+bQ3/6uu9DGwCrGsPtjJ1A= -github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g= github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= From 3f866de1da0b5bc9f7847b3d4d113ca9e5e73a83 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 19 Feb 2025 09:40:24 -0600 Subject: [PATCH 7/7] Add content type check for registerValidators --- .golangci.yaml | 1 + services/api/service.go | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/.golangci.yaml b/.golangci.yaml index cfa60f9d..b186ca1e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -29,6 +29,7 @@ linters: - interfacebloat - exhaustruct - tenv + - gocognit # # Disabled because of generics: diff --git a/services/api/service.go b/services/api/service.go index c48df911..2f0bdc1d 100644 --- a/services/api/service.go +++ b/services/api/service.go @@ -1004,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