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

RFD for official SELinux policy for SSH agents #52529

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

capnspacehook
Copy link
Contributor

No description provided.

@github-actions github-actions bot added rfd Request for Discussion size/sm labels Feb 26, 2025
@github-actions github-actions bot requested review from avatus and Tener February 26, 2025 21:05
@avatus
Copy link
Contributor

avatus commented Feb 27, 2025

I'll get to this later today, but is there anyone else more knowledgable than me that you can add to the reviewers?

@avatus avatus requested review from r0mant and rosstimothy February 27, 2025 14:54
@rosstimothy rosstimothy requested a review from Joerger February 27, 2025 14:54
@capnspacehook capnspacehook force-pushed the capnspacehook/rfd/selinux-ssh-agent branch from 53df916 to fcbd355 Compare February 27, 2025 16:05
- The intermediary process will then create the child session process, which only needs standard permissions of the host user that the SSH session
was created for.

Additionally, the SSH agent may create a networking child process to handle X11 forwarding, port forwarding, and SSH Agent forwarding. This process is a child process of the intermediary process as well.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The networking process is actually started by the parent process, if that changes anything.

Suggested change
Additionally, the SSH agent may create a networking child process to handle X11 forwarding, port forwarding, and SSH Agent forwarding. This process is a child process of the intermediary process as well.
Additionally, the SSH agent may create a networking child process to handle X11 forwarding, port forwarding, and SSH Agent forwarding. This process is a child process of the parent process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is but the immediate child of the SSH server is the intermediate process that then creates another child that actually preforms the networkings tasks as far as I can tell, is that incorrect?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite, that's only the case for the teleport exec command which launches a child exec/shell command.

For the networking command, the main SSH server process reexecs itself with teleport networking, which starts the networking process. The networking process doesn't spawn any child processes, it just handles networking requests directly.

- Enabling this will allow the `teleport_ssh_agent` domain to call necessary binaries such as `useradd`, `usermod` and `groupadd`
- Enhanced session recording with BPF
- Enabling this will allow the `teleport_ssh_agent` domain to create and manage BPF ring buffers, and load BPF programs
- Creation of PAM contexts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this SELinux policy interact with arbitrary/custom PAM modules, like the one we used to use for user creation? Would users need to "fork" our policy to support specific use cases like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They shouldn't have to, I added a bit of info on this to the RFD. From reading the docs you linked it seems like we require the PAM module to be in a specific location, so that shouldn't be difficult to support arbitrary modules

Comment on lines +101 to +104
Install the policy in permissive mode before running through test cases that involve the SSH service. Check that exercising the test cases do not
create any policy violations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably would want to create a separate test plan section so that several testers aren't burdened with setting up RHEL 7 for testing. We only need to exercise features with direct OS interactions that could be affected by SELinux policies, so should rule out things like per-session MFA or moderated sessions, which just add server-side logic, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, added this to the RFD

Joerger

This comment was marked as resolved.

@Joerger Joerger dismissed their stale review February 27, 2025 20:05

Accidental approval, then requested changes instead of dismissing approval.

@Tener Tener removed their request for review February 28, 2025 07:37
@capnspacehook capnspacehook force-pushed the capnspacehook/rfd/selinux-ssh-agent branch from fcbd355 to 83ae922 Compare February 28, 2025 17:42
@capnspacehook capnspacehook requested a review from Joerger March 5, 2025 01:28
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the UX for someone that makes changes to the ssh_service config when using our policies? Are they expected to run teleport selinux after making any changes to their config file? Is teleport start --selinux going to catch any deviations between the active policy and the active config options? If not, would SELinux prevent accessing the host because the policy doesn't permit the updated feature set specified in the config file?

Similarly, what happens if we update our policy in newer versions of Teleport?

Comment on lines +79 to +81
The SELinux policy will be updated in the future to support new features and maintain compatibility with existing features.
Any changes to the SSH service that result in new files being managed, new network activity (listening on new ports, dialing new addresses etc),
new binaries being executed, or new Linux syscalls being called will require updates to the SELinux policy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will we identify that we've broken an existing SELinux policy with a change to an existing feature or when adding a new feature?


### Installation and updating

The policy source will be embedded in future Teleport agent binaries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How large are the policies? Will they have any measurable impact on binary size?

The policy source will be embedded in future Teleport agent binaries.

A `selinux` subcommand will be added to Teleport agent binaries to allow users to install and configure the SELinux policy. This subcommand
will will use the Teleport agent configuration file to determine how to configure some of the tunable SELinux policy contexts, such as port
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
will will use the Teleport agent configuration file to determine how to configure some of the tunable SELinux policy contexts, such as port
will use the Teleport agent configuration file to determine how to configure some of the tunable SELinux policy contexts, such as port


The policy source will be embedded in future Teleport agent binaries.

A `selinux` subcommand will be added to Teleport agent binaries to allow users to install and configure the SELinux policy. This subcommand
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will users have any insight into what this command will output? Will there be a dry run mode or a way for users to examine the policy before it is applied?

Comment on lines +108 to +113
To ensure the SSH agent SELinux policy is installed and SELinux is configured to enforce it, a `--selinux` flag will be added to the `teleport start` subcommand
of Teleport agents. If the SELinux policy is not installed or SELinux is not present and in enforcing mode and `--selinux` is passed, the Teleport agent
will exit with an error.

Additionally since the SSH service is the only Teleport agent service that will be initially supported by the policy, if the `--selinux` flag is passed
and the Teleport agent does not have the SSH service enabled, or at least one other Teleport service is enabled then the Teleport agent will exit with an error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the policy is removed or tampered with after the initial check? Do we care? What if a policy is installed but it is out of date or is not a policy that was installed via teleport selinux command?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfd Request for Discussion size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants