From 52a323b90dc209bb545d7dfc5eca5af778231a9b Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Fri, 30 Sep 2022 20:44:23 +0200 Subject: [PATCH 01/19] Add SSH capability advertisement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advertises the SSH capability, and parses the SSH ACLs to pass to the tailscale client. Doesn’t support ‘autogroup’ ACL functionality. Co-authored-by: Daniel Brooks --- acls.go | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ acls_test.go | 73 +++++++++++++++++++++++++++++++ acls_types.go | 10 +++++ api_common.go | 1 + app.go | 1 + machine.go | 6 ++- 6 files changed, 206 insertions(+), 1 deletion(-) diff --git a/acls.go b/acls.go index 0b365c1f..bb8c4bd9 100644 --- a/acls.go +++ b/acls.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/rs/zerolog/log" "github.com/tailscale/hujson" @@ -120,6 +121,16 @@ func (h *Headscale) UpdateACLRules() error { log.Trace().Interface("ACL", rules).Msg("ACL rules generated") h.aclRules = rules + sshRules, err := h.generateSSHRules() + if err != nil { + return err + } + log.Trace().Interface("SSH", sshRules).Msg("SSH rules generated") + if h.sshPolicy == nil { + h.sshPolicy = &tailcfg.SSHPolicy{} + } + h.sshPolicy.Rules = sshRules + return nil } @@ -187,6 +198,111 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { return rules, nil } +func (h *Headscale) generateSSHRules() ([]*tailcfg.SSHRule, error) { + rules := []*tailcfg.SSHRule{} + + if h.aclPolicy == nil { + return nil, errEmptyPolicy + } + + machines, err := h.ListMachines() + if err != nil { + return nil, err + } + + acceptAction := tailcfg.SSHAction{ + Message: "", + Reject: false, + Accept: true, + SessionDuration: 0, + AllowAgentForwarding: false, + HoldAndDelegate: "", + AllowLocalPortForwarding: true, + } + + rejectAction := tailcfg.SSHAction{ + Message: "", + Reject: true, + Accept: false, + SessionDuration: 0, + AllowAgentForwarding: false, + HoldAndDelegate: "", + AllowLocalPortForwarding: false, + } + + for index, sshACL := range h.aclPolicy.SSHs { + action := rejectAction + switch sshACL.Action { + case "accept": + action = acceptAction + case "check": + checkAction, err := sshCheckAction(sshACL.CheckPeriod) + if err != nil { + log.Error(). + Msgf("Error parsing SSH %d, check action with unparsable duration '%s'", index, sshACL.CheckPeriod) + } else { + action = *checkAction + } + default: + log.Error(). + Msgf("Error parsing SSH %d, unknown action '%s'", index, sshACL.Action) + + return nil, err + } + + principals := make([]*tailcfg.SSHPrincipal, 0, len(sshACL.Sources)) + for innerIndex, rawSrc := range sshACL.Sources { + expandedSrcs, err := expandAlias( + machines, + *h.aclPolicy, + rawSrc, + h.cfg.OIDC.StripEmaildomain, + ) + if err != nil { + log.Error(). + Msgf("Error parsing SSH %d, Source %d", index, innerIndex) + + return nil, err + } + for _, expandedSrc := range expandedSrcs { + principals = append(principals, &tailcfg.SSHPrincipal{ + NodeIP: expandedSrc, + }) + } + } + + userMap := make(map[string]string, len(sshACL.Users)) + for _, user := range sshACL.Users { + userMap[user] = "=" + } + rules = append(rules, &tailcfg.SSHRule{ + RuleExpires: nil, + Principals: principals, + SSHUsers: userMap, + Action: &action, + }) + } + + return rules, nil +} + +func sshCheckAction(duration string) (*tailcfg.SSHAction, error) { + sessionLength, err := time.ParseDuration(duration) + if err != nil { + return nil, err + } + + return &tailcfg.SSHAction{ + Message: "", + Reject: false, + Accept: true, + SessionDuration: sessionLength, + AllowAgentForwarding: false, + HoldAndDelegate: "", + AllowLocalPortForwarding: true, + }, nil +} + func (h *Headscale) generateACLPolicySrcIP( machines []Machine, aclPolicy ACLPolicy, diff --git a/acls_test.go b/acls_test.go index 41f8d39d..0f3c262f 100644 --- a/acls_test.go +++ b/acls_test.go @@ -73,6 +73,79 @@ func (s *Suite) TestInvalidAction(c *check.C) { c.Assert(errors.Is(err, errInvalidAction), check.Equals, true) } +func (s *Suite) TestSshRules(c *check.C) { + namespace, err := app.CreateNamespace("user1") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("user1", "testmachine") + c.Assert(err, check.NotNil) + hostInfo := tailcfg.Hostinfo{ + OS: "centos", + Hostname: "testmachine", + RequestTags: []string{"tag:test"}, + } + + machine := Machine{ + ID: 0, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Hostname: "testmachine", + IPAddresses: MachineAddresses{netip.MustParseAddr("100.64.0.1")}, + NamespaceID: namespace.ID, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: HostInfo(hostInfo), + } + app.db.Save(&machine) + + app.aclPolicy = &ACLPolicy{ + Groups: Groups{ + "group:test": []string{"user1"}, + }, + Hosts: Hosts{ + "client": netip.PrefixFrom(netip.MustParseAddr("100.64.99.42"), 32), + }, + ACLs: []ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: []string{"group:test"}, + Destinations: []string{"client"}, + Users: []string{"autogroup:nonroot"}, + }, + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"client"}, + Users: []string{"autogroup:nonroot"}, + }, + }, + } + + err = app.UpdateACLRules() + + c.Assert(err, check.IsNil) + c.Assert(app.sshPolicy, check.NotNil) + c.Assert(app.sshPolicy.Rules, check.HasLen, 2) + c.Assert(app.sshPolicy.Rules[0].SSHUsers, check.HasLen, 1) + c.Assert(app.sshPolicy.Rules[0].Principals, check.HasLen, 1) + c.Assert(app.sshPolicy.Rules[0].Principals[0].NodeIP, check.Matches, "100.64.0.1") + + c.Assert(app.sshPolicy.Rules[1].SSHUsers, check.HasLen, 1) + c.Assert(app.sshPolicy.Rules[1].Principals, check.HasLen, 1) + c.Assert(app.sshPolicy.Rules[1].Principals[0].NodeIP, check.Matches, "*") +} + func (s *Suite) TestInvalidGroupInGroup(c *check.C) { // this ACL is wrong because the group in Sources sections doesn't exist app.aclPolicy = &ACLPolicy{ diff --git a/acls_types.go b/acls_types.go index 638a456f..da981d38 100644 --- a/acls_types.go +++ b/acls_types.go @@ -17,6 +17,7 @@ type ACLPolicy struct { ACLs []ACL `json:"acls" yaml:"acls"` Tests []ACLTest `json:"tests" yaml:"tests"` AutoApprovers AutoApprovers `json:"autoApprovers" yaml:"autoApprovers"` + SSHs []SSH `json:"ssh" yaml:"ssh"` } // ACL is a basic rule for the ACL Policy. @@ -50,6 +51,15 @@ type AutoApprovers struct { ExitNode []string `json:"exitNode" yaml:"exitNode"` } +// SSH controls who can ssh into which machines. +type SSH struct { + Action string `json:"action" yaml:"action"` + Sources []string `json:"src" yaml:"src"` + Destinations []string `json:"dst" yaml:"dst"` + Users []string `json:"users" yaml:"users"` + CheckPeriod string `json:"checkPeriod,omitempty" yaml:"checkPeriod,omitempty"` +} + // UnmarshalJSON allows to parse the Hosts directly into netip objects. func (hosts *Hosts) UnmarshalJSON(data []byte) error { newHosts := Hosts{} diff --git a/api_common.go b/api_common.go index 9e7ff480..c4f9c798 100644 --- a/api_common.go +++ b/api_common.go @@ -62,6 +62,7 @@ func (h *Headscale) generateMapResponse( DNSConfig: dnsConfig, Domain: h.cfg.BaseDomain, PacketFilter: h.aclRules, + SSHPolicy: h.sshPolicy, DERPMap: h.DERPMap, UserProfiles: profiles, Debug: &tailcfg.Debug{ diff --git a/app.go b/app.go index 69d40794..aec8d689 100644 --- a/app.go +++ b/app.go @@ -88,6 +88,7 @@ type Headscale struct { aclPolicy *ACLPolicy aclRules []tailcfg.FilterRule + sshPolicy *tailcfg.SSHPolicy lastStateChange *xsync.MapOf[string, time.Time] diff --git a/machine.go b/machine.go index b688be65..e9dbb606 100644 --- a/machine.go +++ b/machine.go @@ -744,7 +744,11 @@ func (machine Machine) toNode( KeepAlive: true, MachineAuthorized: !machine.isExpired(), - Capabilities: []string{tailcfg.CapabilityFileSharing}, + Capabilities: []string{ + tailcfg.CapabilityFileSharing, + tailcfg.CapabilityAdmin, + tailcfg.CapabilitySSH, + }, } return &node, nil From 519f22f9bf785d2039be3068d47c1bece9a69fe4 Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Wed, 26 Oct 2022 13:31:30 +0200 Subject: [PATCH 02/19] SSH integration test setup --- Dockerfile.tailscale | 6 +- Dockerfile.tailscale-HEAD | 5 +- integration/ssh_test.go | 115 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 integration/ssh_test.go diff --git a/Dockerfile.tailscale b/Dockerfile.tailscale index 145ab6f7..69f6f656 100644 --- a/Dockerfile.tailscale +++ b/Dockerfile.tailscale @@ -4,13 +4,17 @@ ARG TAILSCALE_VERSION=* ARG TAILSCALE_CHANNEL=stable RUN apt-get update \ - && apt-get install -y gnupg curl \ + && apt-get install -y gnupg curl ssh \ && curl -fsSL https://pkgs.tailscale.com/${TAILSCALE_CHANNEL}/ubuntu/focal.gpg | apt-key add - \ && curl -fsSL https://pkgs.tailscale.com/${TAILSCALE_CHANNEL}/ubuntu/focal.list | tee /etc/apt/sources.list.d/tailscale.list \ && apt-get update \ && apt-get install -y ca-certificates tailscale=${TAILSCALE_VERSION} dnsutils \ && rm -rf /var/lib/apt/lists/* +RUN adduser --shell=/bin/bash ssh-it-user + +RUN service ssh start + ADD integration_test/etc_embedded_derp/tls/server.crt /usr/local/share/ca-certificates/ RUN chmod 644 /usr/local/share/ca-certificates/server.crt diff --git a/Dockerfile.tailscale-HEAD b/Dockerfile.tailscale-HEAD index c6e894da..4b0eead9 100644 --- a/Dockerfile.tailscale-HEAD +++ b/Dockerfile.tailscale-HEAD @@ -1,9 +1,12 @@ FROM golang:latest RUN apt-get update \ - && apt-get install -y ca-certificates dnsutils git iptables \ + && apt-get install -y ca-certificates dnsutils git iptables ssh \ && rm -rf /var/lib/apt/lists/* +RUN useradd --shell=/bin/bash --create-home ssh-it-user + +RUN service ssh start RUN git clone https://github.com/tailscale/tailscale.git diff --git a/integration/ssh_test.go b/integration/ssh_test.go new file mode 100644 index 00000000..22ad228f --- /dev/null +++ b/integration/ssh_test.go @@ -0,0 +1,115 @@ +package integration + +import ( + "fmt" + "testing" + + "github.com/juanfont/headscale" +) + +func TestSSHIntoAll(t *testing.T) { + IntegrationSkip(t) + + scenario, err := NewScenario() + if err != nil { + t.Errorf("failed to create scenario: %s", err) + } + + spec := &HeadscaleSpec{ + namespaces: map[string]int{ + // Omit versions before 1.24 because they don't support SSH + "namespace1": len(TailscaleVersions) - 4, + "namespace2": len(TailscaleVersions) - 4, + }, + enableSSH: true, + acl: &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:integration-test": {"namespace1", "namespace2"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []headscale.SSH{ + { + Action: "accept", + Sources: []string{"group:integration-test"}, + Destinations: []string{"group:integration-test"}, + Users: []string{"ssh-it-user"}, + }, + }, + }, + } + + err = scenario.CreateHeadscaleEnv(spec) + if err != nil { + t.Errorf("failed to create headscale environment: %s", err) + } + err = scenario.WaitForTailscaleSync() + if err != nil { + t.Errorf("failed wait for tailscale clients to be in sync: %s", err) + } + + for namespace := range spec.namespaces { + // This will essentially fetch and cache all the FQDNs for the given namespace + nsFQDNs, err := scenario.ListTailscaleClientsFQDNs(namespace) + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } + + nsClients, err := scenario.ListTailscaleClients(namespace) + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + for _, client := range nsClients { + currentClientFqdn, _ := client.FQDN() + sshTargets := removeFromSlice(nsFQDNs, currentClientFqdn) + + for _, target := range sshTargets { + t.Run( + fmt.Sprintf("%s-%s", currentClientFqdn, target), + func(t *testing.T) { + command := []string{ + "ssh", "-o StrictHostKeyChecking=no", + fmt.Sprintf("%s@%s", "ssh-it-user", target), + "'hostname'", + } + + result, err := client.Execute(command) + if err != nil { + t.Errorf("failed to execute command over SSH: %s", err) + } + + if result != target { + t.Logf("result=%s, target=%s", result, target) + t.Fail() + } + + t.Logf("Result for %s: %s\n", target, result) + }, + ) + } + + // t.Logf("%s wants to SSH into %+v", currentClientFqdn, sshTargets) + } + } + + err = scenario.Shutdown() + if err != nil { + t.Errorf("failed to tear down scenario: %s", err) + } +} + +func removeFromSlice(haystack []string, needle string) []string { + for i, value := range haystack { + if needle == value { + return append(haystack[:i], haystack[i+1:]...) + } + } + + return haystack +} From d207c3094958128e4cefce97bcf5f22e066e7f05 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 8 Nov 2022 15:09:19 +0000 Subject: [PATCH 03/19] Ensure we have ssh in container Signed-off-by: Kristoffer Dalby --- Dockerfile.tailscale | 4 +--- Dockerfile.tailscale-HEAD | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Dockerfile.tailscale b/Dockerfile.tailscale index 69f6f656..fc02aeaa 100644 --- a/Dockerfile.tailscale +++ b/Dockerfile.tailscale @@ -13,9 +13,7 @@ RUN apt-get update \ RUN adduser --shell=/bin/bash ssh-it-user -RUN service ssh start - ADD integration_test/etc_embedded_derp/tls/server.crt /usr/local/share/ca-certificates/ -RUN chmod 644 /usr/local/share/ca-certificates/server.crt +RUN chmod 644 /usr/local/share/ca-certificates/server.crt RUN update-ca-certificates diff --git a/Dockerfile.tailscale-HEAD b/Dockerfile.tailscale-HEAD index 4b0eead9..c9a04189 100644 --- a/Dockerfile.tailscale-HEAD +++ b/Dockerfile.tailscale-HEAD @@ -6,8 +6,6 @@ RUN apt-get update \ RUN useradd --shell=/bin/bash --create-home ssh-it-user -RUN service ssh start - RUN git clone https://github.com/tailscale/tailscale.git WORKDIR /go/tailscale @@ -21,6 +19,6 @@ RUN cp tailscale /usr/local/bin/ RUN cp tailscaled /usr/local/bin/ ADD integration_test/etc_embedded_derp/tls/server.crt /usr/local/share/ca-certificates/ -RUN chmod 644 /usr/local/share/ca-certificates/server.crt +RUN chmod 644 /usr/local/share/ca-certificates/server.crt RUN update-ca-certificates From cfaa36e51afb6c1400065f041201a77c6c66136c Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 8 Nov 2022 15:09:52 +0000 Subject: [PATCH 04/19] Add method to expose container id Signed-off-by: Kristoffer Dalby --- integration/tailscale.go | 1 + integration/tsic/tsic.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/integration/tailscale.go b/integration/tailscale.go index 6b51193a..c560c4fd 100644 --- a/integration/tailscale.go +++ b/integration/tailscale.go @@ -20,4 +20,5 @@ type TailscaleClient interface { WaitForReady() error WaitForPeers(expected int) error Ping(hostnameOrIP string) error + ID() string } diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index d79f7ba6..5e576658 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -176,6 +176,10 @@ func (t *TailscaleInContainer) Version() string { return t.version } +func (t *TailscaleInContainer) ID() string { + return t.container.Container.ID +} + func (t *TailscaleInContainer) Execute( command []string, ) (string, string, error) { From 36952842868cf48778b995ad6627312b5d63a781 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 8 Nov 2022 15:10:03 +0000 Subject: [PATCH 05/19] Make simple initial test case This commit makes the initial SSH test a bit simpler: - Use the same pattern/functions for all clients as other tests - Only test within _one_ namespace/user to confirm the base case - Use retry function, same as taildrop, there is some funky going on there... Signed-off-by: Kristoffer Dalby --- integration/cli_test.go | 9 +-- integration/general_test.go | 9 +-- integration/scenario.go | 17 +++-- integration/ssh_test.go | 124 +++++++++++++++++++++--------------- integration/tsic/tsic.go | 11 ++++ 5 files changed, 105 insertions(+), 65 deletions(-) diff --git a/integration/cli_test.go b/integration/cli_test.go index 58ef826a..d66bdf67 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -8,6 +8,7 @@ import ( v1 "github.com/juanfont/headscale/gen/go/headscale/v1" "github.com/juanfont/headscale/integration/hsic" + "github.com/juanfont/headscale/integration/tsic" "github.com/stretchr/testify/assert" ) @@ -37,7 +38,7 @@ func TestNamespaceCommand(t *testing.T) { "namespace2": 0, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("clins")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("clins")) assert.NoError(t, err) headscale, err := scenario.Headscale() @@ -118,7 +119,7 @@ func TestPreAuthKeyCommand(t *testing.T) { namespace: 0, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("clipak")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("clipak")) assert.NoError(t, err) headscale, err := scenario.Headscale() @@ -258,7 +259,7 @@ func TestPreAuthKeyCommandWithoutExpiry(t *testing.T) { namespace: 0, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("clipaknaexp")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("clipaknaexp")) assert.NoError(t, err) headscale, err := scenario.Headscale() @@ -323,7 +324,7 @@ func TestPreAuthKeyCommandReusableEphemeral(t *testing.T) { namespace: 0, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("clipakresueeph")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("clipakresueeph")) assert.NoError(t, err) headscale, err := scenario.Headscale() diff --git a/integration/general_test.go b/integration/general_test.go index be745541..27c62fff 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/juanfont/headscale/integration/hsic" + "github.com/juanfont/headscale/integration/tsic" "github.com/rs/zerolog/log" ) @@ -24,7 +25,7 @@ func TestPingAllByIP(t *testing.T) { "namespace2": len(TailscaleVersions), } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("pingallbyip")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("pingallbyip")) if err != nil { t.Errorf("failed to create headscale environment: %s", err) } @@ -80,7 +81,7 @@ func TestPingAllByHostname(t *testing.T) { "namespace4": len(TailscaleVersions) - 1, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("pingallbyname")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("pingallbyname")) if err != nil { t.Errorf("failed to create headscale environment: %s", err) } @@ -148,7 +149,7 @@ func TestTaildrop(t *testing.T) { "taildrop": len(TailscaleVersions) - 1, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("taildrop")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("taildrop")) if err != nil { t.Errorf("failed to create headscale environment: %s", err) } @@ -276,7 +277,7 @@ func TestResolveMagicDNS(t *testing.T) { "magicdns2": len(TailscaleVersions) - 1, } - err = scenario.CreateHeadscaleEnv(spec, hsic.WithTestName("magicdns")) + err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{}, hsic.WithTestName("magicdns")) if err != nil { t.Errorf("failed to create headscale environment: %s", err) } diff --git a/integration/scenario.go b/integration/scenario.go index 20bc4260..0ce4bdf8 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -229,6 +229,7 @@ func (s *Scenario) CreateTailscaleNodesInNamespace( namespaceStr string, requestedVersion string, count int, + opts ...tsic.Option, ) error { if namespace, ok := s.namespaces[namespaceStr]; ok { for i := 0; i < count; i++ { @@ -247,6 +248,11 @@ func (s *Scenario) CreateTailscaleNodesInNamespace( namespace.createWaitGroup.Add(1) + opts = append(opts, + tsic.WithHeadscaleTLS(cert), + tsic.WithHeadscaleName(hostname), + ) + go func() { defer namespace.createWaitGroup.Done() @@ -255,8 +261,7 @@ func (s *Scenario) CreateTailscaleNodesInNamespace( s.pool, version, s.network, - tsic.WithHeadscaleTLS(cert), - tsic.WithHeadscaleName(hostname), + opts..., ) if err != nil { // return fmt.Errorf("failed to add tailscale node: %w", err) @@ -341,7 +346,11 @@ func (s *Scenario) WaitForTailscaleSync() error { // CreateHeadscaleEnv is a conventient method returning a set up Headcale // test environment with nodes of all versions, joined to the server with X // namespaces. -func (s *Scenario) CreateHeadscaleEnv(namespaces map[string]int, opts ...hsic.Option) error { +func (s *Scenario) CreateHeadscaleEnv( + namespaces map[string]int, + tsOpts []tsic.Option, + opts ...hsic.Option, +) error { headscale, err := s.Headscale(opts...) if err != nil { return err @@ -353,7 +362,7 @@ func (s *Scenario) CreateHeadscaleEnv(namespaces map[string]int, opts ...hsic.Op return err } - err = s.CreateTailscaleNodesInNamespace(namespaceName, "all", clientCount) + err = s.CreateTailscaleNodesInNamespace(namespaceName, "all", clientCount, tsOpts...) if err != nil { return err } diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 22ad228f..565434e8 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -2,14 +2,29 @@ package integration import ( "fmt" + "strings" "testing" + "time" "github.com/juanfont/headscale" ) -func TestSSHIntoAll(t *testing.T) { +func TestSSHOneNamespaceAllToAll(t *testing.T) { IntegrationSkip(t) + retry := func(times int, sleepInverval time.Duration, doWork func() (string, error)) (string, error) { + var err error + for attempts := 0; attempts < times; attempts++ { + result, err := doWork() + if err == nil { + return result, nil + } + time.Sleep(sleepInverval) + } + + return "", err + } + scenario, err := NewScenario() if err != nil { t.Errorf("failed to create scenario: %s", err) @@ -17,14 +32,12 @@ func TestSSHIntoAll(t *testing.T) { spec := &HeadscaleSpec{ namespaces: map[string]int{ - // Omit versions before 1.24 because they don't support SSH - "namespace1": len(TailscaleVersions) - 4, - "namespace2": len(TailscaleVersions) - 4, + "namespace1": len(TailscaleVersions) - 5, }, enableSSH: true, acl: &headscale.ACLPolicy{ Groups: map[string][]string{ - "group:integration-test": {"namespace1", "namespace2"}, + "group:integration-test": {"namespace1"}, }, ACLs: []headscale.ACL{ { @@ -48,68 +61,73 @@ func TestSSHIntoAll(t *testing.T) { if err != nil { t.Errorf("failed to create headscale environment: %s", err) } + + allClients, err := scenario.ListTailscaleClients() + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + err = scenario.WaitForTailscaleSync() if err != nil { t.Errorf("failed wait for tailscale clients to be in sync: %s", err) } - for namespace := range spec.namespaces { - // This will essentially fetch and cache all the FQDNs for the given namespace - nsFQDNs, err := scenario.ListTailscaleClientsFQDNs(namespace) - if err != nil { - t.Errorf("failed to get FQDNs: %s", err) - } + _, err = scenario.ListTailscaleClientsFQDNs() + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } - nsClients, err := scenario.ListTailscaleClients(namespace) - if err != nil { - t.Errorf("failed to get clients: %s", err) - } + success := 0 - for _, client := range nsClients { - currentClientFqdn, _ := client.FQDN() - sshTargets := removeFromSlice(nsFQDNs, currentClientFqdn) - - for _, target := range sshTargets { - t.Run( - fmt.Sprintf("%s-%s", currentClientFqdn, target), - func(t *testing.T) { - command := []string{ - "ssh", "-o StrictHostKeyChecking=no", - fmt.Sprintf("%s@%s", "ssh-it-user", target), - "'hostname'", - } - - result, err := client.Execute(command) - if err != nil { - t.Errorf("failed to execute command over SSH: %s", err) - } - - if result != target { - t.Logf("result=%s, target=%s", result, target) - t.Fail() - } - - t.Logf("Result for %s: %s\n", target, result) - }, - ) + for _, client := range allClients { + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue } - // t.Logf("%s wants to SSH into %+v", currentClientFqdn, sshTargets) + clientFQDN, _ := client.FQDN() + peerFQDN, _ := peer.FQDN() + + t.Run( + fmt.Sprintf("%s-%s", clientFQDN, peerFQDN), + func(t *testing.T) { + command := []string{ + "ssh", "-o StrictHostKeyChecking=no", "-o ConnectTimeout=1", + fmt.Sprintf("%s@%s", "ssh-it-user", peer.Hostname()), + "'hostname'", + } + + result, err := retry(10, 1*time.Second, func() (string, error) { + return client.Execute(command) + }) + if err != nil { + t.Errorf("failed to execute command over SSH: %s", err) + } + + if strings.Contains(peer.ID(), result) { + t.Logf( + "failed to get correct container ID from %s, expected: %s, got: %s", + peer.Hostname(), + peer.ID(), + result, + ) + t.Fail() + } else { + success++ + } + }, + ) } } + t.Logf( + "%d successful pings out of %d", + success, + (len(allClients)*len(allClients))-len(allClients), + ) + err = scenario.Shutdown() if err != nil { t.Errorf("failed to tear down scenario: %s", err) } } - -func removeFromSlice(haystack []string, needle string) []string { - for i, value := range haystack { - if needle == value { - return append(haystack[:i], haystack[i+1:]...) - } - } - - return haystack -} diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 5e576658..d656b1c0 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -47,6 +47,7 @@ type TailscaleInContainer struct { // optional config headscaleCert []byte headscaleHostname string + withSSH bool } type Option = func(c *TailscaleInContainer) @@ -83,6 +84,12 @@ func WithHeadscaleName(hsName string) Option { } } +func WithSSH() Option { + return func(tsic *TailscaleInContainer) { + tsic.withSSH = true + } +} + func New( pool *dockertest.Pool, version string, @@ -219,6 +226,10 @@ func (t *TailscaleInContainer) Up( t.hostname, } + if t.withSSH { + command = append(command, "--ssh") + } + if _, _, err := t.Execute(command); err != nil { return fmt.Errorf("failed to join tailscale client: %w", err) } From fd6d25b5c1934f65399f5d3ff9ee3485f2c796d2 Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Tue, 8 Nov 2022 22:33:11 +0100 Subject: [PATCH 06/19] SSH: Lint and typos --- integration/scenario_test.go | 2 +- integration/ssh_test.go | 7 ++++--- integration/tailscale.go | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/integration/scenario_test.go b/integration/scenario_test.go index 5504a814..363f8e37 100644 --- a/integration/scenario_test.go +++ b/integration/scenario_test.go @@ -6,7 +6,7 @@ import ( "github.com/juanfont/headscale/integration/dockertestutil" ) -// This file is intendet to "test the test framework", by proxy it will also test +// This file is intended to "test the test framework", by proxy it will also test // some Headcsale/Tailscale stuff, but mostly in very simple ways. func IntegrationSkip(t *testing.T) { diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 565434e8..fe364c5e 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -12,14 +12,15 @@ import ( func TestSSHOneNamespaceAllToAll(t *testing.T) { IntegrationSkip(t) - retry := func(times int, sleepInverval time.Duration, doWork func() (string, error)) (string, error) { + retry := func(times int, sleepInterval time.Duration, doWork func() (string, error)) (string, error) { var err error for attempts := 0; attempts < times; attempts++ { - result, err := doWork() + var result string + result, err = doWork() if err == nil { return result, nil } - time.Sleep(sleepInverval) + time.Sleep(sleepInterval) } return "", err diff --git a/integration/tailscale.go b/integration/tailscale.go index c560c4fd..b69b217a 100644 --- a/integration/tailscale.go +++ b/integration/tailscale.go @@ -7,6 +7,7 @@ import ( "tailscale.com/ipn/ipnstate" ) +//nolint type TailscaleClient interface { Hostname() string Shutdown() error From f610be632e035f1cd7fe6f1ea3ce5bae015ef48b Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Fri, 11 Nov 2022 13:20:12 +0100 Subject: [PATCH 07/19] SSH: add test between namespaces --- integration/ssh_test.go | 250 ++++++++++++++++++++++++++++------------ 1 file changed, 178 insertions(+), 72 deletions(-) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index fe364c5e..0fbebdca 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -7,58 +7,61 @@ import ( "time" "github.com/juanfont/headscale" + "github.com/juanfont/headscale/integration/hsic" + "github.com/juanfont/headscale/integration/tsic" ) +var retry = func(times int, sleepInterval time.Duration, doWork func() (string, error)) (string, error) { + var err error + for attempts := 0; attempts < times; attempts++ { + var result string + result, err = doWork() + if err == nil { + return result, nil + } + time.Sleep(sleepInterval) + } + + return "", err +} + func TestSSHOneNamespaceAllToAll(t *testing.T) { IntegrationSkip(t) - retry := func(times int, sleepInterval time.Duration, doWork func() (string, error)) (string, error) { - var err error - for attempts := 0; attempts < times; attempts++ { - var result string - result, err = doWork() - if err == nil { - return result, nil - } - time.Sleep(sleepInterval) - } - - return "", err - } - scenario, err := NewScenario() if err != nil { t.Errorf("failed to create scenario: %s", err) } - spec := &HeadscaleSpec{ - namespaces: map[string]int{ - "namespace1": len(TailscaleVersions) - 5, - }, - enableSSH: true, - acl: &headscale.ACLPolicy{ - Groups: map[string][]string{ - "group:integration-test": {"namespace1"}, - }, - ACLs: []headscale.ACL{ - { - Action: "accept", - Sources: []string{"*"}, - Destinations: []string{"*:*"}, - }, - }, - SSHs: []headscale.SSH{ - { - Action: "accept", - Sources: []string{"group:integration-test"}, - Destinations: []string{"group:integration-test"}, - Users: []string{"ssh-it-user"}, - }, - }, - }, + spec := map[string]int{ + "namespace1": len(TailscaleVersions) - 5, } - err = scenario.CreateHeadscaleEnv(spec) + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{tsic.WithSSH()}, + hsic.WithACLPolicy( + &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:integration-test": {"namespace1"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []headscale.SSH{ + { + Action: "accept", + Sources: []string{"group:integration-test"}, + Destinations: []string{"group:integration-test"}, + Users: []string{"ssh-it-user"}, + }, + }, + }, + ), + ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) } @@ -86,38 +89,9 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { continue } - clientFQDN, _ := client.FQDN() - peerFQDN, _ := peer.FQDN() - - t.Run( - fmt.Sprintf("%s-%s", clientFQDN, peerFQDN), - func(t *testing.T) { - command := []string{ - "ssh", "-o StrictHostKeyChecking=no", "-o ConnectTimeout=1", - fmt.Sprintf("%s@%s", "ssh-it-user", peer.Hostname()), - "'hostname'", - } - - result, err := retry(10, 1*time.Second, func() (string, error) { - return client.Execute(command) - }) - if err != nil { - t.Errorf("failed to execute command over SSH: %s", err) - } - - if strings.Contains(peer.ID(), result) { - t.Logf( - "failed to get correct container ID from %s, expected: %s, got: %s", - peer.Hostname(), - peer.ID(), - result, - ) - t.Fail() - } else { - success++ - } - }, - ) + if doSSH(t, client, peer) { + success++ + } } } @@ -132,3 +106,135 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { t.Errorf("failed to tear down scenario: %s", err) } } + +func TestSSHMultipleNamespacesAllToAll(t *testing.T) { + IntegrationSkip(t) + + scenario, err := NewScenario() + if err != nil { + t.Errorf("failed to create scenario: %s", err) + } + + spec := map[string]int{ + "namespace1": len(TailscaleVersions) - 5, + "namespace2": len(TailscaleVersions) - 5, + } + + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{tsic.WithSSH()}, + hsic.WithACLPolicy( + &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:integration-test": {"namespace1", "namespace2"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []headscale.SSH{ + { + Action: "accept", + Sources: []string{"group:integration-test"}, + Destinations: []string{"group:integration-test"}, + Users: []string{"ssh-it-user"}, + }, + }, + }, + ), + ) + if err != nil { + t.Errorf("failed to create headscale environment: %s", err) + } + + nsOneClients, err := scenario.ListTailscaleClients("namespace1") + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + nsTwoClients, err := scenario.ListTailscaleClients("namespace2") + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + err = scenario.WaitForTailscaleSync() + if err != nil { + t.Errorf("failed wait for tailscale clients to be in sync: %s", err) + } + + _, err = scenario.ListTailscaleClientsFQDNs() + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } + + success := 0 + + testInterNamespaceSSH := func(sourceClients []TailscaleClient, targetClients []TailscaleClient) { + for _, client := range sourceClients { + for _, peer := range targetClients { + if doSSH(t, client, peer) { + success++ + } + } + } + } + + testInterNamespaceSSH(nsOneClients, nsTwoClients) + testInterNamespaceSSH(nsTwoClients, nsOneClients) + + t.Logf( + "%d successful pings out of %d", + success, + ((len(nsOneClients)*len(nsOneClients))-len(nsOneClients))*2, + ) + + err = scenario.Shutdown() + if err != nil { + t.Errorf("failed to tear down scenario: %s", err) + } +} + +func doSSH(t *testing.T, client TailscaleClient, peer TailscaleClient) bool { + t.Helper() + + clientFQDN, _ := client.FQDN() + peerFQDN, _ := peer.FQDN() + + success := false + + t.Run( + fmt.Sprintf("%s-%s", clientFQDN, peerFQDN), + func(t *testing.T) { + command := []string{ + "ssh", "-o StrictHostKeyChecking=no", "-o ConnectTimeout=1", + fmt.Sprintf("%s@%s", "ssh-it-user", peerFQDN), + "'hostname'", + } + + result, err := retry(10, 1*time.Second, func() (string, error) { + result, _, err := client.Execute(command) + + return result, err + }) + if err != nil { + t.Errorf("failed to execute command over SSH: %s", err) + } + + if strings.Contains(peer.ID(), result) { + t.Logf( + "failed to get correct container ID from %s, expected: %s, got: %s", + peer.Hostname(), + peer.ID(), + result, + ) + t.Fail() + } else { + success = true + } + }, + ) + + return success +} From e28d308796ec67f27c28549acbad56ce75516e10 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 19 Nov 2022 12:41:12 +0100 Subject: [PATCH 08/19] Add negative tests Signed-off-by: Kristoffer Dalby --- integration/ssh_test.go | 375 +++++++++++++++++++++++++++++++++------- 1 file changed, 313 insertions(+), 62 deletions(-) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 0fbebdca..6520e994 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -2,27 +2,35 @@ package integration import ( "fmt" - "strings" "testing" "time" "github.com/juanfont/headscale" "github.com/juanfont/headscale/integration/hsic" "github.com/juanfont/headscale/integration/tsic" + "github.com/stretchr/testify/assert" ) -var retry = func(times int, sleepInterval time.Duration, doWork func() (string, error)) (string, error) { +var retry = func(times int, sleepInterval time.Duration, + doWork func() (string, string, error), +) (string, string, error) { + var result string + var stderr string var err error + for attempts := 0; attempts < times; attempts++ { - var result string - result, err = doWork() + tempResult, tempStderr, err := doWork() + + result += tempResult + stderr += tempStderr + if err == nil { - return result, nil + return result, stderr, nil } time.Sleep(sleepInterval) } - return "", err + return result, stderr, err } func TestSSHOneNamespaceAllToAll(t *testing.T) { @@ -81,26 +89,16 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { t.Errorf("failed to get FQDNs: %s", err) } - success := 0 - for _, client := range allClients { for _, peer := range allClients { if client.Hostname() == peer.Hostname() { continue } - if doSSH(t, client, peer) { - success++ - } + assertSSHHostname(t, client, peer) } } - t.Logf( - "%d successful pings out of %d", - success, - (len(allClients)*len(allClients))-len(allClients), - ) - err = scenario.Shutdown() if err != nil { t.Errorf("failed to tear down scenario: %s", err) @@ -169,14 +167,10 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { t.Errorf("failed to get FQDNs: %s", err) } - success := 0 - testInterNamespaceSSH := func(sourceClients []TailscaleClient, targetClients []TailscaleClient) { for _, client := range sourceClients { for _, peer := range targetClients { - if doSSH(t, client, peer) { - success++ - } + assertSSHHostname(t, client, peer) } } } @@ -184,11 +178,71 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { testInterNamespaceSSH(nsOneClients, nsTwoClients) testInterNamespaceSSH(nsTwoClients, nsOneClients) - t.Logf( - "%d successful pings out of %d", - success, - ((len(nsOneClients)*len(nsOneClients))-len(nsOneClients))*2, + err = scenario.Shutdown() + if err != nil { + t.Errorf("failed to tear down scenario: %s", err) + } +} + +func TestSSHNoSSHConfigured(t *testing.T) { + IntegrationSkip(t) + + scenario, err := NewScenario() + if err != nil { + t.Errorf("failed to create scenario: %s", err) + } + + spec := map[string]int{ + "namespace1": len(TailscaleVersions) - 5, + } + + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{tsic.WithSSH()}, + hsic.WithACLPolicy( + &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:integration-test": {"namespace1"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []headscale.SSH{}, + }, + ), + hsic.WithTestName("sshnoneconfigured"), ) + if err != nil { + t.Errorf("failed to create headscale environment: %s", err) + } + + allClients, err := scenario.ListTailscaleClients() + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + err = scenario.WaitForTailscaleSync() + if err != nil { + t.Errorf("failed wait for tailscale clients to be in sync: %s", err) + } + + _, err = scenario.ListTailscaleClientsFQDNs() + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } + + for _, client := range allClients { + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + assertSSHPermissionDenied(t, client, peer) + } + } err = scenario.Shutdown() if err != nil { @@ -196,45 +250,242 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { } } -func doSSH(t *testing.T, client TailscaleClient, peer TailscaleClient) bool { +func TestSSHIsBlockedInACL(t *testing.T) { + IntegrationSkip(t) + + scenario, err := NewScenario() + if err != nil { + t.Errorf("failed to create scenario: %s", err) + } + + spec := map[string]int{ + "namespace1": len(TailscaleVersions) - 5, + } + + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{tsic.WithSSH()}, + hsic.WithACLPolicy( + &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:integration-test": {"namespace1"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:80"}, + }, + }, + SSHs: []headscale.SSH{ + { + Action: "accept", + Sources: []string{"group:integration-test"}, + Destinations: []string{"group:integration-test"}, + Users: []string{"ssh-it-user"}, + }, + }, + }, + ), + hsic.WithTestName("sshisblockedinacl"), + ) + if err != nil { + t.Errorf("failed to create headscale environment: %s", err) + } + + allClients, err := scenario.ListTailscaleClients() + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + err = scenario.WaitForTailscaleSync() + if err != nil { + t.Errorf("failed wait for tailscale clients to be in sync: %s", err) + } + + _, err = scenario.ListTailscaleClientsFQDNs() + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } + + for _, client := range allClients { + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + assertSSHTimeout(t, client, peer) + } + } + + err = scenario.Shutdown() + if err != nil { + t.Errorf("failed to tear down scenario: %s", err) + } +} + +func TestSSNamespaceOnlyIsolation(t *testing.T) { + IntegrationSkip(t) + + scenario, err := NewScenario() + if err != nil { + t.Errorf("failed to create scenario: %s", err) + } + + spec := map[string]int{ + "namespaceacl1": len(TailscaleVersions) - 5, + "namespaceacl2": len(TailscaleVersions) - 5, + } + + err = scenario.CreateHeadscaleEnv(spec, + []tsic.Option{tsic.WithSSH()}, + hsic.WithACLPolicy( + &headscale.ACLPolicy{ + Groups: map[string][]string{ + "group:ssh1": {"namespaceacl1"}, + "group:ssh2": {"namespaceacl2"}, + }, + ACLs: []headscale.ACL{ + { + Action: "accept", + Sources: []string{"*"}, + Destinations: []string{"*:*"}, + }, + }, + SSHs: []headscale.SSH{ + { + Action: "accept", + Sources: []string{"group:ssh1"}, + Destinations: []string{"group:ssh1"}, + Users: []string{"ssh-it-user"}, + }, + { + Action: "accept", + Sources: []string{"group:ssh2"}, + Destinations: []string{"group:ssh2"}, + Users: []string{"ssh-it-user"}, + }, + }, + }, + ), + hsic.WithTestName("sshtwonamespaceaclblock"), + ) + if err != nil { + t.Errorf("failed to create headscale environment: %s", err) + } + + ssh1Clients, err := scenario.ListTailscaleClients("namespaceacl1") + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + ssh2Clients, err := scenario.ListTailscaleClients("namespaceacl2") + if err != nil { + t.Errorf("failed to get clients: %s", err) + } + + err = scenario.WaitForTailscaleSync() + if err != nil { + t.Errorf("failed wait for tailscale clients to be in sync: %s", err) + } + + _, err = scenario.ListTailscaleClientsFQDNs() + if err != nil { + t.Errorf("failed to get FQDNs: %s", err) + } + + // TODO(kradalby,evenh): ACLs do currently not cover reject + // cases properly, and currently will accept all incomming connections + // as long as a rule is present. + // + // for _, client := range ssh1Clients { + // for _, peer := range ssh2Clients { + // if client.Hostname() == peer.Hostname() { + // continue + // } + // + // assertSSHPermissionDenied(t, client, peer) + // } + // } + // + // for _, client := range ssh2Clients { + // for _, peer := range ssh1Clients { + // if client.Hostname() == peer.Hostname() { + // continue + // } + // + // assertSSHPermissionDenied(t, client, peer) + // } + // } + + for _, client := range ssh1Clients { + for _, peer := range ssh1Clients { + if client.Hostname() == peer.Hostname() { + continue + } + + assertSSHHostname(t, client, peer) + } + } + + for _, client := range ssh2Clients { + for _, peer := range ssh2Clients { + if client.Hostname() == peer.Hostname() { + continue + } + + assertSSHHostname(t, client, peer) + } + } + + err = scenario.Shutdown() + if err != nil { + t.Errorf("failed to tear down scenario: %s", err) + } +} + +func doSSH(t *testing.T, client TailscaleClient, peer TailscaleClient) (string, string, error) { t.Helper() - clientFQDN, _ := client.FQDN() peerFQDN, _ := peer.FQDN() - success := false + command := []string{ + "ssh", "-o StrictHostKeyChecking=no", "-o ConnectTimeout=1", + fmt.Sprintf("%s@%s", "ssh-it-user", peerFQDN), + "'hostname'", + } - t.Run( - fmt.Sprintf("%s-%s", clientFQDN, peerFQDN), - func(t *testing.T) { - command := []string{ - "ssh", "-o StrictHostKeyChecking=no", "-o ConnectTimeout=1", - fmt.Sprintf("%s@%s", "ssh-it-user", peerFQDN), - "'hostname'", - } - - result, err := retry(10, 1*time.Second, func() (string, error) { - result, _, err := client.Execute(command) - - return result, err - }) - if err != nil { - t.Errorf("failed to execute command over SSH: %s", err) - } - - if strings.Contains(peer.ID(), result) { - t.Logf( - "failed to get correct container ID from %s, expected: %s, got: %s", - peer.Hostname(), - peer.ID(), - result, - ) - t.Fail() - } else { - success = true - } - }, - ) - - return success + return retry(10, 1*time.Second, func() (string, string, error) { + return client.Execute(command) + }) +} + +func assertSSHHostname(t *testing.T, client TailscaleClient, peer TailscaleClient) { + t.Helper() + + result, _, err := doSSH(t, client, peer) + assert.NoError(t, err) + + assert.Contains(t, peer.ID(), result) +} + +func assertSSHPermissionDenied(t *testing.T, client TailscaleClient, peer TailscaleClient) { + t.Helper() + + result, stderr, err := doSSH(t, client, peer) + assert.Error(t, err) + + assert.Empty(t, result) + + assert.Contains(t, stderr, "Permission denied (tailscale)") +} + +func assertSSHTimeout(t *testing.T, client TailscaleClient, peer TailscaleClient) { + t.Helper() + + result, stderr, err := doSSH(t, client, peer) + assert.NoError(t, err) + + assert.Empty(t, result) + + assert.Contains(t, stderr, "Connection timed out") } From f34e7c341b9bf10ee259887f540089c435f80d6b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 21 Nov 2022 13:56:18 +0100 Subject: [PATCH 09/19] Strip newline from hostname Signed-off-by: Kristoffer Dalby --- integration/ssh_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 6520e994..d480f8df 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -2,6 +2,7 @@ package integration import ( "fmt" + "strings" "testing" "time" @@ -465,7 +466,7 @@ func assertSSHHostname(t *testing.T, client TailscaleClient, peer TailscaleClien result, _, err := doSSH(t, client, peer) assert.NoError(t, err) - assert.Contains(t, peer.ID(), result) + assert.Contains(t, peer.ID(), strings.ReplaceAll(result, "\n", "")) } func assertSSHPermissionDenied(t *testing.T, client TailscaleClient, peer TailscaleClient) { From 8a79c2e7ed662b926a2157b6d029a96fb86bd13f Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 21 Nov 2022 14:08:24 +0100 Subject: [PATCH 10/19] Do not retry on permission denied in ssh Signed-off-by: Kristoffer Dalby --- integration/ssh_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index d480f8df..57d85980 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -28,6 +28,13 @@ var retry = func(times int, sleepInterval time.Duration, if err == nil { return result, stderr, nil } + + // If we get a permission denied error, we can fail immediately + // since that is something we wont recover from by retrying. + if err != nil && strings.Contains(stderr, "Permission denied (tailscale)") { + return result, stderr, err + } + time.Sleep(sleepInterval) } From d71aef3b98aff4d2aea3e22e4ecde476f8109f82 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 24 Nov 2022 16:17:24 +0100 Subject: [PATCH 11/19] Mark all tests with Parallel Signed-off-by: Kristoffer Dalby --- integration/ssh_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 57d85980..92fd90cf 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -43,6 +43,7 @@ var retry = func(times int, sleepInterval time.Duration, func TestSSHOneNamespaceAllToAll(t *testing.T) { IntegrationSkip(t) + t.Parallel() scenario, err := NewScenario() if err != nil { @@ -115,6 +116,7 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { func TestSSHMultipleNamespacesAllToAll(t *testing.T) { IntegrationSkip(t) + t.Parallel() scenario, err := NewScenario() if err != nil { @@ -194,6 +196,7 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { func TestSSHNoSSHConfigured(t *testing.T) { IntegrationSkip(t) + t.Parallel() scenario, err := NewScenario() if err != nil { @@ -260,6 +263,7 @@ func TestSSHNoSSHConfigured(t *testing.T) { func TestSSHIsBlockedInACL(t *testing.T) { IntegrationSkip(t) + t.Parallel() scenario, err := NewScenario() if err != nil { @@ -333,6 +337,7 @@ func TestSSHIsBlockedInACL(t *testing.T) { func TestSSNamespaceOnlyIsolation(t *testing.T) { IntegrationSkip(t) + t.Parallel() scenario, err := NewScenario() if err != nil { From 91ed6e2197247b34fac2682626d1c7fdd14d2233 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 24 Nov 2022 16:35:30 +0100 Subject: [PATCH 12/19] Allow WithEnv to be passed multiple times Signed-off-by: Kristoffer Dalby --- integration/hsic/hsic.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index 5945c913..9766c882 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -79,13 +79,9 @@ func WithTLS() Option { func WithConfigEnv(configEnv map[string]string) Option { return func(hsic *HeadscaleInContainer) { - env := []string{} - for key, value := range configEnv { - env = append(env, fmt.Sprintf("%s=%s", key, value)) + hsic.env = append(hsic.env, fmt.Sprintf("%s=%s", key, value)) } - - hsic.env = env } } From c6d31747f78c76e03f82402f9ff8f0a101512f65 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 24 Nov 2022 16:35:55 +0100 Subject: [PATCH 13/19] Add feature flag for SSH, and warning Signed-off-by: Kristoffer Dalby --- acls.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/acls.go b/acls.go index bb8c4bd9..34b213c9 100644 --- a/acls.go +++ b/acls.go @@ -15,6 +15,7 @@ import ( "github.com/rs/zerolog/log" "github.com/tailscale/hujson" "gopkg.in/yaml.v3" + "tailscale.com/envknob" "tailscale.com/tailcfg" ) @@ -55,6 +56,8 @@ const ( ProtocolFC = 133 // Fibre Channel ) +var featureEnableSSH = envknob.RegisterBool("HEADSCALE_FEATURE_SSH") + // LoadACLPolicy loads the ACL policy from the specify path, and generates the ACL rules. func (h *Headscale) LoadACLPolicy(path string) error { log.Debug(). @@ -121,15 +124,19 @@ func (h *Headscale) UpdateACLRules() error { log.Trace().Interface("ACL", rules).Msg("ACL rules generated") h.aclRules = rules - sshRules, err := h.generateSSHRules() - if err != nil { - return err + if featureEnableSSH() { + sshRules, err := h.generateSSHRules() + if err != nil { + return err + } + log.Trace().Interface("SSH", sshRules).Msg("SSH rules generated") + if h.sshPolicy == nil { + h.sshPolicy = &tailcfg.SSHPolicy{} + } + h.sshPolicy.Rules = sshRules + } else if h.aclPolicy != nil && len(h.aclPolicy.SSHs) > 0 { + log.Info().Msg("SSH ACLs has been defined, but HEADSCALE_FEATURE_SSH is not enabled, this is a unstable feature, check docs before activating") } - log.Trace().Interface("SSH", sshRules).Msg("SSH rules generated") - if h.sshPolicy == nil { - h.sshPolicy = &tailcfg.SSHPolicy{} - } - h.sshPolicy.Rules = sshRules return nil } From 22da5bfc1d0c7e726f3df26c007dee2321f973dd Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 24 Nov 2022 16:41:33 +0100 Subject: [PATCH 14/19] Enable SSH for tests Signed-off-by: Kristoffer Dalby --- integration/ssh_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 92fd90cf..69bcb7ba 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -78,6 +78,9 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { }, }, ), + hsic.WithConfigEnv(map[string]string{ + "HEADSCALE_FEATURE_SSH": "1", + }), ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) @@ -152,6 +155,9 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { }, }, ), + hsic.WithConfigEnv(map[string]string{ + "HEADSCALE_FEATURE_SSH": "1", + }), ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) @@ -225,6 +231,9 @@ func TestSSHNoSSHConfigured(t *testing.T) { }, ), hsic.WithTestName("sshnoneconfigured"), + hsic.WithConfigEnv(map[string]string{ + "HEADSCALE_FEATURE_SSH": "1", + }), ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) @@ -299,6 +308,9 @@ func TestSSHIsBlockedInACL(t *testing.T) { }, ), hsic.WithTestName("sshisblockedinacl"), + hsic.WithConfigEnv(map[string]string{ + "HEADSCALE_FEATURE_SSH": "1", + }), ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) @@ -381,6 +393,9 @@ func TestSSNamespaceOnlyIsolation(t *testing.T) { }, ), hsic.WithTestName("sshtwonamespaceaclblock"), + hsic.WithConfigEnv(map[string]string{ + "HEADSCALE_FEATURE_SSH": "1", + }), ) if err != nil { t.Errorf("failed to create headscale environment: %s", err) From c02e105065706bba8da6b1af98ac49acfbdfc3e1 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 24 Nov 2022 17:26:52 +0100 Subject: [PATCH 15/19] Mark the flag properly experimental Signed-off-by: Kristoffer Dalby --- acls.go | 4 ++-- integration/ssh_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/acls.go b/acls.go index 34b213c9..c6b65a1e 100644 --- a/acls.go +++ b/acls.go @@ -56,7 +56,7 @@ const ( ProtocolFC = 133 // Fibre Channel ) -var featureEnableSSH = envknob.RegisterBool("HEADSCALE_FEATURE_SSH") +var featureEnableSSH = envknob.RegisterBool("HEADSCALE_EXPERIMENTAL_FEATURE_SSH") // LoadACLPolicy loads the ACL policy from the specify path, and generates the ACL rules. func (h *Headscale) LoadACLPolicy(path string) error { @@ -135,7 +135,7 @@ func (h *Headscale) UpdateACLRules() error { } h.sshPolicy.Rules = sshRules } else if h.aclPolicy != nil && len(h.aclPolicy.SSHs) > 0 { - log.Info().Msg("SSH ACLs has been defined, but HEADSCALE_FEATURE_SSH is not enabled, this is a unstable feature, check docs before activating") + log.Info().Msg("SSH ACLs has been defined, but HEADSCALE_EXPERIMENTAL_FEATURE_SSH is not enabled, this is a unstable feature, check docs before activating") } return nil diff --git a/integration/ssh_test.go b/integration/ssh_test.go index 69bcb7ba..47c47333 100644 --- a/integration/ssh_test.go +++ b/integration/ssh_test.go @@ -79,7 +79,7 @@ func TestSSHOneNamespaceAllToAll(t *testing.T) { }, ), hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_FEATURE_SSH": "1", + "HEADSCALE_EXPERIMENTAL_FEATURE_SSH": "1", }), ) if err != nil { @@ -156,7 +156,7 @@ func TestSSHMultipleNamespacesAllToAll(t *testing.T) { }, ), hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_FEATURE_SSH": "1", + "HEADSCALE_EXPERIMENTAL_FEATURE_SSH": "1", }), ) if err != nil { @@ -232,7 +232,7 @@ func TestSSHNoSSHConfigured(t *testing.T) { ), hsic.WithTestName("sshnoneconfigured"), hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_FEATURE_SSH": "1", + "HEADSCALE_EXPERIMENTAL_FEATURE_SSH": "1", }), ) if err != nil { @@ -309,7 +309,7 @@ func TestSSHIsBlockedInACL(t *testing.T) { ), hsic.WithTestName("sshisblockedinacl"), hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_FEATURE_SSH": "1", + "HEADSCALE_EXPERIMENTAL_FEATURE_SSH": "1", }), ) if err != nil { @@ -394,7 +394,7 @@ func TestSSNamespaceOnlyIsolation(t *testing.T) { ), hsic.WithTestName("sshtwonamespaceaclblock"), hsic.WithConfigEnv(map[string]string{ - "HEADSCALE_FEATURE_SSH": "1", + "HEADSCALE_EXPERIMENTAL_FEATURE_SSH": "1", }), ) if err != nil { From c28ca27133d7f5f658d1c401228c6aacbbaba34c Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Mon, 21 Nov 2022 23:26:54 +0100 Subject: [PATCH 16/19] Add SSH ACL to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55331646..9c0ec582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - Fix OIDC registration issues [#960](https://github.com/juanfont/headscale/pull/960) and [#971](https://github.com/juanfont/headscale/pull/971) - Add support for specifying NextDNS DNS-over-HTTPS resolver [#940](https://github.com/juanfont/headscale/pull/940) - Make more sslmode available for postgresql connection [#927](https://github.com/juanfont/headscale/pull/927) +- Add support for [SSH ACL](https://tailscale.com/kb/1018/acls/#tailscale-ssh) blocks [#847](https://github.com/juanfont/headscale/pull/847) ## 0.16.4 (2022-08-21) From d4e3bf184b51f2eb92c2d35f4fadb76d4d5ff670 Mon Sep 17 00:00:00 2001 From: Even Holthe Date: Fri, 25 Nov 2022 14:46:34 +0100 Subject: [PATCH 17/19] Add experimental flag to unit test --- acls_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/acls_test.go b/acls_test.go index 0f3c262f..23e7f917 100644 --- a/acls_test.go +++ b/acls_test.go @@ -7,6 +7,7 @@ import ( "testing" "gopkg.in/check.v1" + "tailscale.com/envknob" "tailscale.com/tailcfg" ) @@ -74,6 +75,8 @@ func (s *Suite) TestInvalidAction(c *check.C) { } func (s *Suite) TestSshRules(c *check.C) { + envknob.Setenv("HEADSCALE_EXPERIMENTAL_FEATURE_SSH", "1") + namespace, err := app.CreateNamespace("user1") c.Assert(err, check.IsNil) From 36b8862e7c13825b228e4d63ae2f7df5ad160dc4 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 26 Nov 2022 09:34:37 +0100 Subject: [PATCH 18/19] Add notes about current ssh status Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c0ec582..15882c8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,14 @@ - Fix OIDC registration issues [#960](https://github.com/juanfont/headscale/pull/960) and [#971](https://github.com/juanfont/headscale/pull/971) - Add support for specifying NextDNS DNS-over-HTTPS resolver [#940](https://github.com/juanfont/headscale/pull/940) - Make more sslmode available for postgresql connection [#927](https://github.com/juanfont/headscale/pull/927) -- Add support for [SSH ACL](https://tailscale.com/kb/1018/acls/#tailscale-ssh) blocks [#847](https://github.com/juanfont/headscale/pull/847) +- Add experimental support for [SSH ACL](https://tailscale.com/kb/1018/acls/#tailscale-ssh) (see docs for limitations) [#847](https://github.com/juanfont/headscale/pull/847) + - Please note that this support should be considered _partially_ implemented + - SSH ACLs status: + - Support `accept` and `check` (SSH can be enabled and used for connecting and authentication) + - Rejecting connections **are not supported**, meaning that if you enable SSH, then assume that _all_ `ssh` connections **will be allowed**. + - If you decied to try this feature, please carefully managed permissions by blocking port `22` with regular ACLs or do _not_ set `--ssh` on your clients. + - We are currently improving our testing of the SSH ACLs, help us get an overview by testing and giving feedback. + - This feature should be considered dangerous and it is disabled by default. Enable by setting `HEADSCALE_EXPERIMENTAL_FEATURE_SSH=1`. ## 0.16.4 (2022-08-21) From eb072a1a74f0bb605d23c31fbe6f26acab99bd3d Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 26 Nov 2022 11:57:51 +0100 Subject: [PATCH 19/19] mark some changes as more important Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15882c8a..ffe0ce7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,20 @@ - Log level option `log_level` was moved to a distinct `log` config section and renamed to `level` [#768](https://github.com/juanfont/headscale/pull/768) - Removed Alpine Linux container image [#962](https://github.com/juanfont/headscale/pull/962) -### Changes +### Important Changes - Added support for Tailscale TS2021 protocol [#738](https://github.com/juanfont/headscale/pull/738) +- Add experimental support for [SSH ACL](https://tailscale.com/kb/1018/acls/#tailscale-ssh) (see docs for limitations) [#847](https://github.com/juanfont/headscale/pull/847) + - Please note that this support should be considered _partially_ implemented + - SSH ACLs status: + - Support `accept` and `check` (SSH can be enabled and used for connecting and authentication) + - Rejecting connections **are not supported**, meaning that if you enable SSH, then assume that _all_ `ssh` connections **will be allowed**. + - If you decied to try this feature, please carefully managed permissions by blocking port `22` with regular ACLs or do _not_ set `--ssh` on your clients. + - We are currently improving our testing of the SSH ACLs, help us get an overview by testing and giving feedback. + - This feature should be considered dangerous and it is disabled by default. Enable by setting `HEADSCALE_EXPERIMENTAL_FEATURE_SSH=1`. + +### Changes + - Add ability to specify config location via env var `HEADSCALE_CONFIG` [#674](https://github.com/juanfont/headscale/issues/674) - Target Go 1.19 for Headscale [#778](https://github.com/juanfont/headscale/pull/778) - Target Tailscale v1.30.0 to build Headscale [#780](https://github.com/juanfont/headscale/pull/780) @@ -29,14 +40,6 @@ - Fix OIDC registration issues [#960](https://github.com/juanfont/headscale/pull/960) and [#971](https://github.com/juanfont/headscale/pull/971) - Add support for specifying NextDNS DNS-over-HTTPS resolver [#940](https://github.com/juanfont/headscale/pull/940) - Make more sslmode available for postgresql connection [#927](https://github.com/juanfont/headscale/pull/927) -- Add experimental support for [SSH ACL](https://tailscale.com/kb/1018/acls/#tailscale-ssh) (see docs for limitations) [#847](https://github.com/juanfont/headscale/pull/847) - - Please note that this support should be considered _partially_ implemented - - SSH ACLs status: - - Support `accept` and `check` (SSH can be enabled and used for connecting and authentication) - - Rejecting connections **are not supported**, meaning that if you enable SSH, then assume that _all_ `ssh` connections **will be allowed**. - - If you decied to try this feature, please carefully managed permissions by blocking port `22` with regular ACLs or do _not_ set `--ssh` on your clients. - - We are currently improving our testing of the SSH ACLs, help us get an overview by testing and giving feedback. - - This feature should be considered dangerous and it is disabled by default. Enable by setting `HEADSCALE_EXPERIMENTAL_FEATURE_SSH=1`. ## 0.16.4 (2022-08-21)