Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Configurability of cgroup Slice Path #36304

Closed
programmerq opened this issue Jan 4, 2024 · 2 comments · Fixed by #38066
Closed

Allow Configurability of cgroup Slice Path #36304

programmerq opened this issue Jan 4, 2024 · 2 comments · Fixed by #38066
Labels
bpf Used to bugs with bpf and enhanced session recording. bug c-ju Internal Customer Reference

Comments

@programmerq
Copy link
Contributor

Expected behavior:

Users should have the ability to customize the cgroup slice path for processes initiated by teleport exec to allow diverse operational requirements and security policies to be met.

Current behavior:

As per the implementation in lib/cgroup/cgroup.go#L391, the cgroup slice for shell sessions and child processes started by teleport exec is hardcoded to /teleport. This does not accommodate environments with multiple Teleport agents where distinct cgroup slice paths are necessary for proper traffic filtering and security enforcement.

Bug details:

  • Teleport version: The cgroup seems to be hardcoded in all the currently supported teleport versions. Current latest is 14.3.0.
  • Relevant code: The referenced line in codebase where cgroup path is hardcoded is at cgroup.go#L389
  • Suggested improvement: Introduce configuration option in Teleport to specify custom cgroup slice path for enhanced session recording. This would allow for separate handling of sessions for distinct Teleport clusters operated on the same host.

Possible Workarounds?

  • I tried a few different PAM session profile configurations to try to get the pam_systemd.so cgroups to take effect. It appears that Teleport's code specifically waits for PAM cgroup session configuration before setting it to this hardcoded slice. I only tested in an environment with enhanced session recording turned on.
@programmerq programmerq added bug bpf Used to bugs with bpf and enhanced session recording. labels Jan 4, 2024
@zmb3 zmb3 added the c-ju Internal Customer Reference label Jan 14, 2024
@gabrielcorado
Copy link
Contributor

gabrielcorado commented Feb 5, 2024

UPDATE: The following suggestion won't solve the issue as the cgroup hierarchy would be kept the same. This means the /teleport folder would still be the same for both agents.

With the current implementation, multiple agent environments might be problematic if they use the same mount path (defined by ssh_service.enhanced_recording.cgroup_path config) because after one server is shut down , the cgroup hierarchy would be unmounted (except on restart scenarios), causing new sessions from the running agents to fail. Given this, having an option only to rename the /teleport part won't be enough for them to work correctly.

Example of multiple agents same cgroup2 mount path error

How to reproduce: Start both agents on the same machine. After they are all set, close one and try to SSH into the remaining one.

Start both agents on the same machine:

$ teleport start -c agent-1.yaml
$ teleport start -c agent-2.yaml

Close agent-1, keeping only agent-2 running. Then, try to SSH into agent-2:

$ tsh ssh ubuntu@agent-2
EOF

Agent 2 logs:

2024-02-05T19:18:30Z ERRO [SESSION:N] Failed to open enhanced recording (interactive) session error:[
ERROR REPORT:
Original Error: *fs.PathError mkdir /cgroup2/teleport/e27c02b4-ed3e-4005-9182-3cbffc9dc6c2/d6efed80-a539-4be0-8caf-bbd87bbfba73: no such file or directory
Stack Trace:
        github.com/gravitational/teleport/lib/cgroup/cgroup.go:125 github.com/gravitational/teleport/lib/cgroup.(*Service).Create
        github.com/gravitational/teleport/lib/bpf/bpf.go:227 github.com/gravitational/teleport/lib/bpf.(*Service).OpenSession
        github.com/gravitational/teleport/lib/srv/sess.go:1283 github.com/gravitational/teleport/lib/srv.(*session).startInteractive
        github.com/gravitational/teleport/lib/srv/sess.go:345 github.com/gravitational/teleport/lib/srv.(*SessionRegistry).OpenSession
        github.com/gravitational/teleport/lib/srv/termhandlers.go:128 github.com/gravitational/teleport/lib/srv.(*TermHandlers).HandleShell
        github.com/gravitational/teleport/lib/srv/regular/sshserver.go:1740 github.com/gravitational/teleport/lib/srv/regular.(*Server).dispatch
        github.com/gravitational/teleport/lib/srv/regular/sshserver.go:1611 github.com/gravitational/teleport/lib/srv/regular.(*Server).handleSessionRequests
        github.com/gravitational/teleport/lib/srv/regular/sshserver.go:1378 github.com/gravitational/teleport/lib/srv/regular.(*Server).HandleNewChan.func1
        runtime/asm_arm64.s:1197 runtime.goexit
User Message: mkdir /cgroup2/teleport/e27c02b4-ed3e-4005-9182-3cbffc9dc6c2/d6efed80-a539-4be0-8caf-bbd87bbfba73: no such file or directory] session_id:d6efed80-a539-4be0-8caf-bbd87bbfba73 srv/sess.go:1284

Check the /cgroup2 directory is empty:

$ ls /cgroup2


Looking at the mount and cgroupv2 docs, there doesn't seem to be any limitation to duplicating those hierarchies (as they will be the same).

@programmerq Do you think it would be enough for users to set their cgroup_path and be able to configure their traffic filtering and security enforcements?

Sample setup

Agent 1 config:

ssh_service:
  enabled: "yes"
  enhanced_recording:
    enabled: true
    cgroup_path: /cgroup2/agent-1

Agent 2 config:

ssh_service:
  enabled: "yes"
  enhanced_recording:
    enabled: true
    cgroup_path: /cgroup2/agent-2

SSH on both agents:

$ tsh ssh ubuntu@agent-1

$ tsh ssh ubuntu@agent-2

Check on the /cgroup2 structure:

$ ls /cgroup2/
agent-1  agent-2

$ ls /cgroup2/agent-1/
cgroup.controllers      cgroup.subtree_control  dev-hugepages.mount  io.prio.class     misc.capacity                  system.slice
cgroup.max.depth        cgroup.threads          dev-mqueue.mount     io.stat           proc-sys-fs-binfmt_misc.mount  teleport
cgroup.max.descendants  cpu.pressure            init.scope           memory.numa_stat  sys-fs-fuse-connections.mount  tp
cgroup.pressure         cpuset.cpus.effective   io.cost.model        memory.pressure   sys-kernel-config.mount        user.slice
cgroup.procs            cpuset.mems.effective   io.cost.qos          memory.reclaim    sys-kernel-debug.mount
cgroup.stat             cpu.stat                io.pressure          memory.stat       sys-kernel-tracing.mount

$ ls /cgroup2/agent-2/
cgroup.controllers      cgroup.subtree_control  dev-hugepages.mount  io.prio.class     misc.capacity                  system.slice
cgroup.max.depth        cgroup.threads          dev-mqueue.mount     io.stat           proc-sys-fs-binfmt_misc.mount  teleport
cgroup.max.descendants  cpu.pressure            init.scope           memory.numa_stat  sys-fs-fuse-connections.mount  tp
cgroup.pressure         cpuset.cpus.effective   io.cost.model        memory.pressure   sys-kernel-config.mount        user.slice
cgroup.procs            cpuset.mems.effective   io.cost.qos          memory.reclaim    sys-kernel-debug.mount

@arianvp
Copy link

arianvp commented Mar 18, 2024

Note that creating cgroups in the cgroup v2 tree outside of systemd is not really supported. The kernel requires a single writer for the whole cgroup tree.

https://systemd.io/CGROUP_DELEGATION/

Ideally teleport would create its own .scope units in the cgroup tree for subprocesses it spawns. Or if PAM is used this can be automatically be done by pam_systemd.so

See also this comment: #39501 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bpf Used to bugs with bpf and enhanced session recording. bug c-ju Internal Customer Reference
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants