Re: [PATCH 4/4] pidfs: implement fh_to_dentry

[Christian Brauner <brauner@kernel.org>]

On Wed, Nov 13, 2024 at 11:11:47AM +0100, Erin Shepherd wrote:
> On 13/11/2024 09:01, Amir Goldstein wrote:
> 
> > I don't like playing pseudo cryptographic games, we are not
> > crypto experts so we are bound to lose in this game.
> 
> I agree. It would be one thing to obfusficate things in order to prevent
> userspace from relying upon something that's not ABI; it would be another
> to do so with the intent of hiding data. If we wanted to do that, we'd
> need to actually encrypt the PID (with e.g. AES-CTR(key, iv=inode_nr))
> 
> > My thinking is the other way around -
> > - encode FILEID_INO32_GEN with pid_nr + i_generation
> > - pid_nr is obviously not unique across pidns and reusable
> >   but that makes it just like i_ino across filesystems
> > - the resulting file handle is thus usable only in the pidns where
> >   it was encoded - is that a bad thing?
> >
> > Erin,
> >
> > You write that "To ensure file handles are invariant and can move
> > between pid namespaces, we stash a pid from the initial namespace
> > inside the file handle."
> >
> > Why is it a requirement for userspace that pidfs file handles are
> > invariant to pidns?
> 
> I don't think it's a requirement, but I do think its useful - it is nice if

It kind of is though, no? Because you need a reliable way to decode the
pidfs file handle to a struct pid. If one encodes pid numbers as seen
from the encoders pid namespace the decoder has no way of knowing what
pid namespace to resolve it in as the same pid number can obviously be
present in multiple pid namespace. So not encoding the global pid number
would be inherently ambiguous.

> a service inside a pidns can pass you a file handle and you can restore it and
> things are fine (consider also handles stored on the filesystem, as a better
> analog for PID files)
> 
> But I too was uncertain about exposing root namespace PIDs to containers. I
> have no objections to limiting restore of file handles to the same pid ns -
> though I think we should defnitely document that such a limitation may be
> lifted in the future.

The point is really just the provided pid needs to be resolvable in the
pid namespace of the caller. Encoding a global pid number means that
internally we can always resolve it as we know that we always encode
pids in the init pid namespace.

In a second step we can then verify that the struct pid we found based
on the pid number is a member of the pid namespace hierarchy of the
caller. If that is the case then the caller is allowed to get a pidfd
from open_by_handle_at() as they would also be able to get a pidfd via
pidfd_open().

So a container will never be able to a pidfd from a pid number that
resolves to a struct pid that is outside its pid namespace hierarchy.

Let me know if I misunderstood the concerns.