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

Fix: Can't use Unix Sockets in Quick Tunnel mode #1367

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
15 changes: 11 additions & 4 deletions cmd/cloudflared/tunnel/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ const (

tunnelCmdErrorMessage = `You did not specify any valid additional argument to the cloudflared tunnel command.

If you are trying to run a Quick Tunnel then you need to explicitly pass the --url flag.
Eg. cloudflared tunnel --url localhost:8080/.
If you are trying to run a Quick Tunnel then you need to explicitly pass a --url or --unix-socket flag.
Eg. 'cloudflared tunnel --url localhost:8080/' or 'cloudflared tunnel --unix-socket /tmp/socket'.

Please note that Quick Tunnels are meant to be ephemeral and should only be used for testing purposes.
For production usage, we recommend creating Named Tunnels. (https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/install-and-setup/tunnel-guide/)
Expand Down Expand Up @@ -286,7 +286,14 @@ See https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/in
}
}

// This is so that we can mock QuickTunnelRunner for TunnelCommand test cases
type QuickTunnelRunner func(*subcommandContext) error

func TunnelCommand(c *cli.Context) error {
return tunnelCommandImpl(c, RunQuickTunnel)
}

func tunnelCommandImpl(c *cli.Context, quickTunnelRunner QuickTunnelRunner) error {
sc, err := newSubcommandContext(c)
if err != nil {
return err
Expand All @@ -313,9 +320,9 @@ func TunnelCommand(c *cli.Context) error {
// Run a quick tunnel
// A unauthenticated named tunnel hosted on <random>.<quick-tunnels-service>.com
// We don't support running proxy-dns and a quick tunnel at the same time as the same process
shouldRunQuickTunnel := c.IsSet("url") || c.IsSet(ingress.HelloWorldFlag)
shouldRunQuickTunnel := c.IsSet("url") || c.IsSet("unix-socket") || c.IsSet(ingress.HelloWorldFlag)
Copy link
Author

@rhyswilliamsza rhyswilliamsza Dec 10, 2024

Choose a reason for hiding this comment

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

I haven't done any side-effect analysis to check whether anyone could be relying on --unix-socket forcing a trapdoor to proxy-dns, but that seems unlikely so it should be okay.

Choose a reason for hiding this comment

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

I tried going through the code.
In TunnelCommand function in cmd.go, if --name is not specified, only two non failure scenarios exists - quick tunnel or proxy dns.
And in case of proxy-dns, I did not found a possible use of --unix-socket anywhere, as much I could understand the code flow.

So, looks good.

if !c.IsSet("proxy-dns") && c.String("quick-service") != "" && shouldRunQuickTunnel {
return RunQuickTunnel(sc)
return quickTunnelRunner(sc)
}

// If user provides a config, check to see if they meant to use `tunnel run` instead
Expand Down
74 changes: 74 additions & 0 deletions cmd/cloudflared/tunnel/cmd_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package tunnel

import (
"flag"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v2"
)

func TestHostnameFromURI(t *testing.T) {
Expand All @@ -15,3 +18,74 @@ func TestHostnameFromURI(t *testing.T) {
assert.Equal(t, "", hostnameFromURI("trash"))
assert.Equal(t, "", hostnameFromURI("https://awesomesauce.com"))
}

func TestShouldRunQuickTunnel(t *testing.T) {
Copy link
Author

Choose a reason for hiding this comment

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

I've scoped this specifically to quick tunnels, so it won't test falling through to other tunnel types.

tests := []struct {
name string
flags map[string]string
expectError bool
}{
{
name: "Quick tunnel with URL set",
flags: map[string]string{"url": "http://127.0.0.1:8080", "quick-service": "https://fakeapi.trycloudflare.com"},
expectError: false,
},
{
name: "Quick tunnel with unix-socket set",
flags: map[string]string{"unix-socket": "/tmp/socket", "quick-service": "https://fakeapi.trycloudflare.com"},
expectError: false,
},
{
name: "Quick tunnel with hello-world flag",
flags: map[string]string{"hello-world": "true", "quick-service": "https://fakeapi.trycloudflare.com"},
expectError: false,
},
{
name: "Quick tunnel with proxy-dns (invalid combo)",
flags: map[string]string{"url": "http://127.0.0.1:9090", "proxy-dns": "true", "quick-service": "https://fakeapi.trycloudflare.com"},
expectError: true,
},
{
name: "No quick-service set",
flags: map[string]string{"url": "http://127.0.0.1:9090"},
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Mock RunQuickTunnel Function
mockCalled := false
mockQuickTunnelRunner := func(sc *subcommandContext) error {
mockCalled = true
return nil
}

// Mock App Context
app := &cli.App{}
set := flagSetFromMap(tt.flags)
context := cli.NewContext(app, set, nil)

// Call TunnelCommand
err := tunnelCommandImpl(context, mockQuickTunnelRunner)

// Validate
if tt.expectError {
assert.False(t, mockCalled)
require.Error(t, err)
} else {
assert.True(t, mockCalled)
require.NoError(t, err)
}
})
}
}

func flagSetFromMap(flags map[string]string) *flag.FlagSet {
set := flag.NewFlagSet("test", 0)
for key, value := range flags {
set.String(key, "", "")
set.Set(key, value)
}
return set
}