-
Notifications
You must be signed in to change notification settings - Fork 1k
[Polkadot] Verify the workers at node startup #9716
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
base: master
Are you sure you want to change the base?
Conversation
fixes paritytech#8117 - Added `probe_worker_security` utility to `polkadot/node/core/pvf` to run the prepare-worker binary with special CLI flags (similar to version checks) - Integrated security probes into `determine_workers_paths` so that landlock, seccomp (x86_64 only), unshare+change_root, and secure_clone are validated upfront during node startup - Ensures node aborts early if required Linux kernel security features are missing, rather than discovering issues later in PVF execution. - Test added for probe utility and worker path determination `should_probe_worker_security_successfully` - All probes gated behind `#[cfg(target_os = "linux")]`
20f7490
to
1671437
Compare
#[cfg(target_os = "linux")] | ||
{ | ||
polkadot_node_core_pvf::probe_worker_security( | ||
&prep_worker_path, | ||
"--check-can-enable-landlock", | ||
std::iter::empty::<&str>(), | ||
)?; | ||
|
||
// Seccomp (only on x86_64) | ||
#[cfg(target_arch = "x86_64")] | ||
polkadot_node_core_pvf::probe_worker_security( | ||
&prep_worker_path, | ||
"--check-can-enable-seccomp", | ||
std::iter::empty::<&str>(), | ||
)?; | ||
|
||
// Unshare user namespace + change root needs a temp dir | ||
let tmp = tempfile::Builder::new().prefix("pvf-check-can-unshare-").tempdir()?; | ||
polkadot_node_core_pvf::probe_worker_security( | ||
&prep_worker_path, | ||
"--check-can-unshare-user-namespace-and-change-root", | ||
std::iter::once(tmp.path()), | ||
)?; | ||
|
||
// Secure clone | ||
polkadot_node_core_pvf::probe_worker_security( | ||
&prep_worker_path, | ||
"--check-can-do-secure-clone", | ||
std::iter::empty::<&str>(), | ||
)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of fucked up in my issue description! Sorry :)
These tests that you are calling here are already optional and only enabled when they succeeded before, this is the reason there exist a command line flag for them already.
What I actually want, is that everything in between here: https://github.com/paritytech/polkadot-sdk/blob/d2fd53645654d3b8e12cbf735b67b93078d70113/polkadot/node/core/pvf/common/src/worker/mod.rs#L320-395
That is returning an error, is checked when launching the node. This requires that you introduce a new cli argument that will do these checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah got it, I'll try to get on it around this weekend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification earlier! I wasn’t able to find time over the weekend but I'll try to get this done soon (hopefully within this week).
Description
Fixes #8117
probe_worker_security
utility topolkadot/node/core/pvf
to run the prepare-worker binary with special CLI flags (similar to version checks)determine_workers_paths
so that landlock, seccomp (x86_64 only), unshare+change_root, and secure_clone are validated upfront during node startupshould_probe_worker_security_successfully
#[cfg(target_os = "linux")]
Integration
This PR adds worker security feature probes (landlock, seccomp, unshare+change_root, and secure_clone) during node startup. These checks are executed by calling the prepare-worker binary with CLI flags, in the same way that version checks are already performed.
determine_workers_paths
is unchanged; only its behavior is extended to run additional security checks.Review Notes
This PR extends worker binary validation in determine_workers_paths by adding security capability probes, in addition to the existing version checks.
Previously, determine_workers_paths only:
Now, on Linux hosts, the node also probes security features directly by spawning the worker with special CLI flags. This mirrors the checks already implemented in
security.rs
but ensures they run at worker selection time, before the node starts up fully.Implementation Details
determine_workers_paths
now calls this function after version checks:CI check comments:
I presume that all tests starting with "Zombienet" aren't related to my code, but not 100% sure if "tests linux stable" checks have anything to do with what's introduced in this PR.
Check semver is failing right now, because I thought I should wait for a maintainer's guidance on the versioning to avoid breaking something due to a wrong bump.