Skip to content

Commit

Permalink
Add desktop_clipboard role option (#10165)
Browse files Browse the repository at this point in the history 
* Add desktop_clipboard role option

As described in RFD 49, desktop_clipboard defaults to true.

Since sharing clipboards with a remote machine requires a high level
of trust, the presence of a single role in a role set which disables
the clipboard will result in the feature being disabled.

(This is in contrast to session recording, for example, where all
roles in a set must disable recording to turn it off.)

* completing TestBoolOptions

Co-authored-by: Isaiah Becker-Mayer <[email protected]>
  • Loading branch information
@zmb3
zmb3 and Isaiah Becker-Mayer authored Feb 7, 2022
1 parent fc2846c commit 69a96d4
Show file tree
Hide file tree
Showing 6 changed files with 915 additions and 763 deletions.
3 changes: 3 additions & 0 deletions api/types/role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,9 @@ func (r *RoleV4) CheckAndSetDefaults() error {
Desktop: NewBoolOption(true),
}
}
if r.Spec.Options.DesktopClipboard == nil {
r.Spec.Options.DesktopClipboard = NewBoolOption(true)
}

switch r.Version {
case V3:
Expand Down
1,555 changes: 807 additions & 748 deletions api/types/types.pb.go
View file

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions api/types/types.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,15 @@ message RoleOptions {
// RecordDesktopSession indicates whether desktop access sessions should be recorded.
// It defaults to true unless explicitly set to false.
RecordSession RecordSession = 15 [ (gogoproto.jsontag) = "record_session" ];

// DesktopClipboard indicates whether clipboard sharing is allowed between the user's
// workstation and the remote desktop. It defaults to true unless explicitly set to
// false.
BoolValue DesktopClipboard = 16 [
(gogoproto.nullable) = true,
(gogoproto.jsontag) = "desktop_clipboard",
(gogoproto.customtype) = "BoolOption"
];
}

message RecordSession {
Expand Down
22 changes: 20 additions & 2 deletions lib/services/role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,11 @@ type AccessChecker interface {
// CanPortForward returns true if this RoleSet can forward ports.
CanPortForward() bool

// DesktopClipboard returns true if the role set has enabled shared
// clipboard for desktop sessions. Clipboard sharing is disabled if
// one or more of the roles in the set has disabled it.
DesktopClipboard() bool

// MaybeCanReviewRequests attempts to guess if this RoleSet belongs
// to a user who should be submitting access reviews. Because not all rolesets
// are derived from statically assigned roles, this may return false positives.
Expand Down Expand Up @@ -1857,8 +1862,9 @@ func (set RoleSet) CanPortForward() bool {
return false
}

// RecordDesktopSession returns true if a role in the role set has enabled
// desktop session recoring.
// RecordDesktopSession returns true if the role set has enabled desktop
// session recording. Recording is considered enabled if at least one
// role in the set has enabled it.
func (set RoleSet) RecordDesktopSession() bool {
for _, role := range set {
var bo *types.BoolOption
Expand All @@ -1872,6 +1878,18 @@ func (set RoleSet) RecordDesktopSession() bool {
return false
}

// DesktopClipboard returns true if the role set has enabled shared
// clipboard for desktop sessions. Clipboard sharing is disabled if
// one or more of the roles in the set has disabled it.
func (set RoleSet) DesktopClipboard() bool {
for _, role := range set {
if !types.BoolDefaultTrue(role.GetOptions().DesktopClipboard) {
return false
}
}
return true
}

// MaybeCanReviewRequests attempts to guess if this RoleSet belongs
// to a user who should be submitting access reviews. Because not all rolesets
// are derived from statically assigned roles, this may return false positives.
Expand Down
84 changes: 73 additions & 11 deletions lib/services/role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ func TestRoleParse(t *testing.T) {
PortForwarding: types.NewBoolOption(true),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(true)},
BPF: apidefaults.EnhancedEvents(),
DesktopClipboard: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{},
Expand Down Expand Up @@ -224,6 +225,7 @@ func TestRoleParse(t *testing.T) {
PortForwarding: types.NewBoolOption(true),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(true)},
BPF: apidefaults.EnhancedEvents(),
DesktopClipboard: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
Namespaces: []string{apidefaults.Namespace},
Expand All @@ -248,7 +250,8 @@ func TestRoleParse(t *testing.T) {
"port_forwarding": true,
"client_idle_timeout": "17m",
"disconnect_expired_cert": "yes",
"enhanced_recording": ["command", "network"]
"enhanced_recording": ["command", "network"],
"desktop_clipboard": true
},
"allow": {
"node_labels": {"a": "b", "c-d": "e"},
Expand Down Expand Up @@ -291,6 +294,7 @@ func TestRoleParse(t *testing.T) {
ClientIdleTimeout: types.NewDuration(17 * time.Minute),
DisconnectExpiredCert: types.NewBool(true),
BPF: apidefaults.EnhancedEvents(),
DesktopClipboard: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{"a": []string{"b"}, "c-d": []string{"e"}},
Expand Down Expand Up @@ -333,7 +337,8 @@ func TestRoleParse(t *testing.T) {
"forward_agent": "yes",
"client_idle_timeout": "never",
"disconnect_expired_cert": "no",
"enhanced_recording": ["command", "network"]
"enhanced_recording": ["command", "network"],
"desktop_clipboard": true
},
"allow": {
"node_labels": {"a": "b"},
Expand Down Expand Up @@ -374,6 +379,7 @@ func TestRoleParse(t *testing.T) {
ClientIdleTimeout: types.NewDuration(0),
DisconnectExpiredCert: types.NewBool(false),
BPF: apidefaults.EnhancedEvents(),
DesktopClipboard: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{"a": []string{"b"}},
Expand Down Expand Up @@ -414,7 +420,8 @@ func TestRoleParse(t *testing.T) {
"forward_agent": "yes",
"client_idle_timeout": "never",
"disconnect_expired_cert": "no",
"enhanced_recording": ["command", "network"]
"enhanced_recording": ["command", "network"],
"desktop_clipboard": true
},
"allow": {
"node_labels": {"a": "b", "key": ["val"], "key2": ["val2", "val3"]},
Expand Down Expand Up @@ -444,6 +451,7 @@ func TestRoleParse(t *testing.T) {
ClientIdleTimeout: types.NewDuration(0),
DisconnectExpiredCert: types.NewBool(false),
BPF: apidefaults.EnhancedEvents(),
DesktopClipboard: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{
Expand Down Expand Up @@ -2356,37 +2364,44 @@ func TestBoolOptions(t *testing.T) {
outCanPortForward bool
outCanForwardAgents bool
outRecordDesktopSessions bool
outDesktopClipboard bool
}{
// Setting options explicitly off should remain off.
{
inOptions: types.RoleOptions{
ForwardAgent: types.NewBool(false),
PortForwarding: types.NewBoolOption(false),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(false)},
ForwardAgent: types.NewBool(false),
PortForwarding: types.NewBoolOption(false),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(false)},
DesktopClipboard: types.NewBoolOption(false),
},
outCanPortForward: false,
outCanForwardAgents: false,
outRecordDesktopSessions: false,
outDesktopClipboard: false,
},
// Not setting options should set port forwarding to true (default enabled),
// agent forwarding false (default disabled), and
// desktop session recording to true (default enabled)
// agent forwarding false (default disabled),
// desktop session recording to true (default enabled),
// and desktop clipboard sharing to true (default enabled).
{
inOptions: types.RoleOptions{},
outCanPortForward: true,
outCanForwardAgents: false,
outRecordDesktopSessions: true,
outDesktopClipboard: true,
},
// Explicitly enabling should enable them.
{
inOptions: types.RoleOptions{
ForwardAgent: types.NewBool(true),
PortForwarding: types.NewBoolOption(true),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(true)},
ForwardAgent: types.NewBool(true),
PortForwarding: types.NewBoolOption(true),
RecordSession: &types.RecordSession{Desktop: types.NewBoolOption(true)},
DesktopClipboard: types.NewBoolOption(true),
},
outCanPortForward: true,
outCanForwardAgents: true,
outRecordDesktopSessions: true,
outDesktopClipboard: true,
},
}
for _, tt := range tests {
Expand All @@ -2404,6 +2419,7 @@ func TestBoolOptions(t *testing.T) {
require.Equal(t, tt.outCanPortForward, set.CanPortForward())
require.Equal(t, tt.outCanForwardAgents, set.CanForwardAgents())
require.Equal(t, tt.outRecordDesktopSessions, set.RecordDesktopSession())
require.Equal(t, tt.outDesktopClipboard, set.DesktopClipboard())
}
}

Expand Down Expand Up @@ -3226,6 +3242,52 @@ func TestDesktopRecordingEnabled(t *testing.T) {
}
}

func TestDesktopClipboard(t *testing.T) {
for _, test := range []struct {
desc string
roles []types.RoleV4
hasClipboard bool
}{
{
desc: "single role, unspecified, defaults true",
roles: []types.RoleV4{newRole(func(r *types.RoleV4) {})},
hasClipboard: true,
},
{
desc: "single role, explicitly disabled",
roles: []types.RoleV4{
newRole(func(r *types.RoleV4) {
r.SetOptions(types.RoleOptions{
DesktopClipboard: types.NewBoolOption(false),
})
}),
},
hasClipboard: false,
},
{
desc: "multiple conflicting roles, disable wins",
roles: []types.RoleV4{
newRole(func(r *types.RoleV4) {}),
newRole(func(r *types.RoleV4) {
r.SetOptions(types.RoleOptions{
DesktopClipboard: types.NewBoolOption(false),
})
}),
},
hasClipboard: false,
},
} {
t.Run(test.desc, func(t *testing.T) {
var roles []types.Role
for _, r := range test.roles {
roles = append(roles, &r)
}
rs := NewRoleSet(roles...)
require.Equal(t, test.hasClipboard, rs.DesktopClipboard())
})
}
}

func TestCheckAccessToWindowsDesktop(t *testing.T) {
desktopNoLabels := &types.WindowsDesktopV3{
ResourceHeader: types.ResourceHeader{
Expand Down
5 changes: 3 additions & 2 deletions lib/web/resources_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ spec:
deny: {}
options:
cert_format: standard
desktop_clipboard: true
enhanced_recording:
- command
- network
Expand All @@ -130,12 +131,12 @@ version: v3

item, err := ui.NewResourceItem(role)
require.Nil(t, err)
require.Equal(t, item, &ui.ResourceItem{
require.Equal(t, &ui.ResourceItem{
ID: "role:roleName",
Kind: types.KindRole,
Name: "roleName",
Content: contents,
})
}, item)
}

func TestNewResourceItemTrustedCluster(t *testing.T) {
Expand Down

0 comments on commit 69a96d4

Please sign in to comment.