From e38efd3cfaa55682358cedadd719bc5f1dea038b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 20 Mar 2023 08:52:52 +0100 Subject: [PATCH] Add ACL test for limiting a single port. (#1258) --- ...-integration-v2-TestACLAllowUser80Dst.yaml | 57 ++++++++ integration/acl_test.go | 99 ++++++++++++++ integration/tailscale.go | 1 + integration/tsic/tsic.go | 128 +++++++++++++++++- 4 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/test-integration-v2-TestACLAllowUser80Dst.yaml diff --git a/.github/workflows/test-integration-v2-TestACLAllowUser80Dst.yaml b/.github/workflows/test-integration-v2-TestACLAllowUser80Dst.yaml new file mode 100644 index 00000000..2e6c3c2e --- /dev/null +++ b/.github/workflows/test-integration-v2-TestACLAllowUser80Dst.yaml @@ -0,0 +1,57 @@ +# DO NOT EDIT, generated with cmd/gh-action-integration-generator/main.go +# To regenerate, run "go generate" in cmd/gh-action-integration-generator/ + +name: Integration Test v2 - TestACLAllowUser80Dst + +on: [pull_request] + +concurrency: + group: ${{ github.workflow }}-$${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 2 + + - name: Get changed files + id: changed-files + uses: tj-actions/changed-files@v34 + with: + files: | + *.nix + go.* + **/*.go + integration_test/ + config-example.yaml + + - uses: cachix/install-nix-action@v18 + if: ${{ env.ACT }} || steps.changed-files.outputs.any_changed == 'true' + + - name: Run general integration tests + if: steps.changed-files.outputs.any_changed == 'true' + run: | + nix develop --command -- docker run \ + --tty --rm \ + --volume ~/.cache/hs-integration-go:/go \ + --name headscale-test-suite \ + --volume $PWD:$PWD -w $PWD/integration \ + --volume /var/run/docker.sock:/var/run/docker.sock \ + --volume $PWD/control_logs:/tmp/control \ + golang:1 \ + go test ./... \ + -tags ts2019 \ + -failfast \ + -timeout 120m \ + -parallel 1 \ + -run "^TestACLAllowUser80Dst$" + + - uses: actions/upload-artifact@v3 + if: always() && steps.changed-files.outputs.any_changed == 'true' + with: + name: logs + path: "control_logs/*.log" diff --git a/integration/acl_test.go b/integration/acl_test.go index 7e3eaa5f..ed32a642 100644 --- a/integration/acl_test.go +++ b/integration/acl_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "testing" "github.com/juanfont/headscale" @@ -9,6 +10,44 @@ import ( "github.com/stretchr/testify/assert" ) +const numberOfTestClients = 2 + +func aclScenario(t *testing.T, policy headscale.ACLPolicy) *Scenario { + t.Helper() + scenario, err := NewScenario() + assert.NoError(t, err) + + spec := map[string]int{ + "user1": numberOfTestClients, + "user2": numberOfTestClients, + } + + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{ + tsic.WithDockerEntrypoint([]string{ + "/bin/bash", + "-c", + "/bin/sleep 3 ; update-ca-certificates ; python3 -m http.server 80 & tailscaled --tun=tsdev", + }), + tsic.WithDockerWorkdir("/"), + }, + hsic.WithACLPolicy(&policy), + hsic.WithTestName("acldenyallping"), + ) + assert.NoError(t, err) + + // allClients, err := scenario.ListTailscaleClients() + // assert.NoError(t, err) + + err = scenario.WaitForTailscaleSync() + assert.NoError(t, err) + + _, err = scenario.ListTailscaleClientsFQDNs() + assert.NoError(t, err) + + return scenario +} + // This tests a different ACL mechanism, if a host _cannot_ connect // to another node at all based on ACL, it should just not be part // of the NetMap sent to the host. This is slightly different than @@ -179,3 +218,63 @@ func TestACLHostsInNetMapTable(t *testing.T) { }) } } + +// Test to confirm that we can use user:80 from one user +// This should make the node appear in the peer list, but +// disallow ping. +// This ACL will not allow user1 access its own machines. +// Reported: https://github.com/juanfont/headscale/issues/699 +func TestACLAllowUser80Dst(t *testing.T) { + IntegrationSkip(t) + + scenario := aclScenario(t, + headscale.ACLPolicy{ + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"user1"}, + Destinations: []string{"user2:80"}, + }, + }, + }, + ) + + user1Clients, err := scenario.ListTailscaleClients("user1") + assert.NoError(t, err) + + user2Clients, err := scenario.ListTailscaleClients("user2") + assert.NoError(t, err) + + // Test that user1 can visit all user2 + for _, client := range user1Clients { + for _, peer := range user2Clients { + fqdn, err := peer.FQDN() + assert.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Len(t, result, 13) + assert.NoError(t, err) + } + } + + // Test that user2 _cannot_ visit user1 + for _, client := range user2Clients { + for _, peer := range user1Clients { + fqdn, err := peer.FQDN() + assert.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Empty(t, result) + assert.Error(t, err) + } + } + + err = scenario.Shutdown() + assert.NoError(t, err) +} diff --git a/integration/tailscale.go b/integration/tailscale.go index 5a7697a7..fbb1d3e5 100644 --- a/integration/tailscale.go +++ b/integration/tailscale.go @@ -24,5 +24,6 @@ type TailscaleClient interface { WaitForLogout() error WaitForPeers(expected int) error Ping(hostnameOrIP string, opts ...tsic.PingOption) error + Curl(url string, opts ...tsic.CurlOption) (string, error) ID() string } diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 07579735..907bd0d1 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -55,6 +55,8 @@ type TailscaleInContainer struct { headscaleHostname string withSSH bool withTags []string + withEntrypoint []string + workdir string } // Option represent optional settings that can be given to a @@ -115,6 +117,24 @@ func WithSSH() Option { } } +// WithDockerWorkdir allows the docker working directory to be set. +func WithDockerWorkdir(dir string) Option { + return func(tsic *TailscaleInContainer) { + tsic.workdir = dir + } +} + +// WithDockerEntrypoint allows the docker entrypoint of the container +// to be overridden. This is a dangerous option which can make +// the container not work as intended as a typo might prevent +// tailscaled and other processes from starting. +// Use with caution. +func WithDockerEntrypoint(args []string) Option { + return func(tsic *TailscaleInContainer) { + tsic.withEntrypoint = args + } +} + // New returns a new TailscaleInContainer instance. func New( pool *dockertest.Pool, @@ -135,6 +155,12 @@ func New( pool: pool, network: network, + + withEntrypoint: []string{ + "/bin/bash", + "-c", + "/bin/sleep 3 ; update-ca-certificates ; tailscaled --tun=tsdev", + }, } for _, opt := range opts { @@ -147,11 +173,7 @@ func New( // Cmd: []string{ // "tailscaled", "--tun=tsdev", // }, - Entrypoint: []string{ - "/bin/bash", - "-c", - "/bin/sleep 3 ; update-ca-certificates ; tailscaled --tun=tsdev", - }, + Entrypoint: tsic.withEntrypoint, } if tsic.headscaleHostname != "" { @@ -161,6 +183,10 @@ func New( } } + if tsic.workdir != "" { + tailscaleOptions.WorkingDir = tsic.workdir + } + // dockertest isnt very good at handling containers that has already // been created, this is an attempt to make sure this container isnt // present. @@ -520,6 +546,98 @@ func (t *TailscaleInContainer) Ping(hostnameOrIP string, opts ...PingOption) err }) } +type ( + // CurlOption repreent optional settings that can be given + // to curl another host. + CurlOption = func(args *curlArgs) + + curlArgs struct { + connectionTimeout time.Duration + maxTime time.Duration + retry int + retryDelay time.Duration + retryMaxTime time.Duration + } +) + +// WithCurlConnectionTimeout sets the timeout for each connection started +// by curl. +func WithCurlConnectionTimeout(timeout time.Duration) CurlOption { + return func(args *curlArgs) { + args.connectionTimeout = timeout + } +} + +// WithCurlMaxTime sets the max time for a transfer for each connection started +// by curl. +func WithCurlMaxTime(t time.Duration) CurlOption { + return func(args *curlArgs) { + args.maxTime = t + } +} + +// WithCurlRetry sets the number of retries a connection is attempted by curl. +func WithCurlRetry(ret int) CurlOption { + return func(args *curlArgs) { + args.retry = ret + } +} + +const ( + defaultConnectionTimeout = 3 * time.Second + defaultMaxTime = 10 * time.Second + defaultRetry = 5 + defaultRetryDelay = 0 * time.Second + defaultRetryMaxTime = 50 * time.Second +) + +// Curl executes the Tailscale curl command and curls a hostname +// or IP. It accepts a series of CurlOption. +func (t *TailscaleInContainer) Curl(url string, opts ...CurlOption) (string, error) { + args := curlArgs{ + connectionTimeout: defaultConnectionTimeout, + maxTime: defaultMaxTime, + retry: defaultRetry, + retryDelay: defaultRetryDelay, + retryMaxTime: defaultRetryMaxTime, + } + + for _, opt := range opts { + opt(&args) + } + + command := []string{ + "curl", + "--silent", + "--connect-timeout", fmt.Sprintf("%d", int(args.connectionTimeout.Seconds())), + "--max-time", fmt.Sprintf("%d", int(args.maxTime.Seconds())), + "--retry", fmt.Sprintf("%d", args.retry), + "--retry-delay", fmt.Sprintf("%d", int(args.retryDelay.Seconds())), + "--retry-max-time", fmt.Sprintf("%d", int(args.retryMaxTime.Seconds())), + url, + } + + var result string + err := t.pool.Retry(func() error { + var err error + result, _, err = t.Execute(command) + if err != nil { + log.Printf( + "failed to run curl command from %s to %s, err: %s", + t.Hostname(), + url, + err, + ) + + return err + } + + return nil + }) + + return result, err +} + // WriteFile save file inside the Tailscale container. func (t *TailscaleInContainer) WriteFile(path string, data []byte) error { return integrationutil.WriteFileToContainer(t.pool, t.container, path, data)