diff --git a/cli/command/publish/publish.go b/cli/command/publish/publish.go index a002c5d..f73329b 100644 --- a/cli/command/publish/publish.go +++ b/cli/command/publish/publish.go @@ -31,7 +31,7 @@ import ( ) const ( - maxReadmeSize = 50 * 1024 // 50KB + maxReadmeSize = 100 * 1024 // 100KB maxPackedSize int64 = 128 * 1024 * 1024 // 128MB ) @@ -118,7 +118,7 @@ func getReadme(dirPath string) (string, error) { _ = f.Close() }() - // Limit readme size to maxReadmeSize i.e. 50KB + // Limit readme size to maxReadmeSize. data, err := io.ReadAll(io.LimitReader(f, maxReadmeSize)) if err != nil { return "", err diff --git a/pkg/api/client_options.go b/pkg/api/client_options.go index 44ee27a..093a006 100644 --- a/pkg/api/client_options.go +++ b/pkg/api/client_options.go @@ -1,7 +1,6 @@ package api import ( - "errors" "io" "time" ) @@ -42,23 +41,3 @@ type ClientOptions struct { // CacheDir specifies a directory to use for caching GET requests. CacheDir string } - -func optionsNeedResolution(opts ClientOptions) bool { - if opts.Host == "" { - return true - } - - if opts.AuthToken == "" { - return true - } - - return false -} - -func resolveOptions(opts ClientOptions) (ClientOptions, error) { - if opts.Host == "" { - return ClientOptions{}, errors.New("host not found") - } - - return opts, nil -} diff --git a/pkg/api/errors.go b/pkg/api/errors.go index 27dcb13..ad1701f 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -24,7 +24,7 @@ func (err *HTTPError) Error() string { if err.Message == "" { return "wpm registry error: " + strings.ToLower(http.StatusText(err.StatusCode)) } - return "wpm registry error: " + strings.ToLower(err.Message) + return "wpm registry error: " + err.Message } // HandleHTTPError parses a http.Response into a HTTPError. The response body diff --git a/pkg/api/http_client.go b/pkg/api/http_client.go index 1d09eb6..39efa82 100644 --- a/pkg/api/http_client.go +++ b/pkg/api/http_client.go @@ -1,9 +1,12 @@ package api import ( + "errors" "fmt" "io" + "net" "net/http" + "net/url" "regexp" "strings" "sync" @@ -29,15 +32,20 @@ const ( HeaderIfModifiedSince = "If-Modified-Since" HeaderCacheRevalidate = "X-Cache-Revalidate" HeaderContentEncoding = "Content-Encoding" + HeaderContentLength = "Content-Length" HeaderContentType = "Content-Type" HeaderCacheControl = "Cache-Control" HeaderAccept = "Accept" + HeaderAcceptEncoding = "Accept-Encoding" HeaderAuthorization = "Authorization" HeaderUserAgent = "User-Agent" + HeaderExpect = "Expect" // header values CacheHit = "HIT" CacheMiss = "MISS" + + encodingZstd = "zstd" ) var ( @@ -54,12 +62,15 @@ var ( ) func NewHTTPClient(opts ClientOptions) (*http.Client, error) { - if optionsNeedResolution(opts) { - var err error - opts, err = resolveOptions(opts) - if err != nil { - return nil, err - } + base, err := normalizeBaseURL(opts.Host) + if err != nil { + return nil, err + } + host := base.Hostname() + + allowToken := base.Scheme == "https" || isLoopbackHost(host) + if opts.AuthToken != "" && !allowToken { + return nil, fmt.Errorf("refusing to send auth token over http to %q: use https", base.Host) } // Sweep stale cache tmp files left behind by aborted requests. Runs in @@ -72,11 +83,19 @@ func NewHTTPClient(opts ClientOptions) (*http.Client, error) { transport := &Transport{ Base: &http.Transport{ - MaxIdleConns: 100, - MaxIdleConnsPerHost: 100, - IdleConnTimeout: 90 * time.Second, - ForceAttemptHTTP2: true, - DisableCompression: true, + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + MaxIdleConns: 100, + MaxIdleConnsPerHost: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: 30 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + ForceAttemptHTTP2: true, + DisableCompression: true, }, cacheDir: opts.CacheDir, } @@ -90,11 +109,8 @@ func NewHTTPClient(opts ClientOptions) (*http.Client, error) { } var rt http.RoundTripper = transport - - rt = newHeaderRoundTripper(opts.Host, opts.AuthToken, opts.Headers, rt) rt = newDecompressingRoundTripper(rt) rt = newSanitizerRoundTripper(rt) - if opts.Log != nil && zerolog.GlobalLevel() == zerolog.DebugLevel { opts.LogVerboseHTTP = true logger := &httpretty.Logger{ @@ -114,14 +130,67 @@ func NewHTTPClient(opts ClientOptions) (*http.Client, error) { }) rt = logger.RoundTripper(rt) } - - return &http.Client{Transport: rt, Timeout: opts.Timeout}, nil + rt = newHeaderRoundTripper(host, allowToken, opts.AuthToken, opts.Headers, rt) + + return &http.Client{ + Transport: rt, + Timeout: opts.Timeout, + CheckRedirect: func(req *http.Request, _ []*http.Request) error { + if !isSameDomain(req.URL.Hostname(), host) { + return fmt.Errorf("refusing redirect to %q outside registry host %q", req.URL.Host, host) + } + return nil + }, + }, nil } func inspectableMIMEType(t string) bool { return jsonTypeRE.MatchString(t) } +// normalizeBaseURL canonicalizes a configured registry host into a base URL +// with an explicit scheme. A bare host (no scheme) defaults to https so we +// never silently fall back to cleartext. +func normalizeBaseURL(host string) (*url.URL, error) { + host = strings.TrimSpace(host) + if host == "" { + return nil, errors.New("registry host not configured") + } + + lc := strings.ToLower(host) + if !strings.HasPrefix(lc, "http://") && !strings.HasPrefix(lc, "https://") { + host = "https://" + host + } + + u, err := url.Parse(host) + if err != nil { + return nil, fmt.Errorf("invalid registry host %q: %w", host, err) + } + if u.Hostname() == "" { + return nil, fmt.Errorf("invalid registry host %q: missing hostname", host) + } + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("unsupported registry scheme %q, only http and https are supported", u.Scheme) + } + + u.Fragment = "" + u.RawQuery = "" + u.Path = strings.TrimRight(u.Path, "/") + return u, nil +} + +// isLoopbackHost reports whether host refers to the local machine, where +// cleartext traffic never touches the network. +func isLoopbackHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + if ip := net.ParseIP(host); ip != nil { + return ip.IsLoopback() + } + return false +} + func isSameDomain(requestHost, domain string) bool { requestHost = strings.ToLower(requestHost) domain = strings.ToLower(domain) @@ -129,9 +198,10 @@ func isSameDomain(requestHost, domain string) bool { } type headerRoundTripper struct { - headers map[string]string - host string - rt http.RoundTripper + headers map[string]string + host string + allowToken bool + rt http.RoundTripper } func resolveHeaders(headers map[string]string) { @@ -146,29 +216,43 @@ func resolveHeaders(headers map[string]string) { } } -func newHeaderRoundTripper(host, authToken string, headers map[string]string, rt http.RoundTripper) http.RoundTripper { +func newHeaderRoundTripper(host string, allowToken bool, authToken string, headers map[string]string, rt http.RoundTripper) http.RoundTripper { if _, ok := headers[HeaderAuthorization]; !ok && authToken != "" { headers[HeaderAuthorization] = "Bearer " + authToken } if len(headers) == 0 { - return headerRoundTripper{host: host, headers: nil, rt: rt} + headers = nil } - return headerRoundTripper{host: host, headers: headers, rt: rt} + return headerRoundTripper{host: host, allowToken: allowToken, headers: headers, rt: rt} } func (hrt headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { reqCopy := req.Clone(req.Context()) - reqCopy.Header.Set("Accept-Encoding", "zstd") + reqCopy.Header.Set(HeaderAcceptEncoding, encodingZstd) for k, v := range hrt.headers { - if k == HeaderAuthorization && !isSameDomain(reqCopy.URL.Hostname(), hrt.host) { - continue + if k == HeaderAuthorization { + continue // handled separately below } if reqCopy.Header.Get(k) == "" { reqCopy.Header.Set(k, v) } } + // The auth token only travels to the configured registry host over an + // allowed transport. On anything else (cross-host redirect, plaintext to a + // remote host) strip it, including any token a caller set per-request, so + // the credential cannot leak. + if hrt.allowToken && isSameDomain(reqCopy.URL.Hostname(), hrt.host) { + if reqCopy.Header.Get(HeaderAuthorization) == "" { + if token := hrt.headers[HeaderAuthorization]; token != "" { + reqCopy.Header.Set(HeaderAuthorization, token) + } + } + } else { + reqCopy.Header.Del(HeaderAuthorization) + } + return hrt.rt.RoundTrip(reqCopy) } @@ -214,7 +298,7 @@ func (d decompressingRoundTripper) RoundTrip(req *http.Request) (*http.Response, return nil, err } - if resp.Header.Get("Content-Encoding") == "zstd" { + if resp.Header.Get(HeaderContentEncoding) == encodingZstd { decoder := zstdDecoderPool.Get().(*zstd.Decoder) if err := decoder.Reset(resp.Body); err != nil { _ = resp.Body.Close() @@ -228,8 +312,8 @@ func (d decompressingRoundTripper) RoundTrip(req *http.Request) (*http.Response, Decoder: decoder, OriginalBody: resp.Body, } - resp.Header.Del("Content-Encoding") - resp.Header.Del("Content-Length") + resp.Header.Del(HeaderContentLength) + resp.Header.Del(HeaderContentEncoding) resp.ContentLength = -1 } diff --git a/pkg/api/rest_client.go b/pkg/api/rest_client.go index 70cbed2..6e1b3f5 100644 --- a/pkg/api/rest_client.go +++ b/pkg/api/rest_client.go @@ -3,29 +3,24 @@ package api import ( "context" "encoding/json" + "fmt" "io" "net/http" + "net/url" "strings" - - "go.wpm.so/cli/pkg/unsafeconv" ) -type RESTClient struct { - client *http.Client - host string -} +const maxResponseBodySize = 4 << 20 // 4 MiB -func DefaultRESTClient() (*RESTClient, error) { - return NewRESTClient(ClientOptions{}) +type RESTClient struct { + client *http.Client + baseURL *url.URL } func NewRESTClient(opts ClientOptions) (*RESTClient, error) { - if optionsNeedResolution(opts) { - var err error - opts, err = resolveOptions(opts) - if err != nil { - return nil, err - } + base, err := normalizeBaseURL(opts.Host) + if err != nil { + return nil, err } client, err := NewHTTPClient(opts) @@ -33,11 +28,9 @@ func NewRESTClient(opts ClientOptions) (*RESTClient, error) { return nil, err } - host := strings.TrimRight(opts.Host, "/") - return &RESTClient{ - client: client, - host: host, + baseURL: base, + client: client, }, nil } @@ -49,17 +42,18 @@ func WithHeader(key, value string) RequestOption { } } +func WithContentLength(length int64) RequestOption { + return func(req *http.Request) { + req.ContentLength = length + } +} + func (c *RESTClient) DoWithContext(ctx context.Context, method, path string, body io.Reader, response any, opts ...RequestOption) error { - url := restURL(c.host, path) - req, err := http.NewRequestWithContext(ctx, method, url, body) + req, err := c.newRequest(ctx, method, path, body, opts...) if err != nil { return err } - for _, opt := range opts { - opt(req) - } - resp, err := c.client.Do(req) if err != nil { return err @@ -79,35 +73,30 @@ func (c *RESTClient) DoWithContext(ctx context.Context, method, path string, bod switch v := response.(type) { case *string: - var b []byte - b, err = io.ReadAll(resp.Body) - if err == nil { - *v = unsafeconv.UnsafeBytesToString(b) + b, rErr := readAtMost(resp.Body, maxResponseBodySize) + if rErr != nil { + return rErr } - return err + *v = string(b) + return nil case *[]byte: - var b []byte - b, err = io.ReadAll(resp.Body) - if err == nil { - *v = b + b, rErr := readAtMost(resp.Body, maxResponseBodySize) + if rErr != nil { + return rErr } - return err + *v = b + return nil default: - return json.NewDecoder(resp.Body).Decode(v) + return json.NewDecoder(io.LimitReader(resp.Body, maxResponseBodySize)).Decode(v) } } func (c *RESTClient) RequestStream(ctx context.Context, method, path string, body io.Reader, opts ...RequestOption) (io.ReadCloser, error) { - url := restURL(c.host, path) - req, err := http.NewRequestWithContext(ctx, method, url, body) + req, err := c.newRequest(ctx, method, path, body, opts...) if err != nil { return nil, err } - for _, opt := range opts { - opt(req) - } - resp, err := c.client.Do(req) if err != nil { return nil, err @@ -133,16 +122,32 @@ func drainBody(body io.Reader) { _, _ = io.Copy(io.Discard, io.LimitReader(body, 64<<10)) } -func restURL(hostname, pathOrURL string) string { - if strings.HasPrefix(pathOrURL, "https://") || strings.HasPrefix(pathOrURL, "http://") { - return pathOrURL +// readAtMost reads limited bytes and returns an error if the limit +// is exceeded, to prevent OOM from hostile or buggy servers. +func readAtMost(r io.Reader, limit int64) ([]byte, error) { + b, err := io.ReadAll(io.LimitReader(r, limit+1)) + if err != nil { + return nil, err } - if !strings.HasPrefix(pathOrURL, "/") { - pathOrURL = "/" + pathOrURL + if int64(len(b)) > limit { + return nil, fmt.Errorf("response body exceeds %d byte limit", limit) } - return restPrefix(hostname) + pathOrURL + return b, nil } -func restPrefix(hostname string) string { - return "https://" + hostname +func (c *RESTClient) newRequest(ctx context.Context, method, path string, body io.Reader, opts ...RequestOption) (*http.Request, error) { + if strings.Contains(path, "://") { + return nil, fmt.Errorf("path %q must be relative, not an absolute URL", path) + } + + req, err := http.NewRequestWithContext(ctx, method, c.baseURL.JoinPath(path).String(), body) + if err != nil { + return nil, err + } + + for _, opt := range opts { + opt(req) + } + + return req, nil } diff --git a/pkg/pm/registry/client.go b/pkg/pm/registry/client.go index 9081c7e..a2f55a6 100644 --- a/pkg/pm/registry/client.go +++ b/pkg/pm/registry/client.go @@ -1,9 +1,11 @@ package registry import ( + "bytes" "context" - "encoding/base64" + "encoding/binary" "encoding/json" + "fmt" "io" "net/http" @@ -13,7 +15,8 @@ import ( ) const ( - wpmManifestHeader = "X-WPM-Manifest" + continue100 = "100-continue" + maxManifestSize = 256 * 1024 // 256KB contentTypeOctetStream = "application/octet-stream" wpmContentTypeManifestV1 = "application/vnd.wpm.install-v1+json" ) @@ -62,14 +65,31 @@ func (c *client) PutPackage(ctx context.Context, data *manifest.Package, tarball return err } + manifestLen := len(manifestBytes) + if manifestLen > maxManifestSize { + return fmt.Errorf("manifest size exceeds %d bytes, refusing to continue", maxManifestSize) + } + + lengthBytes := make([]byte, 4) + binary.BigEndian.PutUint32(lengthBytes, uint32(manifestLen)) + + bodyReader := io.MultiReader( + bytes.NewReader(lengthBytes), + bytes.NewReader(manifestBytes), + tarball, + ) + + totalContentLength := int64(4+manifestLen) + data.Dist.PackedSize + return c.restClient.DoWithContext( ctx, http.MethodPut, - "/", - tarball, + "/"+data.Name+"/"+data.Version, + bodyReader, nil, + api.WithHeader(api.HeaderExpect, continue100), api.WithHeader(api.HeaderContentType, contentTypeOctetStream), - api.WithHeader(wpmManifestHeader, base64.StdEncoding.EncodeToString(manifestBytes)), + api.WithContentLength(totalContentLength), ) }