[WIP] Enable landlock in cloud-hypervisor #6214
Conversation
liuw
changed the title
|
Changing this to WIP so CI runs are skipped. |
vmm/src/lib.rs
Outdated
| let _ = Landlock::new().unwrap().restrict_self().map_err(Error::ApplyLandlock).map_err(|e| { | ||
| error!("Error applying landlock to signal handler thread: {:?}", e); | ||
| exit_evt.write(1).ok(); | ||
| return; |
There was a problem hiding this comment.
Is the logic correct here? The error from this closure is ignored, but you also write 1 to exit_evt which causes the thread to exit?
There was a problem hiding this comment.
I followed the same logic as the seccomp filter failure above. I will double check this.
| @@ -547,6 +547,12 @@ pub struct TpmConfig { | |||
| pub socket: PathBuf, | |||
| } | |||
|
|
|||
| #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] | |||
| pub struct LandlockConfig { | |||
There was a problem hiding this comment.
This also needs to be exposed via the http endpoint so that Kata and other orchestration layer can use it.
There was a problem hiding this comment.
Agreed. I have to figure how I can order this correctly. Meaning, I need to accept LandlockConfigs sent before vm_create and ignore configs sent after vm_create.
There was a problem hiding this comment.
On second thought, I don't see a lot of value in exposing this via http endpoint. Almost all the config specific rules are self-generated and handled. The only additional rules users have to pass are those for hot-adds.
The Orchestration layer, can only pass landlock-rules before vm_create. It seems fair to let users start cloud-hypervisor with proper --landlock-rules and not worry about receiving additional rules via http endpoint.
Thoughts?
|
@praveen-pk Any plan to write documentation for this feature? |
|
Instead of the |
disk-images is just one of the arguments that takes file paths. There are 13 other arguments that take file paths. So, I kept this argument generic to allow users passing a dir/file path that will be used in any of the other arguments. |
Yes. Once the implementation is complete. |
praveen-pk
force-pushed
the
landlock_draft
branch
2 times, most recently
from
577f855
to
5180e82
Compare
praveen-pk
force-pushed
the
landlock_draft
branch
from
5180e82
to
165793f
Compare
|
With the latest version:
|
praveen-pk
force-pushed
the
landlock_draft
branch
2 times, most recently
from
fe67cc1
to
5bcbabb
Compare
|
With today's push:
|
ARM64 tests are failing. |
|
Can someone help re-trigger the failing ARM tests? On my local Raspberry pi 4, I see gets past build and gets to running unit tests. Also the failure reported doesn't seem to be related the PR. |
| pub const EXECUTE: u8 = 1 << 2; | ||
|
|
||
| #[derive(Debug, Error)] | ||
| pub enum LandlockError { |
There was a problem hiding this comment.
Do we really need this new error type? RulesetError should already contain the required information/types.
There was a problem hiding this comment.
For the most part, the errors here map to ones in RulesetError. The only case that doesn't have an error in RulesetError is when the ruleset is empty. Such a case doesn't map to a proper error in RulesetError.
If you can add such an error case, I will drop LandlockError here.
There was a problem hiding this comment.
The only case that doesn't have an error in
RulesetErroris when the ruleset is empty. Such a case doesn't map to a proper error in RulesetError.If you can add such an error case, I will drop LandlockError here.
An empty ruleset is not an error because it can be used to just drop a set of access rights (without exception), for instance when only using inherited file descriptors.
praveen-pk
force-pushed
the
landlock_draft
branch
2 times, most recently
from
ce09f1c
to
1eac396
Compare
praveen-pk
force-pushed
the
landlock_draft
branch
4 times, most recently
from
429ff3c
to
7c4b483
Compare
landlock syscalls are required by event_monitor, signal_handler, http-server and vmm threads. Rest of the threads are spawned by the vmm thread and they automatically inherit the ruleset from the vmm thread. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Users can use this cmdline option to enable/disable Landlock LSM while starting cloud-hypervisor. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Users can use this parameter to pass extra paths that 'vmm' and its child threads can use at runtime. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
This module introduces methods to apply Landlock LSM to cloud-hypervisor threads. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
vm_config structs with PathBuf elements now have apply_landlock method. These methods will be used to add config specific rules to landlock ruleset. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Add file/dir paths from landlock-rules arguments to ruleset. Invoke apply_landlock on VmConfig to apply config specific rules to ruleset. Once done, any threads spawned by vmm thread will be automatically sandboxed with the ruleset in vmm thread. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
praveen-pk
force-pushed
the
landlock_draft
branch
from
7c4b483
to
642ab3e
Compare
Pty devices are created during vm_boot. To enable Landlock in VMs with pty devices, the devices have to be created during vm_create itself. This commit moves creation of pty devices to vm_create and saves the device info in relevant Configs. During vm_boot, device_manager retrieves the saved device info and uses them as required. With this change Landlock works in VMs with pty devices enabled. Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
praveen-pk
force-pushed
the
landlock_draft
branch
from
642ab3e
to
5cb47f6
Compare
|
With the latest version:
Next:
|
| @@ -1202,12 +1243,58 @@ impl Vmm { | |||
| } | |||
| } | |||
|
|
|||
| /* Create ptys here and add the slave paths to Serial, Console and DebugConsole | |||
| configs. This allows apply_landlok to add the slave paths to landlock rules | |||
| later. | ||
| */ | ||
| fn create_ptys(config: Arc<Mutex<VmConfig>>) -> result::Result<(), LandlockError> { | ||
| if config.lock().unwrap().console.mode == ConsoleOutputMode::Pty { |
There was a problem hiding this comment.
You can do let config = config.lock().unwrap() once at the start of the function.
| @@ -115,7 +121,15 @@ pub struct MemoryZoneConfig { | |||
| #[serde(default)] | |||
| pub prefault: bool, | |||
| } | |||
|
|
|||
| impl MemoryZoneConfig { | |||
| pub fn apply_landlock(self, landlock: &mut Landlock) -> LandlockResult<()> { | |||
There was a problem hiding this comment.
The first parameter should be &self. There is no need to consume the structure.
|
|
||
| if let Some(mem_zones) = self.memory.zones.as_ref() { | ||
| for zone in mem_zones.iter() { | ||
| zone.clone().apply_landlock(&mut landlock)?; |
There was a problem hiding this comment.
Please drop all the clones. See my other comment about &self.
|
#6403 is required to pre-create console devices before |
I am sending this PR for some early feedback on my current approach for enabling Landlock in Cloud-Hypervisor.
Approach
I explicitly enable landlock in
event-monitor,http-server,signal-handleras these threads are spawned beforevmmthread. By the time these 3 threads are spawned, all the necessary FDs these threads need have already been opened. So, I just create landlock object and invokerestrict_selfto sandbox these threads.In the
vmmthread, I enable landlock invm_createmethod. Doing this will allow us to support both the invocation modes:All the paths from VMConfig and LandlockConfig are appended to ruleset before the
vmmthread is sandboxed. Rest of the threads (serial-manager,vcpu*,_disk*,_net*,__rng) are all spawned byvmmthread. So, they will automatically inherit the ruleset applied invmmthread.Known TODOs:
vm_restoreCurrent Testing
Step1
To this VM if a new disk is hot-added with:
Step2
By passing the additional
landlock-rulesargument, hot-add request now passes: