You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Outbound HTTP client layout: strict protections are not available as an *http.Client #4619
All strict-mode protections for outbound HTTP live on http/client.StrictHTTPClient, which applies them per request in Do: the public-URL check (core.ParsePublicURLAllowIP), the User-Agent header, and the 1 MiB response body limit. The timeout and redirect policy sit on the *http.Client inside it, and the SSRF dial guard and TLS 1.2 floor sit on SafeHttpTransport below that. StrictHTTPClient is not an *http.Client and does not expose the one it wraps.
That is fine for code we write ourselves (auth, discovery, vcr, vdr/didweb all call client.New* and use Do). It is a problem for every library or generated client that insists on a *http.Client:
jsonld/ldutils.go: json-gold's DefaultDocumentLoader takes a *http.Client. Until JSON-LD: time out remote context fetches and remember failures #4617 it got nil, meaning http.DefaultClient: no timeout, no SSRF guard, no body limit. JSON-LD: time out remote context fetches and remember failures #4617 fixes it by wrapping the whole StrictHTTPClient as an http.RoundTripper inside a second, outer http.Client. This works, but the outer client's Timeout and CheckRedirect are dead config, and the adapter bends the RoundTripper contract (it follows redirects and buffers the body inside RoundTrip).
pki/validator.go:104: CRL fetching builds a raw http.Client on http.DefaultTransport. CRL distribution points come from certificates we did not issue, so this is an outbound fetch to URLs we do not control, with no SSRF guard, no body limit and no strict-mode URL check. Only the timeout is set.
crypto/storage/external/client.go:96: raw http.Client on http.DefaultTransport. This one targets an operator-configured internal address, so the strict client would be wrong for it, but it also means there is no single place that says "this is the client for trusted internal targets".
core/http_client.go:131 (CreateHTTPInternalClient): raw http.Client, used for CLI-to-node calls. Same category as the previous one.
The comment on SafeHttpTransport already warns not to build raw clients on it, but the package offers no alternative when a *http.Client is required, so people either bypass the protections or write an adapter like the one in #4617.
Proposal
Move the per-request protections from StrictHTTPClient.Do into an http.RoundTripper in http/client, so they apply per hop and a real *http.Client can be built on it:
A strictTransport that wraps SafeHttpTransport (or the caching or custom-TLS variant) and does, per hop: the public-URL check, set User-Agent, and limit the response body. Redirect targets are then checked by the transport on every hop as well, which makes the current checkRedirect re-check redundant but harmless.
client.New* keep returning *StrictHTTPClient so the existing call sites do not change. StrictHTTPClient.Do becomes a thin call to the inner client.
A new constructor, for example client.NewHTTPClient(timeout, options...) *http.Client, that returns the inner client directly for libraries that need one. Timeout and CheckRedirect are set on that client, so there is one place to configure them.
jsonld drops its private adapter and uses NewHTTPClient. pki switches CRL fetching to it (with a response size option sized for CRLs; some are several MB).
Document in http/client which constructor to use for which target class: public internet (strict), operator-configured internal services (no SSRF guard, but still timeout and body limit), and CLI-to-node.
Response limit semantics
The response limit is a buffer-and-error, not a streaming cap: Do reads up to limit+1 bytes into memory and fails the request on overflow. It never truncates, because a silently shortened body (DER, a JSON-LD context) can parse into something smaller than intended. The transport must keep that: RoundTrip reads the body, errors on overflow, and returns an in-memory body on success. http.Client.Do then surfaces the error exactly as StrictHTTPClient.Do does today, so callers never see a response object on overflow.
Two consequences: http.Client wraps transport errors in *url.Error, so tests comparing error strings change (errors.Is/errors.As keep working), and redirect hops are limited as well, which is stricter than today. A lazy limiting body would allow streaming but moves the error from Do to the first read, which is a behaviour change for every existing caller, so it is out of scope here.
Non-goals
Changing behaviour for the existing client.New* callers.
Touching the generated OpenAPI clients; they take an HttpRequestDoer interface and already accept StrictHTTPClient.
Unifying the two internal-target clients (crypto/storage/external, core.CreateHTTPInternalClient); documenting which constructor they should use is enough.
Open questions
Tracing: getTransport wraps in otelhttp today. The strict transport should sit inside the otel wrapper so a rejected URL still produces a span, or outside so it does not; decide which.
Problem
All strict-mode protections for outbound HTTP live on
http/client.StrictHTTPClient, which applies them per request inDo: the public-URL check (core.ParsePublicURLAllowIP), the User-Agent header, and the 1 MiB response body limit. The timeout and redirect policy sit on the*http.Clientinside it, and the SSRF dial guard and TLS 1.2 floor sit onSafeHttpTransportbelow that.StrictHTTPClientis not an*http.Clientand does not expose the one it wraps.That is fine for code we write ourselves (auth, discovery, vcr, vdr/didweb all call
client.New*and useDo). It is a problem for every library or generated client that insists on a*http.Client:jsonld/ldutils.go: json-gold'sDefaultDocumentLoadertakes a*http.Client. Until JSON-LD: time out remote context fetches and remember failures #4617 it gotnil, meaninghttp.DefaultClient: no timeout, no SSRF guard, no body limit. JSON-LD: time out remote context fetches and remember failures #4617 fixes it by wrapping the wholeStrictHTTPClientas anhttp.RoundTripperinside a second, outerhttp.Client. This works, but the outer client'sTimeoutandCheckRedirectare dead config, and the adapter bends theRoundTrippercontract (it follows redirects and buffers the body insideRoundTrip).pki/validator.go:104: CRL fetching builds a rawhttp.Clientonhttp.DefaultTransport. CRL distribution points come from certificates we did not issue, so this is an outbound fetch to URLs we do not control, with no SSRF guard, no body limit and no strict-mode URL check. Only the timeout is set.crypto/storage/external/client.go:96: rawhttp.Clientonhttp.DefaultTransport. This one targets an operator-configured internal address, so the strict client would be wrong for it, but it also means there is no single place that says "this is the client for trusted internal targets".core/http_client.go:131(CreateHTTPInternalClient): rawhttp.Client, used for CLI-to-node calls. Same category as the previous one.The comment on
SafeHttpTransportalready warns not to build raw clients on it, but the package offers no alternative when a*http.Clientis required, so people either bypass the protections or write an adapter like the one in #4617.Proposal
Move the per-request protections from
StrictHTTPClient.Dointo anhttp.RoundTripperinhttp/client, so they apply per hop and a real*http.Clientcan be built on it:strictTransportthat wrapsSafeHttpTransport(or the caching or custom-TLS variant) and does, per hop: the public-URL check, set User-Agent, and limit the response body. Redirect targets are then checked by the transport on every hop as well, which makes the currentcheckRedirectre-check redundant but harmless.client.New*keep returning*StrictHTTPClientso the existing call sites do not change.StrictHTTPClient.Dobecomes a thin call to the inner client.client.NewHTTPClient(timeout, options...) *http.Client, that returns the inner client directly for libraries that need one. Timeout andCheckRedirectare set on that client, so there is one place to configure them.jsonlddrops its private adapter and usesNewHTTPClient.pkiswitches CRL fetching to it (with a response size option sized for CRLs; some are several MB).http/clientwhich constructor to use for which target class: public internet (strict), operator-configured internal services (no SSRF guard, but still timeout and body limit), and CLI-to-node.Response limit semantics
The response limit is a buffer-and-error, not a streaming cap:
Doreads up to limit+1 bytes into memory and fails the request on overflow. It never truncates, because a silently shortened body (DER, a JSON-LD context) can parse into something smaller than intended. The transport must keep that:RoundTripreads the body, errors on overflow, and returns an in-memory body on success.http.Client.Dothen surfaces the error exactly asStrictHTTPClient.Dodoes today, so callers never see a response object on overflow.Two consequences:
http.Clientwraps transport errors in*url.Error, so tests comparing error strings change (errors.Is/errors.Askeep working), and redirect hops are limited as well, which is stricter than today. A lazy limiting body would allow streaming but moves the error fromDoto the first read, which is a behaviour change for every existing caller, so it is out of scope here.Non-goals
client.New*callers.HttpRequestDoerinterface and already acceptStrictHTTPClient.crypto/storage/external,core.CreateHTTPInternalClient); documenting which constructor they should use is enough.Open questions
getTransportwraps inotelhttptoday. The strict transport should sit inside the otel wrapper so a rejected URL still produces a span, or outside so it does not; decide which.Related