Skip to content
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

introduce a flag to turn off SSRF protection for local development #16622

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gentle-lies-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#added Introduce DisableSSRFProtection flag for local development
2 changes: 2 additions & 0 deletions core/config/docs/core.toml
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,8 @@ OCRDevelopmentMode = false # Default
InfiniteDepthQueries = false # Default
# DisableRateLimiting skips ratelimiting on asset requests.
DisableRateLimiting = false # Default
# DisableSSRFProtection disables validation of IPs and URLs of outgoing requests.
DisableSSRFProtection = false # Default

[Tracing]
# Enabled turns trace collection on or off. On requires an OTEL Tracing Collector.
Expand Down
1 change: 1 addition & 0 deletions core/config/insecure_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ type Insecure interface {
DevWebServer() bool
OCRDevelopmentMode() bool
DisableRateLimiting() bool
DisableSSRFProtection() bool
InfiniteDepthQueries() bool
}
12 changes: 8 additions & 4 deletions core/config/toml/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1377,10 +1377,11 @@ func (s *Sentry) setFrom(f *Sentry) {
}

type Insecure struct {
DevWebServer *bool
OCRDevelopmentMode *bool
InfiniteDepthQueries *bool
DisableRateLimiting *bool
DevWebServer *bool
OCRDevelopmentMode *bool
InfiniteDepthQueries *bool
DisableRateLimiting *bool
DisableSSRFProtection *bool
}

func (ins *Insecure) ValidateConfig() (err error) {
Expand Down Expand Up @@ -1417,6 +1418,9 @@ func (ins *Insecure) setFrom(f *Insecure) {
if v := f.DisableRateLimiting; v != nil {
ins.DisableRateLimiting = f.DisableRateLimiting
}
if v := f.DisableSSRFProtection; v != nil {
ins.DisableSSRFProtection = f.DisableSSRFProtection
}
if v := f.OCRDevelopmentMode; v != nil {
ins.OCRDevelopmentMode = f.OCRDevelopmentMode
}
Expand Down
1 change: 1 addition & 0 deletions core/services/chainlink/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ func NewApplication(opts ApplicationOpts) (Application, error) {
legacyEVMChains,
keyStore.Eth()),
job.Gateway: gateway.NewDelegate(
cfg,
legacyEVMChains,
keyStore.Eth(),
opts.DS,
Expand Down
3 changes: 3 additions & 0 deletions core/services/chainlink/config_general_dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestTOMLGeneralConfig_DevModeInsecureConfig(t *testing.T) {

assert.False(t, config.Insecure().DevWebServer())
assert.False(t, config.Insecure().DisableRateLimiting())
assert.False(t, config.Insecure().DisableSSRFProtection())
assert.False(t, config.Insecure().InfiniteDepthQueries())
assert.False(t, config.Insecure().OCRDevelopmentMode())
})
Expand All @@ -32,13 +33,15 @@ func TestTOMLGeneralConfig_DevModeInsecureConfig(t *testing.T) {
OverrideFn: func(c *Config, s *Secrets) {
*c.Insecure.DevWebServer = true
*c.Insecure.DisableRateLimiting = true
*c.Insecure.DisableSSRFProtection = true
*c.Insecure.InfiniteDepthQueries = true
*c.Insecure.OCRDevelopmentMode = true
}}.New(logger.TestLogger(t))
require.NoError(t, err)

assert.True(t, config.Insecure().DevWebServer())
assert.True(t, config.Insecure().DisableRateLimiting())
assert.True(t, config.Insecure().DisableSSRFProtection())
assert.True(t, config.Insecure().InfiniteDepthQueries())
assert.True(t, config.OCRDevelopmentMode())
})
Expand Down
5 changes: 5 additions & 0 deletions core/services/chainlink/config_general_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func TestTOMLGeneralConfig_InsecureConfig(t *testing.T) {

assert.False(t, config.Insecure().DevWebServer())
assert.False(t, config.Insecure().DisableRateLimiting())
assert.False(t, config.Insecure().DisableSSRFProtection())
assert.False(t, config.Insecure().InfiniteDepthQueries())
assert.False(t, config.Insecure().OCRDevelopmentMode())
})
Expand All @@ -51,6 +52,7 @@ func TestTOMLGeneralConfig_InsecureConfig(t *testing.T) {
OverrideFn: func(c *Config, s *Secrets) {
*c.Insecure.DevWebServer = true
*c.Insecure.DisableRateLimiting = true
*c.Insecure.DisableSSRFProtection = true
*c.Insecure.InfiniteDepthQueries = true
*c.AuditLogger.Enabled = true
}}.New()
Expand All @@ -61,6 +63,8 @@ func TestTOMLGeneralConfig_InsecureConfig(t *testing.T) {

assert.False(t, config.Insecure().DevWebServer())
assert.False(t, config.Insecure().DisableRateLimiting())
assert.False(t, config.Insecure().DisableSSRFProtection())
assert.False(t, config.Insecure().DisableSSRFProtection())
assert.False(t, config.Insecure().InfiniteDepthQueries())
})

Expand All @@ -69,6 +73,7 @@ func TestTOMLGeneralConfig_InsecureConfig(t *testing.T) {
[insecure]
DevWebServer = true
DisableRateLimiting = false
DisableSSRFProtection = false
InfiniteDepthQueries = false
OCRDevelopmentMode = false
`
Expand Down
5 changes: 5 additions & 0 deletions core/services/chainlink/config_insecure.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ func (i *insecureConfig) DisableRateLimiting() bool {
*i.c.DisableRateLimiting
}

func (i *insecureConfig) DisableSSRFProtection() bool {
return build.IsDev() && i.c.DisableSSRFProtection != nil &&
*i.c.DisableSSRFProtection
}

func (i *insecureConfig) OCRDevelopmentMode() bool {
// OCRDevelopmentMode is allowed in TestBuilds as well
return (build.IsDev() || build.IsTest()) && i.c.OCRDevelopmentMode != nil &&
Expand Down
1 change: 1 addition & 0 deletions core/services/chainlink/config_insecure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func TestInsecureConfig(t *testing.T) {
ins := cfg.Insecure()
assert.False(t, ins.DevWebServer())
assert.False(t, ins.DisableRateLimiting())
assert.False(t, ins.DisableSSRFProtection())
assert.False(t, ins.OCRDevelopmentMode())
assert.False(t, ins.InfiniteDepthQueries())
}
10 changes: 6 additions & 4 deletions core/services/chainlink/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,11 @@ func TestConfig_Marshal(t *testing.T) {
RootDir: ptr("test/root/dir"),
ShutdownGracePeriod: commoncfg.MustNewDuration(10 * time.Second),
Insecure: toml.Insecure{
DevWebServer: ptr(false),
OCRDevelopmentMode: ptr(false),
InfiniteDepthQueries: ptr(false),
DisableRateLimiting: ptr(false),
DevWebServer: ptr(false),
OCRDevelopmentMode: ptr(false),
InfiniteDepthQueries: ptr(false),
DisableRateLimiting: ptr(false),
DisableSSRFProtection: ptr(false),
},
Tracing: toml.Tracing{
Enabled: ptr(true),
Expand Down Expand Up @@ -823,6 +824,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
1 change: 1 addition & 0 deletions core/services/chainlink/testdata/config-full.toml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
15 changes: 11 additions & 4 deletions core/services/gateway/delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,31 @@ import (

"github.com/smartcontractkit/chainlink-common/pkg/sqlutil"
"github.com/smartcontractkit/chainlink/v2/core/chains/legacyevm"
"github.com/smartcontractkit/chainlink/v2/core/config"
"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/config"
gatewayconfig "github.com/smartcontractkit/chainlink/v2/core/services/gateway/config"
"github.com/smartcontractkit/chainlink/v2/core/services/gateway/network"
"github.com/smartcontractkit/chainlink/v2/core/services/job"
"github.com/smartcontractkit/chainlink/v2/core/services/keystore"
)

type Delegate struct {
cfg Config
legacyChains legacyevm.LegacyChainContainer
ks keystore.Eth
ds sqlutil.DataSource
lggr logger.Logger
}

type Config interface {
Insecure() config.Insecure
}

var _ job.Delegate = (*Delegate)(nil)

func NewDelegate(legacyChains legacyevm.LegacyChainContainer, ks keystore.Eth, ds sqlutil.DataSource, lggr logger.Logger) *Delegate {
func NewDelegate(config Config, legacyChains legacyevm.LegacyChainContainer, ks keystore.Eth, ds sqlutil.DataSource, lggr logger.Logger) *Delegate {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will other fields from the config need to be accessed eventually? If not, does it make more sense to add a single boolean param to the constructor here instead? And maybe further along the param list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially other fields from the config may be needed in the future. Mostly just borrowed from the pattern in other delegates (code). Created a narrow interface here so that it doesn't access fields from anything other than Insecure config.

return &Delegate{
cfg: config,
legacyChains: legacyChains,
ks: ks,
ds: ds,
Expand All @@ -50,12 +57,12 @@ func (d *Delegate) ServicesForSpec(ctx context.Context, spec job.Job) (services
return nil, errors.Errorf("services.Delegate expects a *jobSpec.GatewaySpec to be present, got %v", spec)
}

var gatewayConfig config.GatewayConfig
var gatewayConfig gatewayconfig.GatewayConfig
err2 := json.Unmarshal(spec.GatewaySpec.GatewayConfig.Bytes(), &gatewayConfig)
if err2 != nil {
return nil, errors.Wrap(err2, "unmarshal gateway config")
}
httpClient, err := network.NewHTTPClient(gatewayConfig.HTTPClientConfig, d.lggr)
httpClient, err := network.NewHTTPClient(gatewayConfig.HTTPClientConfig, d.lggr, d.cfg.Insecure().DisableSSRFProtection())
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestIntegration_Gateway_NoFullNodes_BasicConnectionAndMessage(t *testing.T)
c, err := network.NewHTTPClient(network.HTTPClientConfig{
DefaultTimeout: 5 * time.Second,
MaxResponseBytes: 1000,
}, lggr)
}, lggr, false)
require.NoError(t, err)
gateway, err := gateway.NewGatewayFromConfig(parseGatewayConfig(t, gatewayConfig), gateway.NewHandlerFactory(nil, nil, c, lggr), lggr)
require.NoError(t, err)
Expand Down
41 changes: 28 additions & 13 deletions core/services/gateway/network/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,29 +77,44 @@
}

type httpClient struct {
client *safeurl.WrappedClient
client client
config HTTPClientConfig
lggr logger.Logger
}

type client interface {
Do(req *http.Request) (*http.Response, error)
}

// NewHTTPClient creates a new NewHTTPClient
// As of now, the client does not support TLS configuration but may be extended in the future
func NewHTTPClient(config HTTPClientConfig, lggr logger.Logger) (HTTPClient, error) {
// disableSSRFProtection is a flag to disable SSRF protection for testing purposes and should be set to FALSE for production
func NewHTTPClient(config HTTPClientConfig, lggr logger.Logger, disableSSRFProtection bool) (HTTPClient, error) {
config.ApplyDefaults()
safeConfig := safeurl.
GetConfigBuilder().
SetTimeout(config.DefaultTimeout).
SetAllowedIPs(config.AllowedIPs...).
SetAllowedPorts(config.AllowedPorts...).
SetAllowedSchemes(config.AllowedSchemes...).
SetBlockedIPs(config.BlockedIPs...).
SetBlockedIPsCIDR(config.BlockedIPsCIDR...).
SetCheckRedirect(disableRedirects).
Build()
var client client

Check failure on line 94 in core/services/gateway/network/httpclient.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint (.)

shadow: declaration of "client" shadows declaration at line 85 (govet)
if disableSSRFProtection {
lggr.Warn("SSRF protection is disabled. Please enable it in production.")
client = &http.Client{
Timeout: config.DefaultTimeout,
Transport: http.DefaultTransport,
}
} else {
safeConfig := safeurl.
GetConfigBuilder().
SetTimeout(config.DefaultTimeout).
SetAllowedIPs(config.AllowedIPs...).
SetAllowedPorts(config.AllowedPorts...).
SetAllowedSchemes(config.AllowedSchemes...).
SetBlockedIPs(config.BlockedIPs...).
SetBlockedIPsCIDR(config.BlockedIPsCIDR...).
SetCheckRedirect(disableRedirects).
Build()
client = safeurl.Client(safeConfig)
}

return &httpClient{
config: config,
client: safeurl.Client(safeConfig),
client: client,
lggr: lggr,
}, nil
}
Expand Down
4 changes: 2 additions & 2 deletions core/services/gateway/network/httpclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func TestHTTPClient_Send(t *testing.T) {
AllowedPorts: []int{int(portInt)},
}

client, err := NewHTTPClient(config, lggr)
client, err := NewHTTPClient(config, lggr, false)
require.NoError(t, err)

tt.request.URL = server.URL + tt.request.URL
Expand Down Expand Up @@ -399,7 +399,7 @@ func TestHTTPClient_BlocksUnallowed(t *testing.T) {
AllowedPorts: allowedPorts,
}

client, err := NewHTTPClient(config, lggr)
client, err := NewHTTPClient(config, lggr, false)
require.NoError(t, err)

require.NoError(t, err)
Expand Down
1 change: 1 addition & 0 deletions core/web/resolver/testdata/config-empty-effective.toml
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
1 change: 1 addition & 0 deletions core/web/resolver/testdata/config-full.toml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
7 changes: 7 additions & 0 deletions docs/CONFIG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,7 @@ DevWebServer = false # Default
OCRDevelopmentMode = false # Default
InfiniteDepthQueries = false # Default
DisableRateLimiting = false # Default
DisableSSRFProtection = false # Default
```
Insecure config family is only allowed in development builds.

Expand Down Expand Up @@ -1857,6 +1858,12 @@ DisableRateLimiting = false # Default
```
DisableRateLimiting skips ratelimiting on asset requests.

### DisableSSRFProtection
```toml
DisableSSRFProtection = false # Default
```
DisableSSRFProtection disables validation of IPs and URLs of outgoing requests.

## Tracing
```toml
[Tracing]
Expand Down
1 change: 1 addition & 0 deletions testdata/scripts/config/merge_raw_configs.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
1 change: 1 addition & 0 deletions testdata/scripts/node/validate/default.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
1 change: 1 addition & 0 deletions testdata/scripts/node/validate/defaults-override.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ DevWebServer = false
OCRDevelopmentMode = false
InfiniteDepthQueries = false
DisableRateLimiting = false
DisableSSRFProtection = false

[Tracing]
Enabled = false
Expand Down
Loading
Loading