lookup: in capability mode, silently strengthen BENEATH to RBENEATH

[kib]

Summary

Otherwise O_BENEATH allowed to temporary escape the capability sandbox and worked as oracle for checks of existence of directories outside it. This was found by the capsicum-test test checking O_BENEATH behavior in capability mode.

[arichardson]

I would change the commit message to use something like strengthen instead of downgrade since RBENEATH is a stricter version of BENEATH.

sys/kern/vfs_lookup.c

333

Does this need to be cleared? Shouldn't RBENEATH have priority if both are set?

[kib]

sys/kern/vfs_lookup.c

333

Internally they work differently. For BENEATH, there is a concept of 'latch' when path descends below the topping directory. The latch is cleared when we leave the hierarchy.

It must not be tracked or checked for RBENEATH, so flags are used in different code paths. As such it is too problematic to keep them both.

[arichardson]

sys/kern/vfs_lookup.c

312–315

This might be needed if we shouldn't be setting both?

333

Thanks for the explanation. In that case maybe there should be a KASSERT() that we never set both?

[kib]

sys/kern/vfs_lookup.c

312–315

'else' is not strictly needed but would be a good change for clarity.

[kib]

Set only one of OPER_{R,}BENEATH flags.

[arichardson]

sys/kern/vfs_lookup.c

312–315

It seems that adding the else fixed my test case using O_BENEATH|O_RESOLVE_BENEATH, so it does seem to make a functional difference.

440

I guess we never set both now, but maybe this should also come first and be an else if?

[kib]

sys/kern/vfs_lookup.c

440

I do not think that this change would add anything. IMO the current structure is even cleaner than going to if else.

[arichardson]

LGTM with the typo in the commit message fixed (strenghen -> strengthen)

[arichardson]

And maybe mention something like "this was found by the capsicum-test test checking O_BENEATH behaviour in capability mode" in the commit message.

[oshogbo]

sys/sys/namei.h

213

Isn't here a typo?