search

landlock-lsm / rust-landlock

A Rust library for the Linux Landlock sandboxing feature
https://crates.io/crates/landlock
Other
91 stars 10 forks source link

Multiple `as_mut` implementations prevent automatic type resolution #48

Closed cd-work closed 1 year ago

cd-work commented 1 year ago

Currently RulesetCreated has the following two as_mut implementations:

AsMut<Option<CompatLevel>> for RulesetCreated

impl AsMut<RulesetCreated> for RulesetCreated

I believe the CompatLevel one is new, which causes this previously working code to fail now:

landlock.as_mut().add_rule(rule)?;

This can be easily changed by changing it to (&mut landlock), but I believe the purpose of this as_mut call was to make non-builder usecase like this easier and with the latest Landlock git version that purpose isn't really working out anymore.

This can be easily fixed by changing one or both of these impls to use a non-trait method instead (could even leave the trait impl too if that's necessary). It's not a massive deal but I was just curious if this regression is something you were aware of.

l0kod commented 1 year ago

Oh, I wasn't aware of this change. Could you please create a PR to fix it?

l0kod commented 1 year ago

I'd like to add this fix (with tests) before releasing a new version.

cd-work commented 1 year ago

Thinking about this, I'm honestly not sure if there's a simple and accessible solution to this.

The AsMut<Option<CompatLevel>> can't really be changed easily because it's required for numerous trait resolutions. So the only option would be to change the AsMut<RulesetCreated> for RulesetCreated and provide a method for that. This is also required for some traits though and using the existing as_mut name is probably a bad idea anyway since that's kinda reserved for the trait.

So the only option would be to add a different function that just turns RulesetCreated into &mut RulesetCreated and I don't think there's any solution to that which is easier than (&mut ruleset). Either way people aren't going to find this unless they explicitly look up some documentation that shows off usage like this, since it's behind so many layers of trait obfuscation.

So if 99% of people looking at the API are just going to use a builder or write ruleset = ruleset.handle_access(...); anyway, then it's probably better to not give a separate option to do the same thing and instead just streamline the process with this API.

The only place where this doesn't work so easily is when storing the ruleset on a struct and trying to call add_rule on self, which was my original motivation behind asking for this feature. But just using (&mut self.ruleset) is probably good enough to support this?