Confusing behavior: allow_stdin overrides allow_read

[picoHz]

Hi.

I was really puzzled why reading from a file failed with a combination of allow_stdin and allow_read and eventually found that allow_stdin negated allow_read.

I think it's confusing and the documentation should make it clear that those methods are exclusive.

[boustrophedon]

Hi! Thanks for the issue.

I agree the documentation needs to be more clear about what happens when two rules have overlapping syscalls.

What's happening is that allow_stdin allows read in the specific case when the fd parameter is 0, whereas allow_read allows read syscalls unconditionally. When you pass both of these rules to libseccomp, it discards the allow_stdin specific rule and allows all reads.

In this specific case I could also add some validation to the SystemIO methods as well, but in the general case two different RuleSets can both enable read so it would still be necessary to do global validation.In this specific case I could also add some validation to the SystemIO methods as well, but in the general case two different RuleSets can both enable read so it would still be necessary to do global validation.

I thought this behavior wasn't great for extrasafe because it's easy to accidentally enable read globally with some other ruleset but mistakenly believe (due to calling allow_stdin) that it's disabled except for the specific cases you allowed.

Do you think printing a warning to stderr would be useful here, besides improving the documentation? "A parameterized rule would have been overridden by the current rule, so the global rule is being ignored."

In this specific case I could also add some validation to the SystemIO methods as well, but in the general case two different RuleSets can both enable read so it would still be necessary to do global validation.

[picoHz]

Thank you for your reply.

Is it possible to just return ConditionalNoEffectError in this case?
I think it is the most consistent behavior.

Do you think printing a warning to stderr would be useful here, besides improving the documentation? "A parameterized rule would have been overridden by the current rule, so the global rule is being ignored."

I don't think it's a good idea because it won't work unless stderr is allowed.
It works before activating seccomp but I still think it is a bit contradictory that the library itself uses stderr when I want to restrict the write syscall.

[boustrophedon]

I don't think it's a good idea because it won't work unless stderr is allowed.

That's a very good point :)

Returning ConditionalNoEffectError in this case might be the best option since you're the second person to bring it up. I was also briefly exploring switching to seccompiler from libseccomp which might make it easier to control this behavior - as far as I'm aware it's not inherent to seccomp itself, just how libseccomp generates its bpf programs.

Another option would be adding a typestate <ReadParameterized, WriteParameterized, ...etc.> parameter to the SystemIO struct that would not implement calling allow_read after calling allow_stdin but would still allow allow_stdout and allow_fd etc, but that's a lot of work and would have to be duplicated for any other ruleset that works like this, and wouldn't be there by default for user-defined rulesets.

Another option might be adding a .strict_mode() that detects this case and errors only when strict mode is enabled, but I don't really like the idea of having a strict mode - it would be confusing and the current mode isn't particularly unstrict anyway.

I guess given that the user can always define their own RuleSet it's okay to error in this case as well.

[boustrophedon]

Hi, I'm sorry it's taken me a year to get back to work on this project. I just pushed a branch with some cleanups I've been working on in preparation for seccompiler and landlock. I think it solves the issue.

Also, it looks like my first comment here somehow got corrupted or maybe I was just very tired - there's an edit on it that seems strange. ¯\(ツ)/¯`

[boustrophedon]

I'm going to close this since you thumbs-up'd my previous comment.

I'm also going to push a new release to crates.io today. If you run into more issues with this feel free to reopen this issue or make a new one!

Thanks again for the report, and sorry it took me so long to get back to it.