-
Notifications
You must be signed in to change notification settings - Fork 42
Security: block private script source targets #656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
7e73f5e
e3d7651
7a07741
3752faa
2377091
f52ff21
986d367
5201d83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "net" | ||
| "net/http" | ||
| "net/url" | ||
| "os" | ||
|
|
@@ -35,16 +36,21 @@ func (f ScriptSourceResolverFunc) Resolve(ctx context.Context, source sources.So | |
| } | ||
|
|
||
| type defaultScriptSourceResolver struct { | ||
| client *http.Client | ||
| env map[string]string | ||
| client *http.Client | ||
| env map[string]string | ||
| validateNetworkTarget func(context.Context, *url.URL) error | ||
| } | ||
|
|
||
| // NewDefaultScriptSourceResolver returns the bounded file and HTTP(S) resolver | ||
| // used by CLI compose loading. | ||
| func NewDefaultScriptSourceResolver(env map[string]string) ScriptSourceResolver { | ||
| resolver := &defaultScriptSourceResolver{env: env} | ||
| resolver := &defaultScriptSourceResolver{env: env, validateNetworkTarget: validateScriptNetworkTarget} | ||
| transport := http.DefaultTransport.(*http.Transport).Clone() | ||
| transport.Proxy = nil | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 移除环境代理后,依赖正向代理出网的环境将无法拉取 HTTP(S) 脚本源且无明确诊断或恢复开关改动将 transport.Proxy 置为 nil 并删除 scriptSourceTransport 封装。对可直连公网的环境行为不变;但在必须经正向代理出网的环境(进程设置了 HTTP_PROXY/HTTPS_PROXY,且防火墙阻断直连)中,HTTP(S) 脚本源拉取会从原来的“经代理成功”变成“直连超时/连接被拒”。基线代码的注释与专测(TestSafeScriptDialContextAllowsConfiguredProxyEndpoint)表明这一场景此前是被有意支持的。现在既没有配置开关/白名单可恢复,用户侧也只看到通用的 dial 错误(如 i/o timeout),无法得知是脚本源有意禁用了代理,排障成本高。 Problem code: Recommendation: |
||
| transport.DialContext = safeScriptDialContext | ||
| resolver.client = &http.Client{ | ||
| Timeout: defaultScriptSourceTimeout, | ||
| Timeout: defaultScriptSourceTimeout, | ||
| Transport: transport, | ||
| CheckRedirect: func(req *http.Request, via []*http.Request) error { | ||
| if len(via) > maxScriptSourceRedirects { | ||
| return fmt.Errorf("too many redirects (maximum %d)", maxScriptSourceRedirects) | ||
|
|
@@ -61,7 +67,7 @@ func NewDefaultScriptSourceResolver(env map[string]string) ScriptSourceResolver | |
| if len(via) > 0 && via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme == "http" { | ||
| return errors.New("HTTPS redirect downgrade to HTTP is not allowed") | ||
| } | ||
| return nil | ||
| return resolver.validateNetworkTarget(req.Context(), req.URL) | ||
| }, | ||
| } | ||
| return resolver | ||
|
|
@@ -234,6 +240,9 @@ func readScriptFile(path string) ([]byte, error) { | |
| } | ||
|
|
||
| func (r *defaultScriptSourceResolver) readHTTP(ctx context.Context, location *url.URL, source sources.Source) ([]byte, error) { | ||
| if err := r.validateNetworkTarget(ctx, location); err != nil { | ||
| return nil, fmt.Errorf("fetch script from %s: %w", redactedScriptURL(location), err) | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, location.String(), nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("create script request for %s", redactedScriptURL(location)) | ||
|
|
@@ -250,6 +259,62 @@ func (r *defaultScriptSourceResolver) readHTTP(ctx context.Context, location *ur | |
| return readLimitedScript(resp.Body) | ||
| } | ||
|
|
||
| func validateScriptNetworkTarget(ctx context.Context, target *url.URL) error { | ||
| host := strings.TrimSpace(target.Hostname()) | ||
| if host == "" { | ||
| return errors.New("script URL requires a valid host") | ||
| } | ||
| addresses, err := net.DefaultResolver.LookupIPAddr(ctx, host) | ||
| if err != nil { | ||
| return fmt.Errorf("resolve script URL host: %w", err) | ||
| } | ||
| if len(addresses) == 0 { | ||
| return errors.New("script URL host resolved to no addresses") | ||
| } | ||
| for _, address := range addresses { | ||
| if !isPublicScriptAddress(address.IP) { | ||
| return fmt.Errorf("script URL host resolves to prohibited address %s", address.IP) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func safeScriptDialContext(ctx context.Context, network, address string) (net.Conn, error) { | ||
| host, port, err := net.SplitHostPort(address) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("parse script endpoint: %w", err) | ||
| } | ||
| addresses, err := net.DefaultResolver.LookupIPAddr(ctx, host) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("resolve script endpoint: %w", err) | ||
| } | ||
| dialer := net.Dialer{} | ||
| var dialErrs error | ||
| for _, candidate := range addresses { | ||
| if !isPublicScriptAddress(candidate.IP) { | ||
| continue | ||
| } | ||
| conn, dialErr := dialer.DialContext(ctx, network, net.JoinHostPort(candidate.IP.String(), port)) | ||
| if dialErr == nil { | ||
| return conn, nil | ||
| } | ||
| dialErrs = errors.Join(dialErrs, dialErr) | ||
| } | ||
| if dialErrs != nil { | ||
| return nil, dialErrs | ||
| } | ||
| return nil, errors.New("script endpoint has no permitted public address") | ||
| } | ||
|
monkeyscan[bot] marked this conversation as resolved.
|
||
|
|
||
| func isPublicScriptAddress(ip net.IP) bool { | ||
| if ip == nil || !ip.IsGlobalUnicast() || ip.IsPrivate() || ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { | ||
| return false | ||
| } | ||
| // Go's IsPrivate intentionally excludes the carrier-grade NAT range. | ||
| cgnat := &net.IPNet{IP: net.IPv4(100, 64, 0, 0), Mask: net.CIDRMask(10, 32)} | ||
| return !cgnat.Contains(ip) | ||
| } | ||
|
monkeyscan[bot] marked this conversation as resolved.
|
||
|
|
||
| func readLimitedScript(reader io.Reader) ([]byte, error) { | ||
| data, err := io.ReadAll(io.LimitReader(reader, maxScriptSourceBytes+1)) | ||
| if err != nil { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.