From 17f94f59e3a8260893a03b14507596e3a7e6b5a9 Mon Sep 17 00:00:00 2001
From: Russell Jones <russell.valerius.jones@gmail.com>
Date: Fri, 13 Dec 2019 01:20:03 +0000
Subject: [PATCH] Allow port forwarding to be disabled.

If the option for port forwarding is not specified, it's enabled by
default. Port forwarding is not specified in the default-implicit-role.
Since it's included in all role sets, port forwarding is always
enabled for all roles.

To fix this, port forwarding in the default-implicit-role is set to
false.
---
 lib/services/role.go      |  4 +++
 lib/services/role_test.go | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/lib/services/role.go b/lib/services/role.go
index fd1b2c0f831c6..57de4ce241dc8 100644
--- a/lib/services/role.go
+++ b/lib/services/role.go
@@ -128,6 +128,10 @@ func NewImplicitRole() Role {
 		Spec: RoleSpecV3{
 			Options: RoleOptions{
 				MaxSessionTTL: MaxDuration(),
+				// PortForwarding has to be set to false in the default-implicit-role
+				// otherwise all roles will be allowed to forward ports (since we default
+				// to true in the check).
+				PortForwarding: NewBoolOption(false),
 			},
 			Allow: RoleConditions{
 				Namespaces: []string{defaults.Namespace},
diff --git a/lib/services/role_test.go b/lib/services/role_test.go
index 1da4ccd862cf4..e16c4d5b93e34 100644
--- a/lib/services/role_test.go
+++ b/lib/services/role_test.go
@@ -1500,6 +1500,57 @@ func (s *RoleSuite) TestExtractFromLegacy(c *C) {
 	c.Assert(traits, DeepEquals, newTraits)
 }
 
+// TestBoolOptions makes sure that bool options (like agent forwarding and
+// port forwarding) can be disabled in a role.
+func (s *RoleSuite) TestBoolOptions(c *C) {
+	var tests = []struct {
+		inOptions           RoleOptions
+		outCanPortForward   bool
+		outCanForwardAgents bool
+	}{
+		// Setting options explicitly off should remain off.
+		{
+			inOptions: RoleOptions{
+				ForwardAgent:   NewBool(false),
+				PortForwarding: NewBoolOption(false),
+			},
+			outCanPortForward:   false,
+			outCanForwardAgents: false,
+		},
+		// Not setting options should set port forwarding to true (default enabled)
+		// and agent forwarding false (default disabled).
+		{
+			inOptions:           RoleOptions{},
+			outCanPortForward:   true,
+			outCanForwardAgents: false,
+		},
+		// Explicitly enabling should enable them.
+		{
+			inOptions: RoleOptions{
+				ForwardAgent:   NewBool(true),
+				PortForwarding: NewBoolOption(true),
+			},
+			outCanPortForward:   true,
+			outCanForwardAgents: true,
+		},
+	}
+	for _, tt := range tests {
+		set := NewRoleSet(&RoleV3{
+			Kind:    KindRole,
+			Version: V3,
+			Metadata: Metadata{
+				Name:      "role-name",
+				Namespace: defaults.Namespace,
+			},
+			Spec: RoleSpecV3{
+				Options: tt.inOptions,
+			},
+		})
+		c.Assert(set.CanPortForward(), Equals, tt.outCanPortForward)
+		c.Assert(set.CanForwardAgents(), Equals, tt.outCanForwardAgents)
+	}
+}
+
 // BenchmarkCheckAccessToServer tests how long it takes to run
 // CheckAccessToServer across 4,000 nodes for 5 roles each with 5 logins each.
 //