TUN-7259: Add warning for missing ingress rules

Providing no ingress rules in the configuration file or via the CLI will now provide a warning and return 502 for all incoming HTTP requests.

CHANGES.md

@@ -1,3 +1,7 @@
+## 2023.3.1
+### Breaking Change
+- Running a tunnel without ingress rules defined in configuration file nor from the CLI flags will no longer provide a default ingress rule to localhost:8080 and instead will return HTTP response code 502 for all incoming HTTP requests.
+
 ## 2023.2.2
 ### Notices
 - Legacy tunnels were officially deprecated on December 1, 2022. Starting with this version, cloudflared no longer supports connecting legacy tunnels.

cmd/cloudflared/tunnel/configuration.go

@@ -227,22 +227,10 @@ func prepareTunnelConfig(
 		Arch:     info.OSArch(),
 	}
 	cfg := config.GetConfiguration()
-	ingressRules, err := ingress.ParseIngress(cfg)
-	if err != nil && err != ingress.ErrNoIngressRules {
+	ingressRules, err := ingress.ParseIngressFromConfigAndCLI(cfg, c, log)
+	if err != nil {
 		return nil, nil, err
 	}
-	if c.IsSet("url") || c.IsSet(ingress.HelloWorldFlag) || c.IsSet(config.BastionFlag) {
-		// Ingress rules cannot be provided with --url, --hello-world or --bastion flag
-		if !ingressRules.IsEmpty() {
-			return nil, nil, ingress.ErrURLIncompatibleWithIngress
-		}
-		// Only for quick or adhoc tunnels will we attempt to parse:
-		// --url, --hello-world, --bastion, or --unix-socket flag for a tunnel ingress rule
-		ingressRules, err = ingress.NewSingleOrigin(c, false)
-		if err != nil {
-			return nil, nil, err
-		}
-	}
 
 	protocolSelector, err := connection.NewProtocolSelector(transportProtocol, namedTunnel.Credentials.AccountTag, c.IsSet(TunnelTokenFlag), c.Bool("post-quantum"), edgediscovery.ProtocolPercentage, connection.ResolveTTL, log)
 	if err != nil {

component-tests/config.py

@@ -30,6 +30,7 @@ class NamedTunnelBaseConfig(BaseConfig):
     tunnel: str = None
     credentials_file: str = None
     ingress: list = None
+    hostname: str = None
 
     def __post_init__(self):
         if self.tunnel is None:
@@ -63,7 +64,7 @@ class NamedTunnelConfig(NamedTunnelBaseConfig):
                            self.merge_config(additional_config))
 
     def get_url(self):
-        return "https://" + self.ingress[0]['hostname']
+        return "https://" + self.hostname
 
     def base_config(self):
         config = self.full_config.copy()

component-tests/conftest.py

@@ -26,7 +26,7 @@ def component_tests_config():
         config = yaml.safe_load(stream)
         LOGGER.info(f"component tests base config {config}")
 
-        def _component_tests_config(additional_config={}, cfd_mode=CfdModes.NAMED, run_proxy_dns=True):
+        def _component_tests_config(additional_config={}, cfd_mode=CfdModes.NAMED, run_proxy_dns=True, provide_ingress=True):
             if run_proxy_dns:
                 # Regression test for TUN-4177, running with proxy-dns should not prevent tunnels from running.
                 # So we run all tests with it.
@@ -36,12 +36,21 @@ def component_tests_config():
                 additional_config.pop("proxy-dns", None)
                 additional_config.pop("proxy-dns-port", None)
 
+            # Allows the ingress rules to be omitted from the provided config
+            ingress = []
+            if provide_ingress:
+                ingress = config['ingress']
+
+            # Provide the hostname to allow routing to the tunnel even if the ingress rule isn't defined in the config
+            hostname = config['ingress'][0]['hostname']
+
             if cfd_mode is CfdModes.NAMED:
                 return NamedTunnelConfig(additional_config=additional_config,
                                          cloudflared_binary=config['cloudflared_binary'],
                                          tunnel=config['tunnel'],
                                          credentials_file=config['credentials_file'],
-                                         ingress=config['ingress'])
+                                         ingress=ingress,
+                                         hostname=hostname)
             elif cfd_mode is CfdModes.PROXY_DNS:
                 return ProxyDnsConfig(cloudflared_binary=config['cloudflared_binary'])
             elif cfd_mode is CfdModes.QUICK:

component-tests/test_quicktunnels.py

@@ -1,10 +1,9 @@
 #!/usr/bin/env python
-import requests
 from conftest import CfdModes
 from constants import METRICS_PORT
 from util import LOGGER, start_cloudflared, wait_tunnel_ready, get_quicktunnel_url, send_requests
 
-class TestCLI:
+class TestQuickTunnels:
     def test_quick_tunnel(self, tmp_path, component_tests_config):
         config = component_tests_config(cfd_mode=CfdModes.QUICK, run_proxy_dns=False)
         LOGGER.debug(config)

component-tests/test_tunnel.py

@@ -0,0 +1,43 @@
+#!/usr/bin/env python
+import requests
+from conftest import CfdModes
+from constants import METRICS_PORT, MAX_RETRIES, BACKOFF_SECS
+from retrying import retry
+from util import LOGGER, start_cloudflared, wait_tunnel_ready, send_requests
+
+class TestTunnel:
+    '''Test tunnels with no ingress rules from config.yaml but ingress rules from CLI only'''
+
+    def test_tunnel_hello_world(self, tmp_path, component_tests_config):
+        config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False)
+        LOGGER.debug(config)
+        with start_cloudflared(tmp_path, config, cfd_args=["run", "--hello-world"], new_process=True):
+            wait_tunnel_ready(tunnel_url=config.get_url(),
+                              require_min_connections=4)
+    
+    def test_tunnel_url(self, tmp_path, component_tests_config):
+        config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False)
+        LOGGER.debug(config)
+        with start_cloudflared(tmp_path, config, cfd_args=["run", "--url", f"http://localhost:{METRICS_PORT}/"], new_process=True):
+            wait_tunnel_ready(require_min_connections=4)
+            send_requests(config.get_url()+"/ready", 3, True)
+
+    def test_tunnel_no_ingress(self, tmp_path, component_tests_config):
+        '''
+        Running a tunnel with no ingress rules provided from either config.yaml or CLI will still work but return 502
+        for all incoming requests.
+        '''
+        config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False)
+        LOGGER.debug(config)
+        with start_cloudflared(tmp_path, config, cfd_args=["run"], new_process=True):
+            wait_tunnel_ready(require_min_connections=4)
+            resp = send_request(config.get_url()+"/")
+            assert resp.status_code == 502, "Expected cloudflared to return 502 for all requests with no ingress defined"
+            resp = send_request(config.get_url()+"/test")
+            assert resp.status_code == 502, "Expected cloudflared to return 502 for all requests with no ingress defined"
+
+
+@retry(stop_max_attempt_number=MAX_RETRIES, wait_fixed=BACKOFF_SECS * 1000)
+def send_request(url):
+    with requests.Session() as s:
+        return s.get(url, timeout=BACKOFF_SECS)

ingress/config.go

@@ -113,7 +113,7 @@ func (rc *RemoteConfig) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
-func originRequestFromSingeRule(c *cli.Context) OriginRequestConfig {
+func originRequestFromSingleRule(c *cli.Context) OriginRequestConfig {
 	var connectTimeout = defaultHTTPConnectTimeout
 	var tlsTimeout = defaultTLSTimeout
 	var tcpKeepAlive = defaultTCPKeepAlive

ingress/config_test.go

@@ -411,7 +411,7 @@ func TestDefaultConfigFromCLI(t *testing.T) {
 		KeepAliveTimeout:     defaultKeepAliveTimeout,
 		ProxyAddress:         defaultProxyAddress,
 	}
-	actual := originRequestFromSingeRule(c)
+	actual := originRequestFromSingleRule(c)
 	require.Equal(t, expected, actual)
 }
 

ingress/ingress.go

@@ -20,6 +20,7 @@ import (
 
 var (
 	ErrNoIngressRules             = errors.New("The config file doesn't contain any ingress rules")
+	ErrNoIngressRulesCLI          = errors.New("No ingress rules were defined in provided config (if any) nor from the cli, cloudflared will return 502 for all incoming HTTP requests")
 	errLastRuleNotCatchAll        = errors.New("The last ingress rule must match all URLs (i.e. it should not have a hostname or path filter)")
 	errBadWildcard                = errors.New("Hostname patterns can have at most one wildcard character (\"*\") and it can only be used for subdomains, e.g. \"*.example.com\"")
 	errHostnameContainsPort       = errors.New("Hostname cannot contain a port")
@@ -70,16 +71,52 @@ type Ingress struct {
 	Defaults OriginRequestConfig `json:"originRequest"`
 }
 
-// NewSingleOrigin constructs an Ingress set with only one rule, constructed from
-// CLI parameters for quick tunnels like --url or --no-chunked-encoding.
-func NewSingleOrigin(c *cli.Context, allowURLFromArgs bool) (Ingress, error) {
+// ParseIngress parses ingress rules, but does not send HTTP requests to the origins.
+func ParseIngress(conf *config.Configuration) (Ingress, error) {
+	if len(conf.Ingress) == 0 {
+		return Ingress{}, ErrNoIngressRules
+	}
+	return validateIngress(conf.Ingress, originRequestFromConfig(conf.OriginRequest))
+}
+
+// ParseIngressFromConfigAndCLI will parse the configuration rules from config files for ingress
+// rules and then attempt to parse CLI for ingress rules.
+// Will always return at least one valid ingress rule. If none are provided by the user, the default
+// will be to return 502 status code for all incoming requests.
+func ParseIngressFromConfigAndCLI(conf *config.Configuration, c *cli.Context, log *zerolog.Logger) (Ingress, error) {
+	// Attempt to parse ingress rules from configuration
+	ingressRules, err := ParseIngress(conf)
+	if err == nil && !ingressRules.IsEmpty() {
+		return ingressRules, nil
+	}
+	if err != ErrNoIngressRules {
+		return Ingress{}, err
+	}
+	// Attempt to parse ingress rules from CLI:
+	//   --url or --unix-socket flag for a tunnel HTTP ingress
+	//   --hello-world for a basic HTTP ingress self-served
+	//   --bastion for ssh bastion service
+	ingressRules, err = parseCLIIngress(c, false)
+	if errors.Is(err, ErrNoIngressRulesCLI) {
+		log.Warn().Msgf(ErrNoIngressRulesCLI.Error())
+		return newDefaultOrigin(c, log), nil
+	}
+	if err != nil {
+		return Ingress{}, err
+	}
+	return ingressRules, nil
+}
+
+// parseCLIIngress constructs an Ingress set with only one rule constructed from
+// CLI parameters: --url, --hello-world, --bastion, or --unix-socket
+func parseCLIIngress(c *cli.Context, allowURLFromArgs bool) (Ingress, error) {
 	service, err := parseSingleOriginService(c, allowURLFromArgs)
 	if err != nil {
 		return Ingress{}, err
 	}
 
 	// Construct an Ingress with the single rule.
-	defaults := originRequestFromSingeRule(c)
+	defaults := originRequestFromSingleRule(c)
 	ing := Ingress{
 		Rules: []Rule{
 			{
@@ -92,6 +129,22 @@ func NewSingleOrigin(c *cli.Context, allowURLFromArgs bool) (Ingress, error) {
 	return ing, err
 }
 
+// newDefaultOrigin always returns a 502 response code to help indicate that there are no ingress
+// rules setup, but the tunnel is reachable.
+func newDefaultOrigin(c *cli.Context, log *zerolog.Logger) Ingress {
+	noRulesService := newDefaultStatusCode(log)
+	defaults := originRequestFromSingleRule(c)
+	ingress := Ingress{
+		Rules: []Rule{
+			{
+				Service: &noRulesService,
+			},
+		},
+		Defaults: defaults,
+	}
+	return ingress
+}
+
 // WarpRoutingService starts a tcp stream between the origin and requests from
 // warp clients.
 type WarpRoutingService struct {
@@ -137,8 +190,7 @@ func parseSingleOriginService(c *cli.Context, allowURLFromArgs bool) (OriginServ
 		}
 		return &unixSocketPath{path: path, scheme: "http"}, nil
 	}
-	u, err := url.Parse("http://localhost:8080")
-	return &httpService{url: u}, err
+	return nil, ErrNoIngressRulesCLI
 }
 
 // IsEmpty checks if there are any ingress rules.
@@ -335,14 +387,6 @@ func (e errRuleShouldNotBeCatchAll) Error() string {
 		"will never be triggered.", e.index+1, e.hostname)
 }
 
-// ParseIngress parses ingress rules, but does not send HTTP requests to the origins.
-func ParseIngress(conf *config.Configuration) (Ingress, error) {
-	if len(conf.Ingress) == 0 {
-		return Ingress{}, ErrNoIngressRules
-	}
-	return validateIngress(conf.Ingress, originRequestFromConfig(conf.OriginRequest))
-}
-
 func isHTTPService(url *url.URL) bool {
 	return url.Scheme == "http" || url.Scheme == "https" || url.Scheme == "ws" || url.Scheme == "wss"
 }

ingress/ingress_test.go

@@ -43,7 +43,7 @@ ingress:
 	require.Equal(t, "https", s.scheme)
 }
 
-func Test_parseIngress(t *testing.T) {
+func TestParseIngress(t *testing.T) {
 	localhost8000 := MustParseURL(t, "https://localhost:8000")
 	localhost8001 := MustParseURL(t, "https://localhost:8001")
 	fourOhFour := newStatusCode(404)
@@ -517,7 +517,7 @@ func TestSingleOriginSetsConfig(t *testing.T) {
 
 	allowURLFromArgs := false
 	require.NoError(t, err)
-	ingress, err := NewSingleOrigin(cliCtx, allowURLFromArgs)
+	ingress, err := parseCLIIngress(cliCtx, allowURLFromArgs)
 	require.NoError(t, err)
 
 	assert.Equal(t, config.CustomDuration{Duration: time.Second}, ingress.Rules[0].Config.ConnectTimeout)
@@ -537,6 +537,119 @@ func TestSingleOriginSetsConfig(t *testing.T) {
 	assert.Equal(t, socksProxy, ingress.Rules[0].Config.ProxyType)
 }
 
+func TestSingleOriginServices(t *testing.T) {
+	host := "://localhost:8080"
+	httpURL := urlMustParse("http" + host)
+	tcpURL := urlMustParse("tcp" + host)
+	unix := "unix://service"
+	newCli := func(params ...string) *cli.Context {
+		flagSet := flag.NewFlagSet(t.Name(), flag.PanicOnError)
+		flagSet.Bool("hello-world", false, "")
+		flagSet.Bool("bastion", false, "")
+		flagSet.String("url", "", "")
+		flagSet.String("unix-socket", "", "")
+		cliCtx := cli.NewContext(cli.NewApp(), flagSet, nil)
+		for i := 0; i < len(params); i += 2 {
+			cliCtx.Set(params[i], params[i+1])
+		}
+
+		return cliCtx
+	}
+
+	tests := []struct {
+		name            string
+		cli             *cli.Context
+		expectedService OriginService
+		err             error
+	}{
+		{
+			name:            "Valid hello-world",
+			cli:             newCli("hello-world", "true"),
+			expectedService: &helloWorld{},
+		},
+		{
+			name:            "Valid bastion",
+			cli:             newCli("bastion", "true"),
+			expectedService: newBastionService(),
+		},
+		{
+			name:            "Valid http url",
+			cli:             newCli("url", httpURL.String()),
+			expectedService: &httpService{url: httpURL},
+		},
+		{
+			name:            "Valid tcp url",
+			cli:             newCli("url", tcpURL.String()),
+			expectedService: newTCPOverWSService(tcpURL),
+		},
+		{
+			name:            "Valid unix-socket",
+			cli:             newCli("unix-socket", unix),
+			expectedService: &unixSocketPath{path: unix, scheme: "http"},
+		},
+		{
+			name: "No origins defined",
+			cli:  newCli(),
+			err:  ErrNoIngressRulesCLI,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ingress, err := parseCLIIngress(test.cli, false)
+			require.Equal(t, err, test.err)
+			if test.err != nil {
+				return
+			}
+			require.Equal(t, 1, len(ingress.Rules))
+			rule := ingress.Rules[0]
+			require.Equal(t, test.expectedService, rule.Service)
+		})
+	}
+}
+
+func urlMustParse(s string) *url.URL {
+	u, err := url.Parse(s)
+	if err != nil {
+		panic(err)
+	}
+	return u
+}
+
+func TestSingleOriginServices_URL(t *testing.T) {
+	host := "://localhost:8080"
+	newCli := func(param string, value string) *cli.Context {
+		flagSet := flag.NewFlagSet(t.Name(), flag.PanicOnError)
+		flagSet.String("url", "", "")
+		cliCtx := cli.NewContext(cli.NewApp(), flagSet, nil)
+		cliCtx.Set(param, value)
+		return cliCtx
+	}
+
+	httpTests := []string{"http", "https"}
+	for _, test := range httpTests {
+		t.Run(test, func(t *testing.T) {
+			url := urlMustParse(test + host)
+			ingress, err := parseCLIIngress(newCli("url", url.String()), false)
+			require.NoError(t, err)
+			require.Equal(t, 1, len(ingress.Rules))
+			rule := ingress.Rules[0]
+			require.Equal(t, &httpService{url: url}, rule.Service)
+		})
+	}
+
+	tcpTests := []string{"ssh", "rdp", "smb", "tcp"}
+	for _, test := range tcpTests {
+		t.Run(test, func(t *testing.T) {
+			url := urlMustParse(test + host)
+			ingress, err := parseCLIIngress(newCli("url", url.String()), false)
+			require.NoError(t, err)
+			require.Equal(t, 1, len(ingress.Rules))
+			rule := ingress.Rules[0]
+			require.Equal(t, newTCPOverWSService(url), rule.Service)
+		})
+	}
+}
+
 func TestFindMatchingRule(t *testing.T) {
 	ingress := Ingress{
 		Rules: []Rule{

ingress/origin_proxy.go

@@ -44,6 +44,9 @@ func (o *httpService) RoundTrip(req *http.Request) (*http.Response, error) {
 }
 
 func (o *statusCode) RoundTrip(_ *http.Request) (*http.Response, error) {
+	if o.defaultResp {
+		o.log.Warn().Msgf(ErrNoIngressRulesCLI.Error())
+	}
 	resp := &http.Response{
 		StatusCode: o.code,
 		Status:     fmt.Sprintf("%d %s", o.code, http.StatusText(o.code)),

ingress/origin_service.go

@@ -247,13 +247,26 @@ func (o helloWorld) MarshalJSON() ([]byte, error) {
 // Typical use-case is "user wants the catch-all rule to just respond 404".
 type statusCode struct {
 	code int
+
+	// Set only when the user has not defined any ingress rules
+	defaultResp bool
+	log         *zerolog.Logger
 }
 
 func newStatusCode(status int) statusCode {
 	return statusCode{code: status}
 }
 
+// default status code (502) that is returned for requests to cloudflared that don't have any ingress rules setup
+func newDefaultStatusCode(log *zerolog.Logger) statusCode {
+	return statusCode{code: 502, defaultResp: true, log: log}
+}
+
 func (o *statusCode) String() string {
+	// returning a different service name can help with identifying users via config that don't setup ingress rules
+	if o.defaultResp {
+		return "default_http_status:502"
+	}
 	return fmt.Sprintf("http_status:%d", o.code)
 }
 

proxy/proxy_test.go

@@ -149,8 +149,7 @@ func TestProxySingleOrigin(t *testing.T) {
 	err := cliCtx.Set("hello-world", "true")
 	require.NoError(t, err)
 
-	allowURLFromArgs := false
-	ingressRule, err := ingress.NewSingleOrigin(cliCtx, allowURLFromArgs)
+	ingressRule, err := ingress.ParseIngressFromConfigAndCLI(&config.Configuration{}, cliCtx, &log)
 	require.NoError(t, err)
 
 	require.NoError(t, ingressRule.StartOrigins(&log, ctx.Done()))