Merge branch 'master' into mvbrock/azure-integration-disco-azure

docs/pages/admin-guides/api/rbac.mdx

@@ -859,7 +859,11 @@ spec:
         enabled: true
     max_session_ttl: 30h0m0s
     pin_source_ip: false
-    port_forwarding: true
+    ssh_port_forwarding:
+      remote:
+        enabled: true
+      local:
+        enabled: true
     record_session:
       default: best_effort
       desktop: true
@@ -906,7 +910,11 @@ spec:
         enabled: true
     max_session_ttl: 30h0m0s
     pin_source_ip: false
-    port_forwarding: true
+    ssh_port_forwarding:
+      remote:
+        enabled: true
+      local:
+        enabled: true
     record_session:
       default: best_effort
       desktop: true

docs/pages/enroll-resources/server-access/rbac.mdx

@@ -135,8 +135,18 @@ spec:
     create_host_user_mode: keep
     # forward_agent controls whether SSH agent forwarding is allowed
     forward_agent: true
-    # port_forwarding controls whether TCP port forwarding is allowed for SSH
-    port_forwarding: true
+    # ssh_port_forwarding controls which TCP port forwarding modes are allowed over SSH. This replaces
+    # the deprecated port_forwarding field, which did not differentiate between remote and local
+    # port forwarding modes. If you have any existing roles that allow forwarding by enabling the
+    # legacy port_forwarding field then the forwarding controls configured in ssh_port_forwarding will be
+    # ignored.
+    ssh_port_forwarding:
+      # configures remote port forwarding behavior
+      remote:
+        enabled: true
+      # configures local port forwarding behavior
+      local:
+        enabled: true
     # ssh_file_copy controls whether file copying (SCP/SFTP) is allowed.
     # Defaults to true.
     ssh_file_copy: false

docs/pages/includes/role-spec.mdx

@@ -13,8 +13,18 @@ spec:
     max_session_ttl: 8h
     # forward_agent controls whether SSH agent forwarding is allowed
     forward_agent: true
-    # port_forwarding controls whether TCP port forwarding is allowed for SSH
-    port_forwarding: true
+    # ssh_port_forwarding controls which TCP port forwarding modes are allowed over SSH. This replaces
+    # the deprecated port_forwarding field, which did not differentiate between remote and local
+    # port forwarding modes. If you have any existing roles that allow forwarding by enabling the
+    # legacy port_forwarding field then the forwarding controls configured in ssh_port_forwarding will be
+    # ignored.
+    ssh_port_forwarding:
+      # configures remote port forwarding behavior
+      remote:
+        enabled: true
+      # configures local port forwarding behavior
+      local:
+        enabled: true
     # ssh_file_copy controls whether file copying (SCP/SFTP) is allowed.
     # Defaults to true.
     ssh_file_copy: false

docs/pages/reference/access-controls/roles.mdx

@@ -52,7 +52,7 @@ user:
 | - | - | - |
 | `max_session_ttl` | Max. time to live (TTL) of a user's SSH certificates | The shortest TTL wins |
 | `forward_agent` | Allow SSH agent forwarding | Logical "OR" i.e. if any role allows agent forwarding, it's allowed |
-| `port_forwarding` | Allow TCP port forwarding | Logical "OR" i.e. if any role allows port forwarding, it's allowed |
+| `ssh_port_forwarding` | Allow TCP port forwarding | Logical "AND" i.e. if any role denies port forwarding, it's denied |
 | `ssh_file_copy` | Allow SCP/SFTP | Logical "AND" i.e. if all roles allows file copying, it's allowed |
 | `client_idle_timeout` | Forcefully terminate active sessions after an idle interval | The shortest timeout value wins, i.e. the most restrictive value is selected |
 | `disconnect_expired_cert` | Forcefully terminate active sessions when a client certificate expires | Logical "OR" i.e. evaluates to "yes" if at least one role requires session termination |

docs/pages/reference/terraform-provider/resources/role.mdx

@@ -27,9 +27,17 @@ resource "teleport_role" "example" {
 
   spec = {
     options = {
-      forward_agent           = false
-      max_session_ttl         = "7m"
-      port_forwarding         = false
+      forward_agent   = false
+      max_session_ttl = "7m"
+      ssh_port_forwarding = {
+        remote = {
+          enabled = false
+        }
+
+        local = {
+          enabled = false
+        }
+      }
       client_idle_timeout     = "1h"
       disconnect_expired_cert = true
       permit_x11_forwarding   = false

examples/resources/admin.yaml

@@ -28,5 +28,9 @@ spec:
     - network
     forward_agent: true
     max_session_ttl: 30h0m0s
-    port_forwarding: true
+    ssh_port_forwarding:
+      remote:
+        enabled: true
+      local:
+        enabled: true
 version: v3

examples/resources/user.yaml

@@ -56,5 +56,9 @@ spec:
     - network
     forward_agent: true
     max_session_ttl: 30h0m0s
-    port_forwarding: true
+    ssh_port_forwarding:
+      remote:
+        enabled: true
+      local:
+        enabled: true
 version: v3

integrations/terraform/examples/resources/teleport_role/resource.tf

@@ -13,9 +13,17 @@ resource "teleport_role" "example" {
 
   spec = {
     options = {
-      forward_agent           = false
-      max_session_ttl         = "7m"
-      port_forwarding         = false
+      forward_agent   = false
+      max_session_ttl = "7m"
+      ssh_port_forwarding = {
+        remote = {
+          enabled = false
+        }
+
+        local = {
+          enabled = false
+        }
+      }
       client_idle_timeout     = "1h"
       disconnect_expired_cert = true
       permit_x11_forwarding   = false

integrations/terraform/reference.mdx

@@ -2051,7 +2051,8 @@ Options is for OpenSSH options like agent forwarding.
 | max_sessions               | number           |          | MaxSessions defines the maximum number of concurrent sessions per connection.                                                                                                                          |
 | permit_x11_forwarding      | bool             |          | PermitX11Forwarding authorizes use of X11 forwarding.                                                                                                                                                  |
 | pin_source_ip              | bool             |          | PinSourceIP forces the same client IP for certificate generation and usage                                                                                                                             |
-| port_forwarding            | bool             |          |                                                                                                                                                                                                        |
+| ssh_port_forwarding        | object           |          | SSHPortForwarding configures what types of SSH port forwarding are allowed by a role.                                                                                                                  |
+| port_forwarding            | bool             |          | Deprecated: Use SSHPortForwarding instead.                                                                                                                                                             |
 | record_session             | object           |          | RecordDesktopSession indicates whether desktop access sessions should be recorded. It defaults to true unless explicitly set to false.                                                                 |
 | request_access             | string           |          | RequestAccess defines the access request strategy (optional|note|always) where optional is the default.                                                                                                |
 | request_prompt             | string           |          | RequestPrompt is an optional message which tells users what they aught to request.                                                                                                                     |
@@ -2085,6 +2086,31 @@ SAML are options related to the Teleport SAML IdP.
 |---------|------|----------|-------------|
 | enabled | bool |          |             |
 
+##### spec.options.ssh_port_forwarding
+
+SSHPortForwarding configures what types of SSH port forwarding are allowed by a role.
+
+|  Name  |  Type  | Required |                    Description                            |
+|--------|--------|----------|-----------------------------------------------------------|
+| remote | object |          | remote contains options related to remote port forwarding |
+| local  | object |          | local contains options related to local port forwarding   |
+
+###### spec.options.ssh_port_forwarding.remote
+
+remote contains options related to remote port forwarding
+
+|  Name   | Type | Required | Description |
+|---------|------|----------|-------------|
+| enabled | bool |          |             |
+
+###### spec.options.ssh_port_forwarding.local
+
+local contains options related to local port forwarding
+
+|  Name   | Type | Required | Description |
+|---------|------|----------|-------------|
+| enabled | bool |          |             |
+
 ##### spec.options.record_session
 
 RecordDesktopSession indicates whether desktop access sessions should be recorded. It defaults to true unless explicitly set to false.
@@ -2114,11 +2140,19 @@ resource "teleport_role" "example" {
     options = {
       forward_agent           = false
       max_session_ttl         = "7m"
-      port_forwarding         = false
       client_idle_timeout     = "1h"
       disconnect_expired_cert = true
       permit_x11_forwarding   = false
       request_access          = "denied"
+      ssh_port_forwarding     = {
+        remote = {
+          enabled = false
+        }
+
+        local = {
+          enabled = false
+        }
+      }
     }
 
     allow = {

lib/auth/auth.go

@@ -1508,12 +1508,11 @@ func (a *Server) runPeriodicOperations() {
 									heartbeatsMissedByAuth.Inc()
 								}
 
+								if srv.GetSubKind() != types.SubKindOpenSSHNode {
+									return false, nil
+								}
 								// TODO(tross) DELETE in v20.0.0 - all invalid hostnames should have been sanitized by then.
 								if !validServerHostname(srv.GetHostname()) {
-									if srv.GetSubKind() != types.SubKindOpenSSHNode {
-										return false, nil
-									}
-
 									logger := a.logger.With("server", srv.GetName(), "hostname", srv.GetHostname())
 
 									logger.DebugContext(a.closeCtx, "sanitizing invalid static SSH server hostname")
@@ -1527,6 +1526,17 @@ func (a *Server) runPeriodicOperations() {
 									if _, err := a.Services.UpdateNode(a.closeCtx, srv); err != nil && !trace.IsCompareFailed(err) {
 										logger.WarnContext(a.closeCtx, "failed to update SSH server hostname", "error", err)
 									}
+								} else if oldHostname, ok := srv.GetLabel(replacedHostnameLabel); ok && validServerHostname(oldHostname) {
+									// If the hostname has been replaced by a sanitized version, revert it back to the original
+									// if the original is valid under the most recent rules.
+									logger := a.logger.With("server", srv.GetName(), "old_hostname", oldHostname, "sanitized_hostname", srv.GetHostname())
+									if err := restoreSanitizedHostname(srv); err != nil {
+										logger.WarnContext(a.closeCtx, "failed to restore sanitized static SSH server hostname", "error", err)
+										return false, nil
+									}
+									if _, err := a.Services.UpdateNode(a.closeCtx, srv); err != nil && !trace.IsCompareFailed(err) {
+										log.Warnf("Failed to update node hostname: %v", err)
+									}
 								}
 
 								return false, nil
@@ -5650,15 +5660,15 @@ func (a *Server) KeepAliveServer(ctx context.Context, h types.KeepAlive) error {
 
 const (
 	serverHostnameMaxLen       = 256
-	serverHostnameRegexPattern = `^[a-zA-Z0-9]([\.-]?[a-zA-Z0-9]+)*$`
+	serverHostnameRegexPattern = `^[a-zA-Z0-9]+[a-zA-Z0-9\.-]*$`
 	replacedHostnameLabel      = types.TeleportInternalLabelPrefix + "invalid-hostname"
 )
 
 var serverHostnameRegex = regexp.MustCompile(serverHostnameRegexPattern)
 
 // validServerHostname returns false if the hostname is longer than 256 characters or
 // does not entirely consist of alphanumeric characters as well as '-' and '.'. A valid hostname also
-// cannot begin with a symbol, and a symbol cannot be followed immediately by another symbol.
+// cannot begin with a symbol.
 func validServerHostname(hostname string) bool {
 	return len(hostname) <= serverHostnameMaxLen && serverHostnameRegex.MatchString(hostname)
 }
@@ -5697,6 +5707,26 @@ func sanitizeHostname(server types.Server) error {
 	return nil
 }
 
+// restoreSanitizedHostname restores the original hostname of a server and removes the label.
+func restoreSanitizedHostname(server types.Server) error {
+	oldHostname, ok := server.GetLabels()[replacedHostnameLabel]
+	// if the label is not present or the hostname is invalid under the most recent rules, do nothing.
+	if !ok || !validServerHostname(oldHostname) {
+		return nil
+	}
+
+	switch s := server.(type) {
+	case *types.ServerV2:
+		// restore the original hostname and remove the label.
+		s.Spec.Hostname = oldHostname
+		delete(s.Metadata.Labels, replacedHostnameLabel)
+	default:
+		return trace.BadParameter("invalid server provided")
+	}
+
+	return nil
+}
+
 // UpsertNode implements [services.Presence] by delegating to [Server.Services]
 // and potentially emitting a [usagereporter] event.
 func (a *Server) UpsertNode(ctx context.Context, server types.Server) (*types.KeepAlive, error) {

lib/auth/auth_test.go

@@ -4478,6 +4478,10 @@ func TestServerHostnameSanitization(t *testing.T) {
 			name:     "uuid dns hostname",
 			hostname: uuid.NewString() + ".example.com",
 		},
+		{
+			name:     "valid dns hostname with multi-dots",
+			hostname: "llama..example.com",
+		},
 		{
 			name:            "empty hostname",
 			hostname:        "",
@@ -4488,11 +4492,6 @@ func TestServerHostnameSanitization(t *testing.T) {
 			hostname:        strings.Repeat("a", serverHostnameMaxLen*2),
 			invalidHostname: true,
 		},
-		{
-			name:            "invalid dns hostname",
-			hostname:        "llama..example.com",
-			invalidHostname: true,
-		},
 		{
 			name:            "spaces in hostname",
 			hostname:        "the quick brown fox jumps over the lazy dog",
@@ -4562,3 +4561,74 @@ func TestServerHostnameSanitization(t *testing.T) {
 		})
 	}
 }
+
+func TestValidServerHostname(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		hostname string
+		want     bool
+	}{
+		{
+			name:     "valid dns hostname",
+			hostname: "llama.example.com",
+			want:     true,
+		},
+		{
+			name:     "valid friendly hostname",
+			hostname: "llama",
+			want:     true,
+		},
+		{
+			name:     "uuid hostname",
+			hostname: uuid.NewString(),
+			want:     true,
+		},
+		{
+			name:     "valid hostname with multi-dashes",
+			hostname: "llama--example.com",
+			want:     true,
+		},
+		{
+			name:     "valid hostname with multi-dots",
+			hostname: "llama..example.com",
+			want:     true,
+		},
+		{
+			name:     "valid hostname with numbers",
+			hostname: "llama9",
+			want:     true,
+		},
+		{
+			name:     "hostname with invalid characters",
+			hostname: "llama?!$",
+			want:     false,
+		},
+		{
+			name:     "super long hostname",
+			hostname: strings.Repeat("a", serverHostnameMaxLen*2),
+			want:     false,
+		},
+		{
+			name:     "hostname with spaces",
+			hostname: "the quick brown fox jumps over the lazy dog",
+			want:     false,
+		},
+		{
+			name:     "hostname with ;",
+			hostname: "llama;example.com",
+			want:     false,
+		},
+		{
+			name:     "empty hostname",
+			hostname: "",
+			want:     false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := validServerHostname(tt.hostname)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}