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

Disable the usage of ptrace() by all processes #242

Merged
merged 3 commits into from
Jul 28, 2024

Conversation

raja-grewal
Copy link
Contributor

Upgrade existing sysctl to now disable ptrace() usage by all processes.

Previously it was restricted to users with CAP_SYS_PTRACE.

It is unlikely that this would cause significantly more software breakages than the previous setting.

Needs to be thoroughly tested before being upgraded.

Changes

kernel.yama.ptrace_scope=2
to
kernel.yama.ptrace_scope=3

Mandatory Checklist

  • Legal agreements accepted. By contributing to this organisation, you acknowledge you have read, understood, and agree to be bound by these these agreements:

Terms of Service, Privacy Policy, Cookie Policy, E-Sign Consent, DMCA, Imprint

Optional Checklist

The following items are optional but might be requested in certain cases.

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

@raja-grewal raja-grewal marked this pull request as ready for review July 18, 2024 01:33
@raja-grewal
Copy link
Contributor Author

@adrelanos
Copy link
Member

Being restricted to CAP_SYS_PTRACE seems sufficient?

@raja-grewal
Copy link
Contributor Author

It probably is for most purposes but one flaw in the current approach is that it can be changed through sysctl on a running environment.

If set to =3, the sysctl value cannot be changed until reboot.

I think this is a very important reduction in attack surface as it will require someone to change the actual configuration files and reboot to enable ptrace().

@adrelanos
Copy link
Member

It probably is for most purposes but one flaw in the current approach is that it can be changed through sysctl on a running environment.

But that's okay because that requires root and for now, as soon as root is compromised it's considered game over anyhow?

@raja-grewal
Copy link
Contributor Author

raja-grewal commented Jul 19, 2024

Yes and no. While obtaining root is obviously game over, in the same way as a root user you can not turn off CPU mitigations until reboot, I think ptrace() should be covered by the same degree of protections since its ability to mess with other running applications is quite nasty.

This will be a good form of defence-in-depth as any user that is considering changing it will as a byproduct be educated on its purpose and it is restricted.

There are plenty of online forum tutorials dealing with broken applications that all suggest enabling ptrace(), yet as far as I am aware, none of them mention anything to do with the severe security implications.

This is particularly important on desktop Linux as enabling ptrace() will allow for its use by all programs. This is in strong contrast with say GrapheneOS where it can disabled globally by default and then sensibly enabled on a per-app basis.

@adrelanos
Copy link
Member

There are probably a ton of settings which are unsafe to change.

If there was an option to lock all settings until reboot (I think there is not), should we do that?

User education by locking settings and requiring reboot seems out-of-scope of security-misc.

Related:

@raja-grewal
Copy link
Contributor Author

Ok I think I understand your reasoning and agree that educating users by locking till a reboot is outside the scope of security-misc.

While I still personally think that ptrace() should be restricted till reboot, I respect your decision to not enforce this across the board.

I have reverted the commit and posted a link to this PR so that this discussion can be logged for future reference.

@adrelanos adrelanos merged commit 0f86fbd into Kicksecure:master Jul 28, 2024
@raja-grewal raja-grewal deleted the ptrace branch August 2, 2024 03:55
@raja-grewal
Copy link
Contributor Author

As an additional reference, KSPP also currently recommends disabling the usage of ptrace() by all processes unless it is very explicitly required by a user.

https://kspp.github.io/Recommended_Settings#sysctls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants