[Virtio-fs] Current file handle status and open questions

[Vivek Goyal]

On Wed, Mar 24, 2021 at 09:06:52AM +0100, Max Reitz wrote:
> On 23.03.21 20:52, Vivek Goyal wrote:
> > On Thu, Mar 18, 2021 at 06:09:58PM +0100, Max Reitz wrote:
> > > Hi,
> > > 
> > > As threatened in our last meeting, I’ve written this mail to give an
> > > overview on where we stand with regards to virtiofsd(-rs) using file
> > > handles.
> > > 
> > > Technically, this should be a reply to the “Securint file handles”
> > > thread, but this mail is so long I think it’s better to split it off.
> > > 
> > > There are multiple problems that somehow relate to file handles, the
> > > ones I’m aware of are:
> > > 
> > > (A) We have a problem with too many FDs open.  To solve it, we could
> > >      attach a file handle to each node, then close the FD (as far as
> > >      possible) and reopen it when needed from the file handle.
> > > 
> > > (B) We want to allow the guest to use persistent file handles.
> > > 
> > > (C) For live migration, the problem isn’t even clear yet, but it seems
> > >      like we’ll want to translate nodes into their file handles and
> > >      transmit those and open them again on the destination (at least on
> > >      shared filesystems).
> > > 
> > > Now every case has its own specific tricky bits:
> > > 
> > > Case (A) is something that we’d really like to have by default, and it
> > > would need to work all the time during runtime.  So the problem here is
> > > that we’d need CAP_DAC_READ_SEARCH, and we’d need it all the time, but
> > > we don’t want that.  One interesting bit is that we don’t need these
> > > file handles to be persistent between virtiofsd invocations.
> > > 
> > > For (B) we have the problem of needing to protect against a potentially
> > > malicious guest, i.e. that it must not be able to reference files
> > > outside the shared directory.  (Perhaps except for cases where the file
> > > was once reachable, i.e. where a file handle was generated by the guest,
> > > then the file was moved outside of the shared directory, but remains
> > > accessible through the file handle.)  Furthermore, file handles should
> > > really be persistent between virtiofsd invocations.  On the positive
> > > side, it would be easier for us to require CAP_DAC_READ_SEARCH for this
> > > case, because it really is optional.  We could require users to give us
> > > that capability if they want file handles in the guest (and we find no
> > > way to avoid requiring that capability).
> > > 
> > > (C) needs persistency between source and destination, but on the
> > > positive side, we only need to be able to open file handles during the
> > > in-migrate phase on the destination.  So requiring CAP_DAC_READ_SEARCH
> > > only during that phase might not be that big of a deal.
> > > 
> > > 
> > > (Ideally, we’d want all cases to work without CAP_DAC_READ_SEARCH, but
> > > as you can see, that requirement is weakened for cases (B) and
> > > especially (C).)
> > > 
> > > 
> > > As far as I’ve understood, (A) is the case that we want to focus on
> > > first, and the main problem there is that we need to open file handles
> > > without CAP_DAC_READ_SEARCH.
> > > 
> > > To do that, I at one point proposed a service process that has
> > > CAP_DAC_READ_SEARCH and would open file handles for virtiofsd.  But that
> > > probably won’t really be an improvement, because this process too would
> > > probably need to run in the container and so if we can’t give virtiofsd
> > > that capability, we can’t give it to that service process either.
> > > 
> > > What Miklos proposed was to modify the kernel to allow processes to open
> > > file handles even if they don’t have CAP_DAC_READ_SEARCH as long as
> > > those files are in the process’s scope.  One way to implement this
> > > restriction (in a very restrictive manner) is to only allow opening file
> > > handles that the process has generated before, e.g. by appending a MAC
> > > to every file handle (generated with a process-specific key) and
> > > checking that when opening a handle.  (You would request this MAC with a
> > > new AT_* flag passed to name_to_handle_at().  open_by_handle_at()
> > > recognizes it due to a special file handle type value.)
> > > 
> > > (Process-specific key = stored in current->files, i.e. the files_struct.
> > > I’m not 100% sure what this is, but I guess this is the structure that
> > > keeps a process’s open file descriptors, and so should generally be
> > > unique to a process, or at least unique to a group of processes that
> > > share the same FDs.)
> > 
> > People are now discussing the idea of doing crash recovery for virtiofsd
> > and pass all the information to systemd(or other process) and get it
> > back once virtiofsd restarts. So tying the key to process life time
> > might not work with crash recovery.
> 
> Well, this would be solved by having the "persistency between virtiofsd
> invocations" I go on to sketch.

Aha.., I did not read the full email and replied early. Will check it
out.

> 
> > May be we can have a kernel system wide keyring and kernel can generate
> > a key and load there and that key can be used for the lifetime of kernel.
> > And every reboot will generate a new key.
> 
> Hm.  The problem I see with a system-wide key, i.e. one that isn’t
> process-specific, is that processes could exchange file handles among each
> other.  Probably not a real problem, because again, if processes can share
> file handles, they can probably do I/O for each other.  OTOH, a file handle
> persists after a process has exited, so maybe this isn’t truly equivalent.
> 
> The nice thing about a global key would be that it would make true
> persistency easier, too, because you could let it be uploaded from an
> arbitrary user space process and don’t have to deal with having to assign it
> to virtiofsd somehow.

If user space were to load a key in kernel, then it has to be out of band
from a different process (and not virtiofsd) which is privileged,
say has CAP_SYS_ADMIN, IIUC.

Otherwise kernel can't trust a unprivileged process to load a key and bypass
CAP_DAC_READ_SEARCH. That will effectively mean, that load a key and
open any file in the filesystem (by crafting the file handle).

> 
> But the question remains whether it is a good idea to have a single key for
> all processes.  I feel uneasy about it.

Not sure. But can't think how per process keys will make it any better
(as opposed to a system wide key).

> 
> Or did you mean that the kernel would generate a new key per process, put it
> into the keyring, and then after a restart the application could select its
> old key again?  I don’t know how you’d verify that the application has the
> right to do so.  I think this would have the same problem as a global key,
> because a process could quit and let some different application use its key
> and its file handles.

No, I did not mean that.

I was thinking along the lines of having a system wide key and just
use that to encrypt/decrypt. And any process in the system will
be able to use that (as long as it requested this by passing a flag
in name_to_handle_at() and open_by_handle_at()).

> 
> > I don't know much about encryption. How does HMAC key look like. Typically
> > with asymmetric keys, we sign kernel modules during build outside the
> > kernel and build public key into the kernel. Is this HMAC key a single
> > key which can be used both for encoding and decoding operation.
> 
> Yes.  You basically combine a single secret key with the message to be
> authenticated, hash it twice, and the resulting hash is the MAC.
> 
> For verification, you do exactly the same process (i.e., it’s the same key)
> and check whether the MAC matches.
> 
> > If same key can be used both for encoding/decoding, then question
> > arises, what are the chances that this key can leak to user space
> > and then user space can artifically encode file handles and be
> > able to open any files. I guess that's the reason TPM kind of things
> > are there to entrust that hardware with private key and nobody else
> > can get to it. (Sorry, I know very little about cryptography, so lot
> > of above might be wrong).
> 
> I don’t understand this question.  We would have the same problem with an
> asymmetric key, right?  The private key could leak to user space.

Generally signing servers (which have private key) are separate and behind
firewall and they don't run general purpose workload. So yes, if one
manages to run malicious process on signing server and somehow has a 
way to get to key, it is possible. But difference here is that server
which is verifying the signature does not have private key (and only
uses public key to verify signature).

In the case of HMAC stuff, same key is being used both for both the
purposes, so we will have to be extra careful that it does not leak
to user space (say, through some sideband channel attack). May be
I am overthinking. 

Thanks
Vivek

> 
> The chances are as high as it is for user space applications to read kernel
> space.  I guess when you upload a key with keyctl() so that no other process
> may read it, you rely on the same protection.
> 
> Max