-
Notifications
You must be signed in to change notification settings - Fork 10
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
Clarify default access status #17
Comments
You're correct. I think it should be safe to require
Hmm, using This would replace: let abi = ABI::V1;
Ruleset::new()
.handle_access(AccessFs::from_all(abi))?
.create()?
.add_rules(path_beneath_rules(&["/usr", "/etc", "/dev"], AccessFs::from_read(abi)))?
.restrict_self()? …with something like this: Ruleset::new(ABI::V1)
.create()?
.add_rules(path_beneath_rules(&["/usr", "/etc", "/dev"], Access::Read))?
.restrict_self()? The current way do to it could still be usable, but for very custom needs. I need to merge #12 before any other changes though. |
I don't think passing ABI to |
Currently, the
handle_access
method states the following:This implies that if something is handled, without adding exceptions as rules, access to it will be denied. However I haven't found any place that explicitly states how things behave when something isn't handled. I think it would be beneficial to have this pointed out on
handle_access
orRuleset::new
in a rustdoc# Safety
block, just to ensure people don't make assumptions that lead to security issues.Right now the default behavior seems to be that things which aren't handled are allowed. So creating a ruleset with
handle_access(AccessFs::from_write(abi))
without restricting read, would allow full read access. I could easily see how this might trip someone up when they're just trying to allow some read access without allowing any write access for example.I think it might also be worth considering if this is the right default. I personally think this library would be much harder to use incorrectly when just
Ruleset::new().create().unwrap().restrict_self().unwrap()
was enough to deny all filesystem access. Especially when taking possible future changes into account (you wouldn't want new stuff to be allowed by default).The text was updated successfully, but these errors were encountered: