overlayfs vs. fscrypt

overlayfs vs. fscrypt

[Richard Weinberger]

Hi!

overlayfs and fscrypt are not friends.
Currently it is not possible to use a fscrypt encrypted directory as upper 
directory with overlayfs.
The reason for that is, fscrypt implements ->d_revalidate().

From fscrypt's point of view having ->d_revalidate() makes sense because it 
wants to hide/show encrypted filenames if someone loads or unlinks a key.

On the other hand, overlayfs makes sure that the upper directory cannot
change beneath it. Therefore it checks whether the upper directory is a remote 
filesystem by checking for ->d_revalidate() and refuses to mount if so.

In my little embedded Linux world it is common to use both UBIFS and 
overlayfs. Now with UBIFS being encrypted using fscrypt, overlayfs is a 
problem.
My current hack is not using fscrypt_d_ops in UBIFS. This works because on a 
typical embedded target you setup your crypto keys exactly once, right before 
you mount overlayfs in an initramfs.

But I'm sure this problem will hit sooner or later users of ext4 and f2fs too.
Therefore I'd like to discuss possible solutions.

So far I see two options:

1. Get rid of ->d_revalidate() in fscrypt.
Maybe we find a way to return a dentry via ->lookup() which is not cached at 
all and therefore no ->d_revalidate() is needed. If unreadable and encrypted
filename lookups are slow, so what?
AFAIU this approach is impossible in the current dcache design since it is not 
allowed to have more than one dentry to the same file.

2. Teach overlayfs to deal with a upper that has ->d_revalidate().
Given the complexity of overlayfs I'm not sure how feasible this is.
But I'm no overlayfs expert, maybe I miss something.

What else could we do?

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Miklos Szeredi]

On Wed, Mar 13, 2019 at 1:31 PM Richard Weinberger <richard@nod.at> wrote:
>
> Hi!
>
> overlayfs and fscrypt are not friends.
> Currently it is not possible to use a fscrypt encrypted directory as upper
> directory with overlayfs.
> The reason for that is, fscrypt implements ->d_revalidate().
>
> From fscrypt's point of view having ->d_revalidate() makes sense because it
> wants to hide/show encrypted filenames if someone loads or unlinks a key.
>
> On the other hand, overlayfs makes sure that the upper directory cannot
> change beneath it. Therefore it checks whether the upper directory is a remote
> filesystem by checking for ->d_revalidate() and refuses to mount if so.
>
> In my little embedded Linux world it is common to use both UBIFS and
> overlayfs. Now with UBIFS being encrypted using fscrypt, overlayfs is a
> problem.
> My current hack is not using fscrypt_d_ops in UBIFS. This works because on a
> typical embedded target you setup your crypto keys exactly once, right before
> you mount overlayfs in an initramfs.
>
> But I'm sure this problem will hit sooner or later users of ext4 and f2fs too.
> Therefore I'd like to discuss possible solutions.
>
> So far I see two options:
>
> 1. Get rid of ->d_revalidate() in fscrypt.
> Maybe we find a way to return a dentry via ->lookup() which is not cached at
> all and therefore no ->d_revalidate() is needed. If unreadable and encrypted
> filename lookups are slow, so what?
> AFAIU this approach is impossible in the current dcache design since it is not
> allowed to have more than one dentry to the same file.

I don't get it.  Does fscrypt try to check permissions via
->d_revalidate?  Why is it not doing that via ->permission()?

>
> 2. Teach overlayfs to deal with a upper that has ->d_revalidate().
> Given the complexity of overlayfs I'm not sure how feasible this is.
> But I'm no overlayfs expert, maybe I miss something.

I don't think it would be too complex.  But first I'd like to
understand exactly why fscrypt is (ab) using d_revalidate().

Thanks,
Miklos

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 13:36:02 CET schrieb Miklos Szeredi: 
> I don't get it.  Does fscrypt try to check permissions via
> ->d_revalidate?  Why is it not doing that via ->permission()?

Please let me explain. Suppose we have a fscrypto directory /mnt and
I *don't* have the key.

When reading the directory contents of /mnt will return an encrypted filename.
e.g.
# ls /mnt
+mcQ46ne5Y8U6JMV9Wdq2C

As soon I load my key the real name is shown and I can read the file contents too.
That's why fscrypt has ->d_revalidate(). It checks for the key, if the key is
still not here -> stay with the old encrypted name. If the key is present
-> reveal the real name.

Same happens on the other direction if I unlink my key from the keyring.

> >
> > 2. Teach overlayfs to deal with a upper that has ->d_revalidate().
> > Given the complexity of overlayfs I'm not sure how feasible this is.
> > But I'm no overlayfs expert, maybe I miss something.
> 
> I don't think it would be too complex.  But first I'd like to
> understand exactly why fscrypt is (ab) using d_revalidate().

I hope my answer makes things more clear.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Miklos Szeredi]

On Wed, Mar 13, 2019 at 1:47 PM Richard Weinberger <richard@nod.at> wrote:
>
> Am Mittwoch, 13. März 2019, 13:36:02 CET schrieb Miklos Szeredi:
> > I don't get it.  Does fscrypt try to check permissions via
> > ->d_revalidate?  Why is it not doing that via ->permission()?
>
> Please let me explain. Suppose we have a fscrypto directory /mnt and
> I *don't* have the key.
>
> When reading the directory contents of /mnt will return an encrypted filename.
> e.g.
> # ls /mnt
> +mcQ46ne5Y8U6JMV9Wdq2C

Why does showing the encrypted contents make any sense?  It could just
return -EPERM on all operations?

Thanks,
Miklos

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 13:58:11 CET schrieb Miklos Szeredi:
> On Wed, Mar 13, 2019 at 1:47 PM Richard Weinberger <richard@nod.at> wrote:
> >
> > Am Mittwoch, 13. März 2019, 13:36:02 CET schrieb Miklos Szeredi:
> > > I don't get it.  Does fscrypt try to check permissions via
> > > ->d_revalidate?  Why is it not doing that via ->permission()?
> >
> > Please let me explain. Suppose we have a fscrypto directory /mnt and
> > I *don't* have the key.
> >
> > When reading the directory contents of /mnt will return an encrypted filename.
> > e.g.
> > # ls /mnt
> > +mcQ46ne5Y8U6JMV9Wdq2C
> 
> Why does showing the encrypted contents make any sense?  It could just
> return -EPERM on all operations?

The use case is that you can delete these files if the DAC/MAC permissions allow it.
Just like on NTFS. If a user encrypts files, the admin cannot read them but can
remove them if the user is gone or loses the key.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Miklos Szeredi]

On Wed, Mar 13, 2019 at 2:00 PM Richard Weinberger <richard@nod.at> wrote:
>
> Am Mittwoch, 13. März 2019, 13:58:11 CET schrieb Miklos Szeredi:
> > On Wed, Mar 13, 2019 at 1:47 PM Richard Weinberger <richard@nod.at> wrote:
> > >
> > > Am Mittwoch, 13. März 2019, 13:36:02 CET schrieb Miklos Szeredi:
> > > > I don't get it.  Does fscrypt try to check permissions via
> > > > ->d_revalidate?  Why is it not doing that via ->permission()?
> > >
> > > Please let me explain. Suppose we have a fscrypto directory /mnt and
> > > I *don't* have the key.
> > >
> > > When reading the directory contents of /mnt will return an encrypted filename.
> > > e.g.
> > > # ls /mnt
> > > +mcQ46ne5Y8U6JMV9Wdq2C
> >
> > Why does showing the encrypted contents make any sense?  It could just
> > return -EPERM on all operations?
>
> The use case is that you can delete these files if the DAC/MAC permissions allow it.
> Just like on NTFS. If a user encrypts files, the admin cannot read them but can
> remove them if the user is gone or loses the key.

There's the underlying filesystem view where admin can delete files,
etc.   And there's the fscrypt layer stacked on top of the underlying
fs, which en/decrypts files *in case the user has the key*.  What if
one user has a key, but the other one doesn't?  Will d_revalidate
constantly switch the set of dentries between the encrypted filenames
and the decrypted ones?  Sounds crazy.  And the fact that NTFS does
this doesn't make it any less crazy...

Thanks,
Miklos

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 14:24:47 CET schrieb Miklos Szeredi:
> > The use case is that you can delete these files if the DAC/MAC permissions allow it.
> > Just like on NTFS. If a user encrypts files, the admin cannot read them but can
> > remove them if the user is gone or loses the key.
> 
> There's the underlying filesystem view where admin can delete files,
> etc.   And there's the fscrypt layer stacked on top of the underlying
> fs, which en/decrypts files *in case the user has the key*.  What if
> one user has a key, but the other one doesn't?  Will d_revalidate
> constantly switch the set of dentries between the encrypted filenames
> and the decrypted ones?  Sounds crazy.  And the fact that NTFS does
> this doesn't make it any less crazy...

Well, I didn't come up with this feature. :-)

If one user has the key and the other not, a classic multi-user
system, then you need to make sure that the affected fscrypt instances
are not visible by both.
For example by using mount namespaces to make sure that user a can only
see /home/foo and user b only /home/bar.
Or removing the search permission on /home/foo and /home/bar.

I know, I know, but that's how it is...
Maybe Ted or Eric can give more details on why they chose this approach.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Amir Goldstein]

On Wed, Mar 13, 2019 at 3:34 PM Richard Weinberger <richard@nod.at> wrote:
>
> Am Mittwoch, 13. März 2019, 14:24:47 CET schrieb Miklos Szeredi:
> > > The use case is that you can delete these files if the DAC/MAC permissions allow it.
> > > Just like on NTFS. If a user encrypts files, the admin cannot read them but can
> > > remove them if the user is gone or loses the key.
> >
> > There's the underlying filesystem view where admin can delete files,
> > etc.   And there's the fscrypt layer stacked on top of the underlying
> > fs, which en/decrypts files *in case the user has the key*.  What if
> > one user has a key, but the other one doesn't?  Will d_revalidate
> > constantly switch the set of dentries between the encrypted filenames
> > and the decrypted ones?  Sounds crazy.  And the fact that NTFS does
> > this doesn't make it any less crazy...
>
> Well, I didn't come up with this feature. :-)
>
> If one user has the key and the other not, a classic multi-user
> system, then you need to make sure that the affected fscrypt instances
> are not visible by both.
> For example by using mount namespaces to make sure that user a can only
> see /home/foo and user b only /home/bar.
> Or removing the search permission on /home/foo and /home/bar.
>
> I know, I know, but that's how it is...
> Maybe Ted or Eric can give more details on why they chose this approach.
>

AFAIK, this feature was born to tailor Android's file based encryption.
https://source.android.com/security/encryption#file-based
It is meant to protect data at rest and what happens when user enters
the screen lock password IIRC, is that some service will get restarted.
IOW, there should NOT be any processes in Android accessing the
encrypted user data folders with and without the key simultaneously.
Also, like OpenWRT, in Android the key does not get removed
(until boot) AFAIK(?).

That dcache behavior remind me of the proposal to make case
insensitive a per mount option (also for an Android use case).
Eventually, that was replaced with per directory flag, which plays
much better with dache.

IMO, the best thing for UBIFS to do would be to modify fscrypt to support
opting out of the revalidate behavior, IWO, sanitize your hack to an API.

It's good that you are thinking about what will happen with overlayfs
over ext4/f2fs, but I think that it will be messy if dentry names would be
changing in underlying fs and the fact the overlayfs accessed the underlying
dirs with different credentials at times makes this even more messy.

The way out of this mess IMO would be for ext4/f2fs to also conditionally
opt-out of d_revalidate behavior at mount time if the fs is expected to be
used under overlayfs.
In Android, for example, I think the use case of "admin deleting
the encrypted directories" is only relevant on "reset to default" and that
happens in recovery boot that could potentially opt-out of encryption
altogether (because there is no user to enter the password anyway).

I could be over simplifying things for the Android use case and my
information could be severely out dated.
CC Paul Lawrence to fill in my Android knowledge gaps.

Thanks,
Amir.

Re: overlayfs vs. fscrypt

[Theodore Ts'o]

On Wed, Mar 13, 2019 at 04:26:54PM +0200, Amir Goldstein wrote:
> AFAIK, this feature was born to tailor Android's file based encryption.
> https://source.android.com/security/encryption#file-based
> It is meant to protect data at rest and what happens when user enters
> the screen lock password IIRC, is that some service will get restarted.
> IOW, there should NOT be any processes in Android accessing the
> encrypted user data folders with and without the key simultaneously.
> Also, like OpenWRT, in Android the key does not get removed
> (until boot) AFAIK(?).

Actually, the original use was for ChromeOS, but the primary
assumption is that keying is per user (or profile), and that users are
mutually distrustful.  So when Alice logs out of the system, her keys
will be invalidated and removed from the kernel.  We can (and do) try
to flush cache entries via "echo 3 > /proc/sys/vm/drop_caches" on
logout.  However, this does not guarantee that all dcache entries will
be removed --- a dcache entry can be pinned due to an open file, a
process's current working directory, a bind mount, etc.

The other issue is negative dentries; if you try open a file in an
encrypted file, the file system won't even *know* whether or not a
file exists, since the directory entries are encrypted; hence, there
may be some negative dentries that need to be invalidated.

So a fundamental assumption with fscrypt is that keys will be added
and removed, and that when this happens, dentries will need to be
invalidated.  This is going to surprise overlayfs, so if overlayfs is
going to support fscrypt it *has* to be aware of the fact that this
can happen.  It's not even clear what the proper security semantics
should be; *especially* if the upper and lower directories aren't
similarly protected using the same fscrypt encryption key.  Suppose
the lower directory is encrypted, and the upper is not.  Now on a copy
up operation, the previously encrypted file, which might contain
credit card numbers, medical records, or other things that would cause
a GDPR regulator to have a freak out attack, would *poof* become
decrypted.

So before we talk about how to make things work from a technical
perspective, we should consider what the use case happens to be, and
what are the security requirements.  *Why* are we trying to use the
combination of overlayfs and fscrypt, and what are the security
properties we are trying to provide to someone who is relying on this
combination?

						- Ted

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 16:16:33 CET schrieb Theodore Ts'o:
> So before we talk about how to make things work from a technical
> perspective, we should consider what the use case happens to be, and
> what are the security requirements.  *Why* are we trying to use the
> combination of overlayfs and fscrypt, and what are the security
> properties we are trying to provide to someone who is relying on this
> combination?

Well, as stated, on (deeply) embedded systems overlayfs is common.
You have a lowerdir with read-only files and an read-write upper dir.
Of course both lower and upper directory need to be encrypted.
In my case ubifs+fscrypt, sometimes also combined with an encrypted+authenticated
squashfs.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 11:16 -0400, Theodore Ts'o wrote:
> So before we talk about how to make things work from a technical
> perspective, we should consider what the use case happens to be, and
> what are the security requirements.  *Why* are we trying to use the
> combination of overlayfs and fscrypt, and what are the security
> properties we are trying to provide to someone who is relying on this
> combination?

I can give one: encrypted containers:

https://github.com/opencontainers/image-spec/issues/747

The current proposal imagines that the key would be delivered to the
physical node and the physical node containerd would decrypt all the
layers before handing them off to to the kubelet.  However, one could
imagine a slightly more secure use case where the layers were
constructed as an encrypted filesystem tar and so the key would go into
the kernel and the layers would be constructed with encryption in place
using fscrypt.

Most of the desired security properties are in image at rest but one
can imagine that the running image wants some protection against
containment breaches by other tenants and using fscrypt could provide
that.

James

Re: overlayfs vs. fscrypt

[Eric Biggers]

Hi James,

On Wed, Mar 13, 2019 at 08:36:34AM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 11:16 -0400, Theodore Ts'o wrote:
> > So before we talk about how to make things work from a technical
> > perspective, we should consider what the use case happens to be, and
> > what are the security requirements.  *Why* are we trying to use the
> > combination of overlayfs and fscrypt, and what are the security
> > properties we are trying to provide to someone who is relying on this
> > combination?
> 
> I can give one: encrypted containers:
> 
> https://github.com/opencontainers/image-spec/issues/747
> 
> The current proposal imagines that the key would be delivered to the
> physical node and the physical node containerd would decrypt all the
> layers before handing them off to to the kubelet.  However, one could
> imagine a slightly more secure use case where the layers were
> constructed as an encrypted filesystem tar and so the key would go into
> the kernel and the layers would be constructed with encryption in place
> using fscrypt.
> 
> Most of the desired security properties are in image at rest but one
> can imagine that the running image wants some protection against
> containment breaches by other tenants and using fscrypt could provide
> that.
> 

What do you mean by "containment breaches by other tenants"?  Note that while
the key is added, fscrypt doesn't prevent access to the encrypted files.
fscrypt is orthogonal to OS-level access control (UNIX mode bits, ACLs, SELinux,
etc.), which can and should be used alongside fscrypt.  fscrypt is a storage
encryption mechanism, not an OS-level access control mechanism.

- Eric

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 08:51 -0700, Eric Biggers wrote:
> Hi James,
> 
> On Wed, Mar 13, 2019 at 08:36:34AM -0700, James Bottomley wrote:
> > On Wed, 2019-03-13 at 11:16 -0400, Theodore Ts'o wrote:
> > > So before we talk about how to make things work from a technical
> > > perspective, we should consider what the use case happens to be,
> > > and what are the security requirements.  *Why* are we trying to
> > > use the combination of overlayfs and fscrypt, and what are the
> > > security properties we are trying to provide to someone who is
> > > relying on this combination?
> > 
> > I can give one: encrypted containers:
> > 
> > https://github.com/opencontainers/image-spec/issues/747
> > 
> > The current proposal imagines that the key would be delivered to
> > the physical node and the physical node containerd would decrypt
> > all the layers before handing them off to to the kubelet.  However,
> > one could imagine a slightly more secure use case where the layers
> > were constructed as an encrypted filesystem tar and so the key
> > would go into the kernel and the layers would be constructed with
> > encryption in place using fscrypt.
> > 
> > Most of the desired security properties are in image at rest but
> > one can imagine that the running image wants some protection
> > against containment breaches by other tenants and using fscrypt
> > could provide that.
> > 
> 
> What do you mean by "containment breaches by other tenants"?  Note
> that while the key is added, fscrypt doesn't prevent access to the
> encrypted files.

You mean it's not multiuser safe?  Even if user a owns the key they add
user b can still see the decrypted contents?

>   fscrypt is orthogonal to OS-level access control (UNIX mode bits,
> ACLs, SELinux, etc.), which can and should be used alongside
> fscrypt.  fscrypt is a storage encryption mechanism, not an OS-level
> access control mechanism.

I was assuming in the multi-user case that if you don't own the keyring
you can't see the files. I suppose absent that it boils down to a
possible way to do the layering then as an fscrypt image rather than
tar then encrypt.

James

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 17:13:52 CET schrieb James Bottomley:
> > What do you mean by "containment breaches by other tenants"?  Note
> > that while the key is added, fscrypt doesn't prevent access to the
> > encrypted files.
> 
> You mean it's not multiuser safe?  Even if user a owns the key they add
> user b can still see the decrypted contents?

If user a reads the file before, yes. Then user b sees it because the contents
got cached.
That's why you need still make sure that your access control is sane.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Theodore Ts'o]

On Wed, Mar 13, 2019 at 08:36:34AM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 11:16 -0400, Theodore Ts'o wrote:
> > So before we talk about how to make things work from a technical
> > perspective, we should consider what the use case happens to be, and
> > what are the security requirements.  *Why* are we trying to use the
> > combination of overlayfs and fscrypt, and what are the security
> > properties we are trying to provide to someone who is relying on this
> > combination?
> 
> I can give one: encrypted containers:
> 
> https://github.com/opencontainers/image-spec/issues/747
> 
> The current proposal imagines that the key would be delivered to the
> physical node and the physical node containerd would decrypt all the
> layers before handing them off to to the kubelet.  However, one could
> imagine a slightly more secure use case where the layers were
> constructed as an encrypted filesystem tar and so the key would go into
> the kernel and the layers would be constructed with encryption in place
> using fscrypt.
> 
> Most of the desired security properties are in image at rest but one
> can imagine that the running image wants some protection against
> containment breaches by other tenants and using fscrypt could provide
> that.

What kind of containment breaches?  If they can break root, it's all
over no matter what sort of encryption you are using.  If they can't
break root, then the OS's user-id based access control checks (or
SELinux checks if you are using SELinux) will still protect you.

	       	      	  		      - Ted

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 12:44 -0400, Theodore Ts'o wrote:
> On Wed, Mar 13, 2019 at 08:36:34AM -0700, James Bottomley wrote:
> > On Wed, 2019-03-13 at 11:16 -0400, Theodore Ts'o wrote:
> > > So before we talk about how to make things work from a technical
> > > perspective, we should consider what the use case happens to be,
> > > and what are the security requirements.  *Why* are we trying to
> > > use the combination of overlayfs and fscrypt, and what are the
> > > security properties we are trying to provide to someone who is
> > > relying on this combination?
> > 
> > I can give one: encrypted containers:
> > 
> > https://github.com/opencontainers/image-spec/issues/747
> > 
> > The current proposal imagines that the key would be delivered to
> > the physical node and the physical node containerd would decrypt
> > all the layers before handing them off to to the kubelet.  However,
> > one could imagine a slightly more secure use case where the layers
> > were constructed as an encrypted filesystem tar and so the key
> > would go into the kernel and the layers would be constructed with
> > encryption in place using fscrypt.
> > 
> > Most of the desired security properties are in image at rest but
> > one can imagine that the running image wants some protection
> > against containment breaches by other tenants and using fscrypt
> > could provide that.
> 
> What kind of containment breaches?  If they can break root, it's all
> over no matter what sort of encryption you are using.

With me it's always unprivileged containers inside a user_ns, so
containment breach means non-root.  I hope eventually this will be the
norm for the container industry as well.

>   If they can't break root, then the OS's user-id based access
> control checks (or SELinux checks if you are using SELinux) will
> still protect you.

Well, that's what one would think about the recent runc exploit as
well.  The thing I was looking to do was reduce the chances that
unencrypted data would be lying around to be discovered.  I suppose the
potentially biggest problem is leaking the image after it's decrypted
by admin means like a badly configured backup, but unencryped data is
potentially discoverable by breakouts as well.

James

Re: overlayfs vs. fscrypt

[Theodore Ts'o]

On Wed, Mar 13, 2019 at 10:45:04AM -0700, James Bottomley wrote:
> >   If they can't break root, then the OS's user-id based access
> > control checks (or SELinux checks if you are using SELinux) will
> > still protect you.
> 
> Well, that's what one would think about the recent runc exploit as
> well.  The thing I was looking to do was reduce the chances that
> unencrypted data would be lying around to be discovered.  I suppose the
> potentially biggest problem is leaking the image after it's decrypted
> by admin means like a badly configured backup, but unencryped data is
> potentially discoverable by breakouts as well.

But while the container is running, the key is available and
instantiated in the kernel, and the kernel is free to decrypt any
encrypted file/block.  The reason why the kernel won't do this is
because of its access control checks.

And we're talking about this within the context of the overlayfs.
When in the container world will we have persistent data that lasts
beyond the lifetime of the running container that will be using
overlayfs?  I didn't think that existed; if you are using, say, a
Docker storage volume, does overlayfs ever get into the act?  And if
so, how, and what are the desired security properties?

    	     	      	  	  - Ted

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 14:58 -0400, Theodore Ts'o wrote:
> On Wed, Mar 13, 2019 at 10:45:04AM -0700, James Bottomley wrote:
> > >   If they can't break root, then the OS's user-id based access
> > > control checks (or SELinux checks if you are using SELinux) will
> > > still protect you.
> > 
> > Well, that's what one would think about the recent runc exploit as
> > well.  The thing I was looking to do was reduce the chances that
> > unencrypted data would be lying around to be discovered.  I suppose
> > the potentially biggest problem is leaking the image after it's
> > decrypted by admin means like a badly configured backup, but
> > unencryped data is potentially discoverable by breakouts as well.
> 
> But while the container is running, the key is available and
> instantiated in the kernel, and the kernel is free to decrypt any
> encrypted file/block.

In the current encrypted tar file implementation, while the container
is running the decrypted tar file is extracted into the container root
and available for all to see.

The main security benefit of this implementation, as I said, is
security of at rest images and the runtime security is guaranteed by
other systems.

>   The reason why the kernel won't do this is because of its access
> control checks.
> 
> And we're talking about this within the context of the overlayfs.
> When in the container world will we have persistent data that lasts
> beyond the lifetime of the running container that will be using
> overlayfs?  I didn't think that existed; if you are using, say, a
> Docker storage volume, does overlayfs ever get into the act?  And if
> so, how, and what are the desired security properties?

Are you asking about persistent volumes?  I can answer, but that's not
the current use case.  The current use case is encrypted images, which
are overlays.  If you mean the misconfigured backup comment then I was
thinking a backup that wrongly sweeps container root while the
container is running.

Lets go back to basics: can fscrypt provide equivalent or better
protection than the current encrypted tarfile approach?  If the answer
is no because it's too tightly tied to the android use case then
perhaps there's not much point discussing it further.

James

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 12:17:52PM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 14:58 -0400, Theodore Ts'o wrote:
> > On Wed, Mar 13, 2019 at 10:45:04AM -0700, James Bottomley wrote:
> > > >   If they can't break root, then the OS's user-id based access
> > > > control checks (or SELinux checks if you are using SELinux) will
> > > > still protect you.
> > > 
> > > Well, that's what one would think about the recent runc exploit as
> > > well.  The thing I was looking to do was reduce the chances that
> > > unencrypted data would be lying around to be discovered.  I suppose
> > > the potentially biggest problem is leaking the image after it's
> > > decrypted by admin means like a badly configured backup, but
> > > unencryped data is potentially discoverable by breakouts as well.
> > 
> > But while the container is running, the key is available and
> > instantiated in the kernel, and the kernel is free to decrypt any
> > encrypted file/block.
> 
> In the current encrypted tar file implementation, while the container
> is running the decrypted tar file is extracted into the container root
> and available for all to see.
> 
> The main security benefit of this implementation, as I said, is
> security of at rest images and the runtime security is guaranteed by
> other systems.

That's not security at rest, because you're decrypting the data and storing it
onto the local disk.

fscrypt would allow the data to be stored encrypted on the local disk, so it's
protected against offline compromise of the disk.

It would not prevent an attacker who has escalated to root or kernel privileges
from reading the data while the container is running, because that would be
impossible.

It would also not prevent non-root users from reading the data, because the
kernel already has a huge variety of access control mechanisms that can do this
and can be used alongside fscrypt.

> 
> >   The reason why the kernel won't do this is because of its access
> > control checks.
> > 
> > And we're talking about this within the context of the overlayfs.
> > When in the container world will we have persistent data that lasts
> > beyond the lifetime of the running container that will be using
> > overlayfs?  I didn't think that existed; if you are using, say, a
> > Docker storage volume, does overlayfs ever get into the act?  And if
> > so, how, and what are the desired security properties?
> 
> Are you asking about persistent volumes?  I can answer, but that's not
> the current use case.  The current use case is encrypted images, which
> are overlays.  If you mean the misconfigured backup comment then I was
> thinking a backup that wrongly sweeps container root while the
> container is running.
> 
> Lets go back to basics: can fscrypt provide equivalent or better
> protection than the current encrypted tarfile approach?  If the answer
> is no because it's too tightly tied to the android use case then
> perhaps there's not much point discussing it further.
> 

It's not tied to the Android use case.  As I mentioned, fscrypt has many other
users, and it wasn't even originally designed for Android.

- Eric

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
> On Wed, Mar 13, 2019 at 12:17:52PM -0700, James Bottomley wrote:
> > On Wed, 2019-03-13 at 14:58 -0400, Theodore Ts'o wrote:
> > > On Wed, Mar 13, 2019 at 10:45:04AM -0700, James Bottomley wrote:
> > > > >   If they can't break root, then the OS's user-id based
> > > > > access control checks (or SELinux checks if you are using
> > > > > SELinux) will still protect you.
> > > > 
> > > > Well, that's what one would think about the recent runc exploit
> > > > as well.  The thing I was looking to do was reduce the chances
> > > > that unencrypted data would be lying around to be
> > > > discovered.  I suppose the potentially biggest problem is
> > > > leaking the image after it's decrypted by admin means like a
> > > > badly configured backup, but unencryped data is potentially
> > > > discoverable by breakouts as well.
> > > 
> > > But while the container is running, the key is available and
> > > instantiated in the kernel, and the kernel is free to decrypt any
> > > encrypted file/block.
> > 
> > In the current encrypted tar file implementation, while the
> > container is running the decrypted tar file is extracted into the
> > container root and available for all to see.
> > 
> > The main security benefit of this implementation, as I said, is
> > security of at rest images and the runtime security is guaranteed
> > by other systems.
> 
> That's not security at rest, because you're decrypting the data and
> storing it onto the local disk.

I mean image at rest and image running.  The local disk untar only
happens for running image.

> fscrypt would allow the data to be stored encrypted on the local
> disk, so it's protected against offline compromise of the disk.

Container images are essentially tars of the overlays.  They only
become actual filesystems when instantiated at runtime.  The current
encrypted container image is an overlay or set of overlays which is
tarred then encrypted.  So to instantiate it is decrypted then
untarred.

The thing I was wondering about was whether instead of a tar encrypt we
could instead produce an encrypted image from a fscrypt filesystem.

James


> It would not prevent an attacker who has escalated to root or kernel
> privileges from reading the data while the container is running,
> because that would be impossible.
> 
> It would also not prevent non-root users from reading the data,
> because the kernel already has a huge variety of access control
> mechanisms that can do this and can be used alongside fscrypt.
> 
> > 
> > >   The reason why the kernel won't do this is because of its
> > > access control checks.
> > > 
> > > And we're talking about this within the context of the overlayfs.
> > > When in the container world will we have persistent data that
> > > lasts beyond the lifetime of the running container that will be
> > > using overlayfs?  I didn't think that existed; if you are using,
> > > say, a Docker storage volume, does overlayfs ever get into the
> > > act?  And if so, how, and what are the desired security
> > > properties?
> > 
> > Are you asking about persistent volumes?  I can answer, but that's
> > not the current use case.  The current use case is encrypted
> > images, which are overlays.  If you mean the misconfigured backup
> > comment then I was thinking a backup that wrongly sweeps container
> > root while the container is running.
> > 
> > Lets go back to basics: can fscrypt provide equivalent or better
> > protection than the current encrypted tarfile approach?  If the
> > answer is no because it's too tightly tied to the android use case
> > then perhaps there's not much point discussing it further.
> > 
> 
> It's not tied to the Android use case.  As I mentioned, fscrypt has
> many other users, and it wasn't even originally designed for Android.
> 
> - Eric
> 

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 01:06:06PM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
> > On Wed, Mar 13, 2019 at 12:17:52PM -0700, James Bottomley wrote:
> > > On Wed, 2019-03-13 at 14:58 -0400, Theodore Ts'o wrote:
> > > > On Wed, Mar 13, 2019 at 10:45:04AM -0700, James Bottomley wrote:
> > > > > >   If they can't break root, then the OS's user-id based
> > > > > > access control checks (or SELinux checks if you are using
> > > > > > SELinux) will still protect you.
> > > > > 
> > > > > Well, that's what one would think about the recent runc exploit
> > > > > as well.  The thing I was looking to do was reduce the chances
> > > > > that unencrypted data would be lying around to be
> > > > > discovered.  I suppose the potentially biggest problem is
> > > > > leaking the image after it's decrypted by admin means like a
> > > > > badly configured backup, but unencryped data is potentially
> > > > > discoverable by breakouts as well.
> > > > 
> > > > But while the container is running, the key is available and
> > > > instantiated in the kernel, and the kernel is free to decrypt any
> > > > encrypted file/block.
> > > 
> > > In the current encrypted tar file implementation, while the
> > > container is running the decrypted tar file is extracted into the
> > > container root and available for all to see.
> > > 
> > > The main security benefit of this implementation, as I said, is
> > > security of at rest images and the runtime security is guaranteed
> > > by other systems.
> > 
> > That's not security at rest, because you're decrypting the data and
> > storing it onto the local disk.
> 
> I mean image at rest and image running.  The local disk untar only
> happens for running image.
> 
> > fscrypt would allow the data to be stored encrypted on the local
> > disk, so it's protected against offline compromise of the disk.
> 
> Container images are essentially tars of the overlays.  They only
> become actual filesystems when instantiated at runtime.  The current
> encrypted container image is an overlay or set of overlays which is
> tarred then encrypted.  So to instantiate it is decrypted then
> untarred.
> 
> The thing I was wondering about was whether instead of a tar encrypt we
> could instead produce an encrypted image from a fscrypt filesystem.
> 

Why do you care whether the container image is encrypted on the local disk, when
you're extracting it in plaintext onto the local disk anyway each time it runs?
Even after the runtime files are "deleted", they may still be recoverable from
the disk.  Are you using shred and BLKSECDISCARD, and a non-COW filesystem?

Now, if you wanted to avoid writing the plaintext to disk entirely (and thereby
use encryption to actually achieve a useful security property that can't be
achieved through file permissions), fscrypt is a good solution for that.

- Eric

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 13:25 -0700, Eric Biggers wrote:
> On Wed, Mar 13, 2019 at 01:06:06PM -0700, James Bottomley wrote:
> > On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
[...]
> > > fscrypt would allow the data to be stored encrypted on the local
> > > disk, so it's protected against offline compromise of the disk.
> > 
> > Container images are essentially tars of the overlays.  They only
> > become actual filesystems when instantiated at runtime.  The
> > current encrypted container image is an overlay or set of overlays
> > which is tarred then encrypted.  So to instantiate it is decrypted
> > then untarred.
> > 
> > The thing I was wondering about was whether instead of a tar
> > encrypt we could instead produce an encrypted image from a fscrypt
> > filesystem.
> > 
> 
> Why do you care whether the container image is encrypted on the local
> disk, when you're extracting it in plaintext onto the local disk
> anyway each time it runs? Even after the runtime files are "deleted",
> they may still be recoverable from the disk.  Are you using shred and
> BLKSECDISCARD, and a non-COW filesystem?
> 
> Now, if you wanted to avoid writing the plaintext to disk entirely
> (and thereby use encryption to actually achieve a useful security
> property that can't be achieved through file permissions), fscrypt is
> a good solution for that.

OK let's start with a cloud and container 101: A container is an
exactly transportable IaaS environment containing an application.  The
format for the exact transport is the "container image" I've been
describing (layered tar file set deployed with overlays).  These images
are usually stored in cloud based registries which may or may not have
useful access controls.  I take it the reason for image encryption to
protect confidentiality within the registry is obvious.

Because of the exact transport, the deployment may be on my laptop, on
my test system or in some type of public or private cloud.  In all
cases bar the laptop, I won't actually own the physical system which
ends up deploying the container.  So in exchange for security
guarantees from the physical system owner, I agree to turn over my
decryption key and possibly a cash payment.  One of these guarantees is
usually that they shred the key after use and that they deploy a useful
key escrow system like vault or keyprotect to guard it even while the
decryption is being done.  Another is that all traces of the container
be shredded after the execution is finished.  The scenarios I'm
considering is could I be protected against either cloud provider
cockups that might leak the image (the misconfigured backup scenario I
suggested) or malicious actions of other tenants.

James

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 02:04:29PM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 13:25 -0700, Eric Biggers wrote:
> > On Wed, Mar 13, 2019 at 01:06:06PM -0700, James Bottomley wrote:
> > > On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
> [...]
> > > > fscrypt would allow the data to be stored encrypted on the local
> > > > disk, so it's protected against offline compromise of the disk.
> > > 
> > > Container images are essentially tars of the overlays.  They only
> > > become actual filesystems when instantiated at runtime.  The
> > > current encrypted container image is an overlay or set of overlays
> > > which is tarred then encrypted.  So to instantiate it is decrypted
> > > then untarred.
> > > 
> > > The thing I was wondering about was whether instead of a tar
> > > encrypt we could instead produce an encrypted image from a fscrypt
> > > filesystem.
> > > 
> > 
> > Why do you care whether the container image is encrypted on the local
> > disk, when you're extracting it in plaintext onto the local disk
> > anyway each time it runs? Even after the runtime files are "deleted",
> > they may still be recoverable from the disk.  Are you using shred and
> > BLKSECDISCARD, and a non-COW filesystem?
> > 
> > Now, if you wanted to avoid writing the plaintext to disk entirely
> > (and thereby use encryption to actually achieve a useful security
> > property that can't be achieved through file permissions), fscrypt is
> > a good solution for that.
> 
> OK let's start with a cloud and container 101: A container is an
> exactly transportable IaaS environment containing an application.  The
> format for the exact transport is the "container image" I've been
> describing (layered tar file set deployed with overlays).  These images
> are usually stored in cloud based registries which may or may not have
> useful access controls.  I take it the reason for image encryption to
> protect confidentiality within the registry is obvious.
> 
> Because of the exact transport, the deployment may be on my laptop, on
> my test system or in some type of public or private cloud.  In all
> cases bar the laptop, I won't actually own the physical system which
> ends up deploying the container.  So in exchange for security
> guarantees from the physical system owner, I agree to turn over my
> decryption key and possibly a cash payment.  One of these guarantees is
> usually that they shred the key after use and that they deploy a useful
> key escrow system like vault or keyprotect to guard it even while the
> decryption is being done.


> Another is that all traces of the container be shredded after the execution is
> finished.

Well, sounds like that's not the case currently even with an encrypted container
image, because the actual runtime files are not encrypted on disk.  Encrypting
the runtime files using fscrypt with an ephemeral key would be useful here.
IOW, randomly generate an encryption key when the container starts, never store
it anywhere, and wipe it when the container stops.

Note that this is separate from the container *image* encryption.

> considering is could I be protected against either cloud provider
> cockups that might leak the image (the misconfigured backup scenario I
> suggested) or malicious actions of other tenants.

If the container image is encrypted with a key not on the system, then its
confidentiality is protected from anything that may happen on that system.

But if the container image encryption key *is* on the system, your container
image may be leaked either accidentally or maliciously.

- Eric

Re: overlayfs vs. fscrypt

[James Bottomley]

On Wed, 2019-03-13 at 15:13 -0700, Eric Biggers wrote:
> On Wed, Mar 13, 2019 at 02:04:29PM -0700, James Bottomley wrote:
> > On Wed, 2019-03-13 at 13:25 -0700, Eric Biggers wrote:
> > > On Wed, Mar 13, 2019 at 01:06:06PM -0700, James Bottomley wrote:
> > > > On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
> > 
> > [...]
> > > > > fscrypt would allow the data to be stored encrypted on the
> > > > > local disk, so it's protected against offline compromise of
> > > > > the disk.
> > > > 
> > > > Container images are essentially tars of the overlays.  They
> > > > only become actual filesystems when instantiated at
> > > > runtime.  The current encrypted container image is an overlay
> > > > or set of overlays which is tarred then encrypted.  So to
> > > > instantiate it is decrypted then untarred.
> > > > 
> > > > The thing I was wondering about was whether instead of a tar
> > > > encrypt we could instead produce an encrypted image from a
> > > > fscrypt filesystem.
> > > > 
> > > 
> > > Why do you care whether the container image is encrypted on the
> > > local disk, when you're extracting it in plaintext onto the local
> > > disk anyway each time it runs? Even after the runtime files are
> > > "deleted", they may still be recoverable from the disk.  Are you
> > > using shred and BLKSECDISCARD, and a non-COW filesystem?
> > > 
> > > Now, if you wanted to avoid writing the plaintext to disk
> > > entirely (and thereby use encryption to actually achieve a useful
> > > security property that can't be achieved through file
> > > permissions), fscrypt is a good solution for that.
> > 
> > OK let's start with a cloud and container 101: A container is an
> > exactly transportable IaaS environment containing an
> > application.  The format for the exact transport is the "container
> > image" I've been describing (layered tar file set deployed with
> > overlays).  These images are usually stored in cloud based
> > registries which may or may not have useful access controls.  I
> > take it the reason for image encryption to protect confidentiality
> > within the registry is obvious.
> > 
> > Because of the exact transport, the deployment may be on my laptop,
> > on my test system or in some type of public or private cloud.  In
> > all cases bar the laptop, I won't actually own the physical system
> > which ends up deploying the container.  So in exchange for security
> > guarantees from the physical system owner, I agree to turn over my
> > decryption key and possibly a cash payment.  One of these
> > guarantees is usually that they shred the key after use and that
> > they deploy a useful key escrow system like vault or keyprotect to
> > guard it even while the decryption is being done.
> 
> 
> > Another is that all traces of the container be shredded after the
> > execution is finished.
> 
> Well, sounds like that's not the case currently even with an
> encrypted container image, because the actual runtime files are not
> encrypted on disk.

Shredding means destroying all trace including in the on-disk image. 
However, one problem with the current implementation is there's a
window between container run and container stop where the unencrypted
files are in memory and on local disk.  Access or cockup in that window
can leak confidential data.

>   Encrypting the runtime files using fscrypt with an ephemeral key
> would be useful here.  IOW, randomly generate an encryption key when
> the container starts, never store it anywhere, and wipe it when the
> container stops.
> 
> Note that this is separate from the container *image* encryption.

Actually, that was my original thought: it needn't be.  If fscrypt can
usefully add runtime security, then we could have the encrypted layer
be simply an fscrypt image ... I presume without the key we can create
a tar image of an fscrypt that is encrypted and would still be visible
on untar if we did have the key?  So the encrypted layer would be a tar
of the fscrypt filesystem without the key.

> > considering is could I be protected against either cloud provider
> > cockups that might leak the image (the misconfigured backup
> > scenario I suggested) or malicious actions of other tenants.
> 
> If the container image is encrypted with a key not on the system,
> then its confidentiality is protected from anything that may happen
> on that system.
> 
> But if the container image encryption key *is* on the system, your
> container image may be leaked either accidentally or maliciously.

Well, yes, but that's like saying if you don't want to pick up a virus
from your network unplug it.  We have to look at ways of deploying the
filesystem and the key such that it's hard to exfiltrate ... which
seems to be similar to your android fscrypt use case.

James

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 03:29:43PM -0700, James Bottomley wrote:
> On Wed, 2019-03-13 at 15:13 -0700, Eric Biggers wrote:
> > On Wed, Mar 13, 2019 at 02:04:29PM -0700, James Bottomley wrote:
> > > On Wed, 2019-03-13 at 13:25 -0700, Eric Biggers wrote:
> > > > On Wed, Mar 13, 2019 at 01:06:06PM -0700, James Bottomley wrote:
> > > > > On Wed, 2019-03-13 at 12:57 -0700, Eric Biggers wrote:
> > > 
> > > [...]
> > > > > > fscrypt would allow the data to be stored encrypted on the
> > > > > > local disk, so it's protected against offline compromise of
> > > > > > the disk.
> > > > > 
> > > > > Container images are essentially tars of the overlays.  They
> > > > > only become actual filesystems when instantiated at
> > > > > runtime.  The current encrypted container image is an overlay
> > > > > or set of overlays which is tarred then encrypted.  So to
> > > > > instantiate it is decrypted then untarred.
> > > > > 
> > > > > The thing I was wondering about was whether instead of a tar
> > > > > encrypt we could instead produce an encrypted image from a
> > > > > fscrypt filesystem.
> > > > > 
> > > > 
> > > > Why do you care whether the container image is encrypted on the
> > > > local disk, when you're extracting it in plaintext onto the local
> > > > disk anyway each time it runs? Even after the runtime files are
> > > > "deleted", they may still be recoverable from the disk.  Are you
> > > > using shred and BLKSECDISCARD, and a non-COW filesystem?
> > > > 
> > > > Now, if you wanted to avoid writing the plaintext to disk
> > > > entirely (and thereby use encryption to actually achieve a useful
> > > > security property that can't be achieved through file
> > > > permissions), fscrypt is a good solution for that.
> > > 
> > > OK let's start with a cloud and container 101: A container is an
> > > exactly transportable IaaS environment containing an
> > > application.  The format for the exact transport is the "container
> > > image" I've been describing (layered tar file set deployed with
> > > overlays).  These images are usually stored in cloud based
> > > registries which may or may not have useful access controls.  I
> > > take it the reason for image encryption to protect confidentiality
> > > within the registry is obvious.
> > > 
> > > Because of the exact transport, the deployment may be on my laptop,
> > > on my test system or in some type of public or private cloud.  In
> > > all cases bar the laptop, I won't actually own the physical system
> > > which ends up deploying the container.  So in exchange for security
> > > guarantees from the physical system owner, I agree to turn over my
> > > decryption key and possibly a cash payment.  One of these
> > > guarantees is usually that they shred the key after use and that
> > > they deploy a useful key escrow system like vault or keyprotect to
> > > guard it even while the decryption is being done.
> > 
> > 
> > > Another is that all traces of the container be shredded after the
> > > execution is finished.
> > 
> > Well, sounds like that's not the case currently even with an
> > encrypted container image, because the actual runtime files are not
> > encrypted on disk.
> 
> Shredding means destroying all trace including in the on-disk image. 
> However, one problem with the current implementation is there's a
> window between container run and container stop where the unencrypted
> files are in memory and on local disk.  Access or cockup in that window
> can leak confidential data.

Well, another problem is that it almost certainly doesn't actually work, because
some plaintext will still be recoverable from disk or the raw flash.  Actually
erasing data on modern filesystems and storage devices is extremely difficult.
To start, you'd have to run BLKSECDISCARD on every single block ever written to
disk to prepare the container, or written by the container during its execution.

That's one reason to use storage encryption: it reduces the secure deletion
problem to just erasing the encryption key.

> 
> >   Encrypting the runtime files using fscrypt with an ephemeral key
> > would be useful here.  IOW, randomly generate an encryption key when
> > the container starts, never store it anywhere, and wipe it when the
> > container stops.
> > 
> > Note that this is separate from the container *image* encryption.
> 
> Actually, that was my original thought: it needn't be.  If fscrypt can
> usefully add runtime security, then we could have the encrypted layer
> be simply an fscrypt image ... I presume without the key we can create
> a tar image of an fscrypt that is encrypted and would still be visible
> on untar if we did have the key?  So the encrypted layer would be a tar
> of the fscrypt filesystem without the key.
> 

fscrypt doesn't support backup and restore without the key, so no you can't do
that.  But there wouldn't be much benefit for doing it that way anyway, since
you'd still have to untar the container image each time the container is
started, then re-tar it each time the container is stopped.

The container image could be an ext4 filesystem image containing an encrypted
directory, in which case the container could be run directly from the image.
But in that case, dm-crypt would be a better choice.

> > > considering is could I be protected against either cloud provider
> > > cockups that might leak the image (the misconfigured backup
> > > scenario I suggested) or malicious actions of other tenants.
> > 
> > If the container image is encrypted with a key not on the system,
> > then its confidentiality is protected from anything that may happen
> > on that system.
> > 
> > But if the container image encryption key *is* on the system, your
> > container image may be leaked either accidentally or maliciously.
> 
> Well, yes, but that's like saying if you don't want to pick up a virus
> from your network unplug it.  We have to look at ways of deploying the
> filesystem and the key such that it's hard to exfiltrate ... which
> seems to be similar to your android fscrypt use case.
> 

I am just stating the facts.  Storage encryption only solves a specific set of
problems, not every problem.  There has already been a huge amount of effort
into developing OS-level access control mechanisms in Linux, so fscrypt does not
duplicate that; it just does storage encryption.

- Eric

Re: overlayfs vs. fscrypt

[Al Viro]

On Wed, Mar 13, 2019 at 11:16:33AM -0400, Theodore Ts'o wrote:
> Actually, the original use was for ChromeOS, but the primary
> assumption is that keying is per user (or profile), and that users are
> mutually distrustful.  So when Alice logs out of the system, her keys
> will be invalidated and removed from the kernel.  We can (and do) try
> to flush cache entries via "echo 3 > /proc/sys/vm/drop_caches" on
> logout.  However, this does not guarantee that all dcache entries will
> be removed --- a dcache entry can be pinned due to an open file, a
> process's current working directory, a bind mount, etc.
> 
> The other issue is negative dentries; if you try open a file in an
> encrypted file, the file system won't even *know* whether or not a
> file exists, since the directory entries are encrypted; hence, there
> may be some negative dentries that need to be invalidated.
> 
> So a fundamental assumption with fscrypt is that keys will be added
> and removed, and that when this happens, dentries will need to be
> invalidated.  This is going to surprise overlayfs, so if overlayfs is
> going to support fscrypt it *has* to be aware of the fact that this
> can happen.  It's not even clear what the proper security semantics
> should be; *especially* if the upper and lower directories aren't
> similarly protected using the same fscrypt encryption key.  Suppose
> the lower directory is encrypted, and the upper is not.  Now on a copy
> up operation, the previously encrypted file, which might contain
> credit card numbers, medical records, or other things that would cause
> a GDPR regulator to have a freak out attack, would *poof* become
> decrypted.

Just to make sure - you do realize that ban on multiple dentries refering
to the same directory inode is *NOT* conditional upon those dentries being
hashed, right?

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 04:06:16PM +0000, Al Viro wrote:
> On Wed, Mar 13, 2019 at 11:16:33AM -0400, Theodore Ts'o wrote:
> > Actually, the original use was for ChromeOS, but the primary
> > assumption is that keying is per user (or profile), and that users are
> > mutually distrustful.  So when Alice logs out of the system, her keys
> > will be invalidated and removed from the kernel.  We can (and do) try
> > to flush cache entries via "echo 3 > /proc/sys/vm/drop_caches" on
> > logout.  However, this does not guarantee that all dcache entries will
> > be removed --- a dcache entry can be pinned due to an open file, a
> > process's current working directory, a bind mount, etc.
> > 
> > The other issue is negative dentries; if you try open a file in an
> > encrypted file, the file system won't even *know* whether or not a
> > file exists, since the directory entries are encrypted; hence, there
> > may be some negative dentries that need to be invalidated.
> > 
> > So a fundamental assumption with fscrypt is that keys will be added
> > and removed, and that when this happens, dentries will need to be
> > invalidated.  This is going to surprise overlayfs, so if overlayfs is
> > going to support fscrypt it *has* to be aware of the fact that this
> > can happen.  It's not even clear what the proper security semantics
> > should be; *especially* if the upper and lower directories aren't
> > similarly protected using the same fscrypt encryption key.  Suppose
> > the lower directory is encrypted, and the upper is not.  Now on a copy
> > up operation, the previously encrypted file, which might contain
> > credit card numbers, medical records, or other things that would cause
> > a GDPR regulator to have a freak out attack, would *poof* become
> > decrypted.
> 
> Just to make sure - you do realize that ban on multiple dentries refering
> to the same directory inode is *NOT* conditional upon those dentries being
> hashed, right?

Isn't this handled by d_splice_alias() already, by moving the old dentry to the
new name?

- Eric

Re: overlayfs vs. fscrypt

[Al Viro]

On Wed, Mar 13, 2019 at 09:44:33AM -0700, Eric Biggers wrote:

> > Just to make sure - you do realize that ban on multiple dentries refering
> > to the same directory inode is *NOT* conditional upon those dentries being
> > hashed, right?
> 
> Isn't this handled by d_splice_alias() already, by moving the old dentry to the
> new name?

... which means that if somebody without the key chdirs into subdirectory
they only see by encrypted name and waits for proper owner to look it up,
they suddenly see it by _un_encrypted name.  Or does O_PATH open, for
that matter, so exec permissions on that thing are not required.

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 07:19:46PM +0000, Al Viro wrote:
> On Wed, Mar 13, 2019 at 09:44:33AM -0700, Eric Biggers wrote:
> 
> > > Just to make sure - you do realize that ban on multiple dentries refering
> > > to the same directory inode is *NOT* conditional upon those dentries being
> > > hashed, right?
> > 
> > Isn't this handled by d_splice_alias() already, by moving the old dentry to the
> > new name?
> 
> ... which means that if somebody without the key chdirs into subdirectory
> they only see by encrypted name and waits for proper owner to look it up,
> they suddenly see it by _un_encrypted name.  Or does O_PATH open, for
> that matter, so exec permissions on that thing are not required.

Is there a real problem here?  After the key is added, the filenames are
supposed to be shown in plaintext, not ciphertext.  This is intrinsic to the
fact that we don't support both "views" at the same time.  Either the directory
has the key or it does not.

If someone is using ciphertext view (e.g. doing a directory traversal)
concurrently with the key being added, that can certainly break things.  But the
ciphertext view only allows a very restricted set of actions such as deleting
files.  And if such actions are necessary, the system userspace is meant to be
designed in such a way that adding the key can't race with it.

- Eric

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 04:26:54PM +0200, Amir Goldstein wrote:
> On Wed, Mar 13, 2019 at 3:34 PM Richard Weinberger <richard@nod.at> wrote:
> >
> > Am Mittwoch, 13. März 2019, 14:24:47 CET schrieb Miklos Szeredi:
> > > > The use case is that you can delete these files if the DAC/MAC permissions allow it.
> > > > Just like on NTFS. If a user encrypts files, the admin cannot read them but can
> > > > remove them if the user is gone or loses the key.
> > >
> > > There's the underlying filesystem view where admin can delete files,
> > > etc.   And there's the fscrypt layer stacked on top of the underlying
> > > fs, which en/decrypts files *in case the user has the key*.  What if
> > > one user has a key, but the other one doesn't?  Will d_revalidate
> > > constantly switch the set of dentries between the encrypted filenames
> > > and the decrypted ones?  Sounds crazy.  And the fact that NTFS does
> > > this doesn't make it any less crazy...
> >
> > Well, I didn't come up with this feature. :-)
> >
> > If one user has the key and the other not, a classic multi-user
> > system, then you need to make sure that the affected fscrypt instances
> > are not visible by both.
> > For example by using mount namespaces to make sure that user a can only
> > see /home/foo and user b only /home/bar.
> > Or removing the search permission on /home/foo and /home/bar.
> >
> > I know, I know, but that's how it is...
> > Maybe Ted or Eric can give more details on why they chose this approach.
> >
> 
> AFAIK, this feature was born to tailor Android's file based encryption.
> https://source.android.com/security/encryption#file-based
> It is meant to protect data at rest and what happens when user enters
> the screen lock password IIRC, is that some service will get restarted.
> IOW, there should NOT be any processes in Android accessing the
> encrypted user data folders with and without the key simultaneously.

See my response to Miklos.  Even if some processes had the key in their keyring
and some didn't, which isn't the case on Android since on Android the fscrypt
keys are placed in a "global" keyring, there's still only one cached inode per
file/directory/symlink, and it either has the key (->i_crypt_info != NULL) or it
doesn't (i_crypt_info == NULL).  And it can only go from ->i_crypt_info == NULL
to ->i_crypt_info != NULL, not vice versa.

Also to be clear, there are other fscrypt users besides Android.  E.g. Chrome OS
where it replaced eCryptfs for home directory encryption and was actually the
original use case, people using it on "regular" Linux distros like Ubuntu via
the userspace tool https://github.com/google/fscrypt, and Richard using UBIFS
encryption on embedded devices.  It's not just for Android.

> Also, like OpenWRT, in Android the key does not get removed
> (until boot) AFAIK(?).

On Android, the fscrypt keys are removed when you switch users on a multi-user
device, or when you turn off work mode on a device with a work profile.  This is
currently accompanied by a 'sync && echo 3 > /proc/sys/vm/drop_caches', so the
inodes get evicted too and the files revert to their ciphertext "view".  I'd
like to replace this with my proposed new ioctl FS_IOC_REMOVE_ENCRYPTION_KEY,
which avoids the drop_caches hack: https://patchwork.kernel.org/patch/10821455/.

> 
> That dcache behavior remind me of the proposal to make case
> insensitive a per mount option (also for an Android use case).
> Eventually, that was replaced with per directory flag, which plays
> much better with dache.
> 
> IMO, the best thing for UBIFS to do would be to modify fscrypt to support
> opting out of the revalidate behavior, IWO, sanitize your hack to an API.

As noted in my other response, a better solution (if this is really needed at
all) would probably be to move a stripped-down version of fscrypt_d_revalidate()
to the VFS, so fscrypt won't need to use any dentry_operations at all.

> 
> It's good that you are thinking about what will happen with overlayfs
> over ext4/f2fs, but I think that it will be messy if dentry names would be
> changing in underlying fs and the fact the overlayfs accessed the underlying
> dirs with different credentials at times makes this even more messy.
> 
> The way out of this mess IMO would be for ext4/f2fs to also conditionally
> opt-out of d_revalidate behavior at mount time if the fs is expected to be
> used under overlayfs.
> In Android, for example, I think the use case of "admin deleting
> the encrypted directories" is only relevant on "reset to default" and that
> happens in recovery boot that could potentially opt-out of encryption
> altogether (because there is no user to enter the password anyway).
> 
> I could be over simplifying things for the Android use case and my
> information could be severely out dated.
> CC Paul Lawrence to fill in my Android knowledge gaps.
> 
> Thanks,
> Amir.

- Eric

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 15:26:54 CET schrieb Amir Goldstein:
> IMO, the best thing for UBIFS to do would be to modify fscrypt to support
> opting out of the revalidate behavior, IWO, sanitize your hack to an API.

Given the WTF/s rate this thread has, this might me a good option.
Actually people already asked me how to disable this feature because
they saw no use of it.
Being able to delete encrypted files looks good on the feature list but in
reality it has very few users but causes confusion, IMHO.

I propose a new fscrypt_operations flag, FS_CFLG_NO_CRYPT_FNAMES.
If this flag is set, a) fscrypt_setup_filename() will return -EPERM if
no key is found.
And b) __fscrypt_prepare_lookup() will not attach fscrypt_d_ops to the dentry.

Eric, what do you think?

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 09:33:10PM +0100, Richard Weinberger wrote:
> Am Mittwoch, 13. März 2019, 15:26:54 CET schrieb Amir Goldstein:
> > IMO, the best thing for UBIFS to do would be to modify fscrypt to support
> > opting out of the revalidate behavior, IWO, sanitize your hack to an API.
> 
> Given the WTF/s rate this thread has, this might me a good option.
> Actually people already asked me how to disable this feature because
> they saw no use of it.
> Being able to delete encrypted files looks good on the feature list but in
> reality it has very few users but causes confusion, IMHO.
> 
> I propose a new fscrypt_operations flag, FS_CFLG_NO_CRYPT_FNAMES.
> If this flag is set, a) fscrypt_setup_filename() will return -EPERM if
> no key is found.
> And b) __fscrypt_prepare_lookup() will not attach fscrypt_d_ops to the dentry.
> 
> Eric, what do you think?
> 
> Thanks,
> //richard
> 

What specifically is wrong with supporting the ciphertext "view" of encrypted
directories, and why do you want to opt UBIFS out of it specifically but not
ext4 and f2fs?  (The fscrypt_operations are per-filesystem type, not
per-filesystem instance, so I assume that's what you had in mind.)  Note that we
can't unconditionally remove it because people need it to delete files without
the key.  We could add a mount option to disable it, but why exactly?

By the way, I suggest that people read Documentation/filesystems/fscrypt.rst for
more information about what fscrypt is supposed to do, as there seems to be a
lot of misconceptions.

- Eric

Re: overlayfs vs. fscrypt

[Richard Weinberger]

Am Mittwoch, 13. März 2019, 23:26:11 CET schrieb Eric Biggers:
> On Wed, Mar 13, 2019 at 09:33:10PM +0100, Richard Weinberger wrote:
> > Am Mittwoch, 13. März 2019, 15:26:54 CET schrieb Amir Goldstein:
> > > IMO, the best thing for UBIFS to do would be to modify fscrypt to support
> > > opting out of the revalidate behavior, IWO, sanitize your hack to an API.
> > 
> > Given the WTF/s rate this thread has, this might me a good option.
> > Actually people already asked me how to disable this feature because
> > they saw no use of it.
> > Being able to delete encrypted files looks good on the feature list but in
> > reality it has very few users but causes confusion, IMHO.
> > 
> > I propose a new fscrypt_operations flag, FS_CFLG_NO_CRYPT_FNAMES.
> > If this flag is set, a) fscrypt_setup_filename() will return -EPERM if
> > no key is found.
> > And b) __fscrypt_prepare_lookup() will not attach fscrypt_d_ops to the dentry.
> > 
> > Eric, what do you think?
> > 
> > Thanks,
> > //richard
> > 
> 
> What specifically is wrong with supporting the ciphertext "view" of encrypted
> directories, and why do you want to opt UBIFS out of it specifically but not
> ext4 and f2fs?  (The fscrypt_operations are per-filesystem type, not
> per-filesystem instance, so I assume that's what you had in mind.)  Note that we
> can't unconditionally remove it because people need it to delete files without
> the key.  We could add a mount option to disable it, but why exactly?

You are right, fscrypt_operations is the wrong structure.
My plan was having it per filesystem instance. So a mount-option seems like
a good option. Of course for all filesystems that support fscrypt, not just UBIFS.

Over the last year I've converted many emebdded systems to fscrypt and it happened
more than once that users, and more importantly, applications got confused that
you can mount and walk the filesystem even if you don't have the key loaded yet.
For them it felt more natural that you cannot even readdir if you don't have the key.

In my opinion having such a mount option is useful to match these expectations.
And it is also useful because you can enable only the features you actually need.
On embedded systems that I have in mind you never delete files without having the key
and since fscrypt is used for the whole filesystem you can just recreate it if you
really lost the key.

Thanks,
//richard

Re: overlayfs vs. fscrypt

[Miklos Szeredi]

On Wed, Mar 13, 2019 at 11:42 PM Richard Weinberger <richard@nod.at> wrote:
>
> Am Mittwoch, 13. März 2019, 23:26:11 CET schrieb Eric Biggers:

> > What specifically is wrong with supporting the ciphertext "view" of encrypted
> > directories, and why do you want to opt UBIFS out of it specifically but not
> > ext4 and f2fs?  (The fscrypt_operations are per-filesystem type, not
> > per-filesystem instance, so I assume that's what you had in mind.)  Note that we
> > can't unconditionally remove it because people need it to delete files without
> > the key.  We could add a mount option to disable it, but why exactly?
>
> You are right, fscrypt_operations is the wrong structure.
> My plan was having it per filesystem instance. So a mount-option seems like
> a good option. Of course for all filesystems that support fscrypt, not just UBIFS.

Yes, please.   Changing filesystem contents based on a mount option is
orders of magnitude more sane than doing so on key insertion/removal.

Thanks,
Miklos

[RFC] fscrypt_key_required mount option

[Richard Weinberger]

This series is an RFC, it outlines how an filesystem, in this case UBIFS,
could implement a mount option to disable filesystem operations on encrypted
files where no key is present.
The desired byproduct is that in this case ->d_revalidate() is not needed
anymore and VFS folks are in less fear about the dcache implications.

Thanks,
//richard

[PATCH 1/4] fscrypt: Implement FS_CFLG_OWN_D_OPS
[PATCH 2/4] fscrypt: Export fscrypt_d_ops
[PATCH 3/4] ubifs: Simplify fscrypt_get_encryption_info() error
[PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[PATCH 1/4] fscrypt: Implement FS_CFLG_OWN_D_OPS

[Richard Weinberger]

If a filesystem sets FS_CFLG_OWN_D_OPS it manages dentry operations
itself and fscrypt is not allowed to set them.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/crypto/hooks.c       | 4 +++-
 include/linux/fscrypt.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 56debb1fcf5e..3ec925405fbe 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -107,7 +107,9 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry)
 		spin_unlock(&dentry->d_lock);
 	}
 
-	d_set_d_op(dentry, &fscrypt_d_ops);
+	if ((dir->i_sb->s_cop->flags & FS_CFLG_OWN_D_OPS) == 0)
+		d_set_d_op(dentry, &fscrypt_d_ops);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index e5194fc3983e..7139a110ac4f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -48,6 +48,7 @@ struct fscrypt_name {
  * fscrypt superblock flags
  */
 #define FS_CFLG_OWN_PAGES (1U << 1)
+#define FS_CFLG_OWN_D_OPS (1U << 2)
 
 /*
  * crypto operations for filesystems
-- 
2.21.0

[PATCH 2/4] fscrypt: Export fscrypt_d_ops

[Richard Weinberger]

If a filesystem manages dentry operations itself it might want to
re-use fscrypt_d_ops.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/crypto/crypto.c          | 1 +
 fs/crypto/fscrypt_private.h | 1 -
 include/linux/fscrypt.h     | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4dc788e3bc96..8018f8bba50d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -357,6 +357,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 const struct dentry_operations fscrypt_d_ops = {
 	.d_revalidate = fscrypt_d_revalidate,
 };
+EXPORT_SYMBOL(fscrypt_d_ops);
 
 void fscrypt_restore_control_page(struct page *page)
 {
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 7da276159593..bced1ee4fd64 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -125,7 +125,6 @@ extern int fscrypt_do_page_crypto(const struct inode *inode,
 				  gfp_t gfp_flags);
 extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
-extern const struct dentry_operations fscrypt_d_ops;
 
 extern void __printf(3, 4) __cold
 fscrypt_msg(struct super_block *sb, const char *level, const char *fmt, ...);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 7139a110ac4f..2b9577e4707f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -231,6 +231,7 @@ extern int __fscrypt_encrypt_symlink(struct inode *inode, const char *target,
 extern const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 				       unsigned int max_size,
 				       struct delayed_call *done);
+extern const struct dentry_operations fscrypt_d_ops;
 #else  /* !CONFIG_FS_ENCRYPTION */
 
 static inline bool fscrypt_has_encryption_key(const struct inode *inode)
-- 
2.21.0

[PATCH 3/4] ubifs: Simplify fscrypt_get_encryption_info() error handling

[Richard Weinberger]

fscrypt_get_encryption_info() does not return -ENOKEY,
there is no need to handle this case.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/dir.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 5767b373a8ff..b0cb913697c5 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -526,7 +526,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 
 	if (encrypted) {
 		err = fscrypt_get_encryption_info(dir);
-		if (err && err != -ENOKEY)
+		if (err)
 			return err;
 
 		err = fscrypt_fname_alloc_buffer(dir, UBIFS_MAX_NLEN, &fstr);
@@ -794,7 +794,7 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry)
 
 	if (ubifs_crypt_is_encrypted(dir)) {
 		err = fscrypt_get_encryption_info(dir);
-		if (err && err != -ENOKEY)
+		if (err)
 			return err;
 	}
 
@@ -904,7 +904,7 @@ static int ubifs_rmdir(struct inode *dir, struct dentry *dentry)
 
 	if (ubifs_crypt_is_encrypted(dir)) {
 		err = fscrypt_get_encryption_info(dir);
-		if (err && err != -ENOKEY)
+		if (err)
 			return err;
 	}
 
-- 
2.21.0

[PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Richard Weinberger]

Usually fscrypt allows limited access to encrypted files even
if no key is available.
Encrypted filenames are shown and based on this names users
can unlink and move files.

This is not always what people expect. The fscrypt_key_required mount
option disables this feature.
If no key is present all access is denied with the -ENOKEY error code.

The side benefit of this is that we don't need ->d_revalidate().
Not having ->d_revalidate() makes an encrypted ubifs usable
as overlayfs upper directory.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/crypto.c |  2 +-
 fs/ubifs/dir.c    | 29 ++++++++++++++++++++++++++---
 fs/ubifs/super.c  | 15 +++++++++++++++
 fs/ubifs/ubifs.h  |  1 +
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index 4aaedf2d7f44..a6a48c5dc058 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -76,7 +76,7 @@ int ubifs_decrypt(const struct inode *inode, struct ubifs_data_node *dn,
 }
 
 const struct fscrypt_operations ubifs_crypt_operations = {
-	.flags			= FS_CFLG_OWN_PAGES,
+	.flags			= FS_CFLG_OWN_PAGES | FS_CFLG_OWN_D_OPS,
 	.key_prefix		= "ubifs:",
 	.get_context		= ubifs_crypt_get_context,
 	.set_context		= ubifs_crypt_set_context,
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index b0cb913697c5..4d029f08b80d 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -208,6 +208,16 @@ static int dbg_check_name(const struct ubifs_info *c,
 	return 0;
 }
 
+static void ubifs_set_dentry_ops(struct inode *dir, struct dentry *dentry)
+{
+#ifdef CONFIG_FS_ENCRYPTION
+	struct ubifs_info *c = dir->i_sb->s_fs_info;
+
+	if (IS_ENCRYPTED(dir) && !c->fscrypt_key_required)
+		d_set_d_op(dentry, &fscrypt_d_ops);
+#endif
+}
+
 static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
 				   unsigned int flags)
 {
@@ -224,7 +234,10 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
 	if (err)
 		return ERR_PTR(err);
 
-	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
+	ubifs_set_dentry_ops(dir, dentry);
+
+	err = fscrypt_setup_filename(dir, &dentry->d_name,
+				     !c->fscrypt_key_required, &nm);
 	if (err)
 		return ERR_PTR(err);
 
@@ -240,6 +253,11 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
 	}
 
 	if (nm.hash) {
+		if (c->fscrypt_key_required) {
+			inode = ERR_PTR(-ENOKEY);
+			goto done;
+		}
+
 		ubifs_assert(c, fname_len(&nm) == 0);
 		ubifs_assert(c, fname_name(&nm) == NULL);
 		dent_key_init_hash(c, &key, dir->i_ino, nm.hash);
@@ -529,6 +547,9 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		if (err)
 			return err;
 
+		if (c->fscrypt_key_required && !dir->i_crypt_info)
+			return -ENOKEY;
+
 		err = fscrypt_fname_alloc_buffer(dir, UBIFS_MAX_NLEN, &fstr);
 		if (err)
 			return err;
@@ -798,7 +819,8 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry)
 			return err;
 	}
 
-	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
+	err = fscrypt_setup_filename(dir, &dentry->d_name, !c->fscrypt_key_required,
+				     &nm);
 	if (err)
 		return err;
 
@@ -908,7 +930,8 @@ static int ubifs_rmdir(struct inode *dir, struct dentry *dentry)
 			return err;
 	}
 
-	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
+	err = fscrypt_setup_filename(dir, &dentry->d_name,
+				     !c->fscrypt_key_required, &nm);
 	if (err)
 		return err;
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 8dc2818fdd84..e081b00236b6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -445,6 +445,9 @@ static int ubifs_show_options(struct seq_file *s, struct dentry *root)
 			   ubifs_compr_name(c, c->mount_opts.compr_type));
 	}
 
+	if (c->fscrypt_key_required)
+		seq_puts(s, ",fscrypt_key_required");
+
 	seq_printf(s, ",assert=%s", ubifs_assert_action_name(c));
 	seq_printf(s, ",ubi=%d,vol=%d", c->vi.ubi_num, c->vi.vol_id);
 
@@ -952,6 +955,7 @@ enum {
 	Opt_assert,
 	Opt_auth_key,
 	Opt_auth_hash_name,
+	Opt_fscrypt_key_required,
 	Opt_ignore,
 	Opt_err,
 };
@@ -969,6 +973,7 @@ static const match_table_t tokens = {
 	{Opt_ignore, "ubi=%s"},
 	{Opt_ignore, "vol=%s"},
 	{Opt_assert, "assert=%s"},
+	{Opt_fscrypt_key_required, "fscrypt_key_required"},
 	{Opt_err, NULL},
 };
 
@@ -1008,6 +1013,7 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 {
 	char *p;
 	substring_t args[MAX_OPT_ARGS];
+	unsigned int old_fscrypt_key_required = c->fscrypt_key_required;
 
 	if (!options)
 		return 0;
@@ -1099,6 +1105,15 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 			if (!c->auth_hash_name)
 				return -ENOMEM;
 			break;
+		case Opt_fscrypt_key_required:
+			c->fscrypt_key_required = 1;
+
+			if (is_remount && (old_fscrypt_key_required != c->fscrypt_key_required)) {
+				ubifs_err(c, "fscrypt_key_required cannot changed by remount");
+				return -EINVAL;
+			}
+
+			break;
 		case Opt_ignore:
 			break;
 		default:
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 1ae12900e01d..71b03a6798ae 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1304,6 +1304,7 @@ struct ubifs_info {
 	unsigned int rw_incompat:1;
 	unsigned int assert_action:2;
 	unsigned int authenticated:1;
+	unsigned int fscrypt_key_required:1;
 
 	struct mutex tnc_mutex;
 	struct ubifs_zbranch zroot;
-- 
2.21.0

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Eric Biggers]

Hi Richard,

On Thu, Mar 14, 2019 at 06:15:59PM +0100, Richard Weinberger wrote:
> Usually fscrypt allows limited access to encrypted files even
> if no key is available.
> Encrypted filenames are shown and based on this names users
> can unlink and move files.

Actually, fscrypt doesn't allow moving files without the key.  It would only be
possible for cross-renames, i.e. renames with the RENAME_EXCHANGE flag.  So for
consistency with regular renames, fscrypt also forbids cross-renames if the key
for either the source or destination directory is missing.

So the main use case for the ciphertext view is *deleting* files.  For example,
deleting a user's home directory after that user has been removed from the
system.  Or the system freeing up space by deleting cache files from a user who
isn't currently logged in.

> 
> This is not always what people expect. The fscrypt_key_required mount
> option disables this feature.
> If no key is present all access is denied with the -ENOKEY error code.

The problem with this mount option is that it allows users to create undeletable
files.  So I'm not really convinced yet this is a good change.  And though the
fscrypt_key_required semantics are easier to implement, we'd still have to
support the existing semantics too, thus increasing the maintenance cost.

> 
> The side benefit of this is that we don't need ->d_revalidate().
> Not having ->d_revalidate() makes an encrypted ubifs usable
> as overlayfs upper directory.
> 

It would be preferable if we could get overlayfs to work without providing a
special mount option.

> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/ubifs/crypto.c |  2 +-
>  fs/ubifs/dir.c    | 29 ++++++++++++++++++++++++++---
>  fs/ubifs/super.c  | 15 +++++++++++++++
>  fs/ubifs/ubifs.h  |  1 +
>  4 files changed, 43 insertions(+), 4 deletions(-)
> 

Shouldn't readlink() honor the mount option too?

> diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
> index 4aaedf2d7f44..a6a48c5dc058 100644
> --- a/fs/ubifs/crypto.c
> +++ b/fs/ubifs/crypto.c
> @@ -76,7 +76,7 @@ int ubifs_decrypt(const struct inode *inode, struct ubifs_data_node *dn,
>  }
>  
>  const struct fscrypt_operations ubifs_crypt_operations = {
> -	.flags			= FS_CFLG_OWN_PAGES,
> +	.flags			= FS_CFLG_OWN_PAGES | FS_CFLG_OWN_D_OPS,
>  	.key_prefix		= "ubifs:",
>  	.get_context		= ubifs_crypt_get_context,
>  	.set_context		= ubifs_crypt_set_context,
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index b0cb913697c5..4d029f08b80d 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -208,6 +208,16 @@ static int dbg_check_name(const struct ubifs_info *c,
>  	return 0;
>  }
>  
> +static void ubifs_set_dentry_ops(struct inode *dir, struct dentry *dentry)
> +{
> +#ifdef CONFIG_FS_ENCRYPTION
> +	struct ubifs_info *c = dir->i_sb->s_fs_info;
> +
> +	if (IS_ENCRYPTED(dir) && !c->fscrypt_key_required)
> +		d_set_d_op(dentry, &fscrypt_d_ops);
> +#endif
> +}
> +
>  static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
>  				   unsigned int flags)
>  {
> @@ -224,7 +234,10 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
>  	if (err)
>  		return ERR_PTR(err);
>  
> -	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
> +	ubifs_set_dentry_ops(dir, dentry);
> +
> +	err = fscrypt_setup_filename(dir, &dentry->d_name,
> +				     !c->fscrypt_key_required, &nm);
>  	if (err)
>  		return ERR_PTR(err);
>  
> @@ -240,6 +253,11 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
>  	}
>  
>  	if (nm.hash) {
> +		if (c->fscrypt_key_required) {
> +			inode = ERR_PTR(-ENOKEY);
> +			goto done;
> +		}
> +
>  		ubifs_assert(c, fname_len(&nm) == 0);
>  		ubifs_assert(c, fname_name(&nm) == NULL);
>  		dent_key_init_hash(c, &key, dir->i_ino, nm.hash);
> @@ -529,6 +547,9 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  		if (err)
>  			return err;
>  
> +		if (c->fscrypt_key_required && !dir->i_crypt_info)
> +			return -ENOKEY;
> +

How about returning -ENOKEY when trying to open the directory in the first
place, rather than allowing getting to readdir()?  That would match the behavior
of regular files.

>  		err = fscrypt_fname_alloc_buffer(dir, UBIFS_MAX_NLEN, &fstr);
>  		if (err)
>  			return err;
> @@ -798,7 +819,8 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry)
>  			return err;
>  	}
>  
> -	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
> +	err = fscrypt_setup_filename(dir, &dentry->d_name, !c->fscrypt_key_required,
> +				     &nm);
>  	if (err)
>  		return err;
>  
> @@ -908,7 +930,8 @@ static int ubifs_rmdir(struct inode *dir, struct dentry *dentry)
>  			return err;
>  	}
>  
> -	err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm);
> +	err = fscrypt_setup_filename(dir, &dentry->d_name,
> +				     !c->fscrypt_key_required, &nm);
>  	if (err)
>  		return err;
>  
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 8dc2818fdd84..e081b00236b6 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -445,6 +445,9 @@ static int ubifs_show_options(struct seq_file *s, struct dentry *root)
>  			   ubifs_compr_name(c, c->mount_opts.compr_type));
>  	}
>  
> +	if (c->fscrypt_key_required)
> +		seq_puts(s, ",fscrypt_key_required");
> +
>  	seq_printf(s, ",assert=%s", ubifs_assert_action_name(c));
>  	seq_printf(s, ",ubi=%d,vol=%d", c->vi.ubi_num, c->vi.vol_id);
>  
> @@ -952,6 +955,7 @@ enum {
>  	Opt_assert,
>  	Opt_auth_key,
>  	Opt_auth_hash_name,
> +	Opt_fscrypt_key_required,
>  	Opt_ignore,
>  	Opt_err,
>  };
> @@ -969,6 +973,7 @@ static const match_table_t tokens = {
>  	{Opt_ignore, "ubi=%s"},
>  	{Opt_ignore, "vol=%s"},
>  	{Opt_assert, "assert=%s"},
> +	{Opt_fscrypt_key_required, "fscrypt_key_required"},
>  	{Opt_err, NULL},
>  };
>  
> @@ -1008,6 +1013,7 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>  {
>  	char *p;
>  	substring_t args[MAX_OPT_ARGS];
> +	unsigned int old_fscrypt_key_required = c->fscrypt_key_required;
>  
>  	if (!options)
>  		return 0;
> @@ -1099,6 +1105,15 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>  			if (!c->auth_hash_name)
>  				return -ENOMEM;
>  			break;
> +		case Opt_fscrypt_key_required:
> +			c->fscrypt_key_required = 1;
> +
> +			if (is_remount && (old_fscrypt_key_required != c->fscrypt_key_required)) {
> +				ubifs_err(c, "fscrypt_key_required cannot changed by remount");
> +				return -EINVAL;
> +			}
> +
> +			break;
>  		case Opt_ignore:
>  			break;
>  		default:
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 1ae12900e01d..71b03a6798ae 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -1304,6 +1304,7 @@ struct ubifs_info {
>  	unsigned int rw_incompat:1;
>  	unsigned int assert_action:2;
>  	unsigned int authenticated:1;
> +	unsigned int fscrypt_key_required:1;
>  
>  	struct mutex tnc_mutex;
>  	struct ubifs_zbranch zroot;
> -- 
> 2.21.0
> 

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Richard Weinberger]

Eric,

Am Donnerstag, 14. März 2019, 18:49:14 CET schrieb Eric Biggers:
> Hi Richard,
> 
> On Thu, Mar 14, 2019 at 06:15:59PM +0100, Richard Weinberger wrote:
> > Usually fscrypt allows limited access to encrypted files even
> > if no key is available.
> > Encrypted filenames are shown and based on this names users
> > can unlink and move files.
> 
> Actually, fscrypt doesn't allow moving files without the key.  It would only be
> possible for cross-renames, i.e. renames with the RENAME_EXCHANGE flag.  So for
> consistency with regular renames, fscrypt also forbids cross-renames if the key
> for either the source or destination directory is missing.
> 
> So the main use case for the ciphertext view is *deleting* files.  For example,
> deleting a user's home directory after that user has been removed from the
> system.  Or the system freeing up space by deleting cache files from a user who
> isn't currently logged in.

Right, I somehow thought beside of deleting you can do more.

> > 
> > This is not always what people expect. The fscrypt_key_required mount
> > option disables this feature.
> > If no key is present all access is denied with the -ENOKEY error code.
> 
> The problem with this mount option is that it allows users to create undeletable
> files.  So I'm not really convinced yet this is a good change.  And though the
> fscrypt_key_required semantics are easier to implement, we'd still have to
> support the existing semantics too, thus increasing the maintenance cost.

The undeletable-file argument is a good point. Thanks for bringing this up.
To get rid of such files root needs to mount without the new mount parameter. ;-\

> > 
> > The side benefit of this is that we don't need ->d_revalidate().
> > Not having ->d_revalidate() makes an encrypted ubifs usable
> > as overlayfs upper directory.
> > 
> 
> It would be preferable if we could get overlayfs to work without providing a
> special mount option.

Yes, but let's see what Al finds in his review.

> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > ---
> >  fs/ubifs/crypto.c |  2 +-
> >  fs/ubifs/dir.c    | 29 ++++++++++++++++++++++++++---
> >  fs/ubifs/super.c  | 15 +++++++++++++++
> >  fs/ubifs/ubifs.h  |  1 +
> >  4 files changed, 43 insertions(+), 4 deletions(-)
> > 
> 
> Shouldn't readlink() honor the mount option too?

Hmmm, yes. We need to honor it in ->get_link() too.

> > +		if (c->fscrypt_key_required && !dir->i_crypt_info)
> > +			return -ENOKEY;
> > +
> 
> How about returning -ENOKEY when trying to open the directory in the first
> place, rather than allowing getting to readdir()?  That would match the behavior
> of regular files.

I'm not sure what the best approach is.
We could also do it in ->permission().

Thanks,
//richard

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Theodore Ts'o]

Richard --- stepping back for a moment, in your use case, are you
assuming that the encryption key is always going to be present while
the system is running?

Ubifs can't use dm-crypt, since it doesn't have a block device, but if
you could, is much more like dm-crypt, in that you have the key
*before* the file system is mounted, and you don't really expect the
key to ever be expunged from the system while it is mounted?

If that's true, maybe the real mismatch is in using fscrypt in the
first place --- and in fact, something where you encrypt everything,
including the file system metadata (ala dm-crypt), would actually give
you much better security properties.

       	       		     	 	- Ted

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Richard Weinberger]

Ted,

Am Freitag, 15. März 2019, 00:07:02 CET schrieb Theodore Ts'o:
> Richard --- stepping back for a moment, in your use case, are you
> assuming that the encryption key is always going to be present while
> the system is running?

it is not a hard requirement, it is something what is common on embedded
systems that utilize UBIFS and fscrypt.

> Ubifs can't use dm-crypt, since it doesn't have a block device, but if
> you could, is much more like dm-crypt, in that you have the key
> *before* the file system is mounted, and you don't really expect the
> key to ever be expunged from the system while it is mounted?
> 
> If that's true, maybe the real mismatch is in using fscrypt in the
> first place --- and in fact, something where you encrypt everything,
> including the file system metadata (ala dm-crypt), would actually give
> you much better security properties.

Well, fscrypt was chosen as UBIFS encryption backend because per-file encryption
with derived keys makes a lot of sense.
Also the implementation was not super hard, David and I weren't keen to reinvent
dm-crypt für UBI/MTD.

That said, I'm happy with fscrypt, it works well in production.
But being not able to use UBIFS as lower dir on overlayfs hurts.
On embedded systems where the key is always present the proposed hack works
fine. If we can get overlayfs work without that I'll be more than happy.

Thanks,
//richard

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Theodore Ts'o]

On Fri, Mar 15, 2019 at 08:48:10AM +0100, Richard Weinberger wrote:
> Ted,
> 
> Am Freitag, 15. März 2019, 00:07:02 CET schrieb Theodore Ts'o:
> > Richard --- stepping back for a moment, in your use case, are you
> > assuming that the encryption key is always going to be present while
> > the system is running?
> 
> it is not a hard requirement, it is something what is common on embedded
> systems that utilize UBIFS and fscrypt.
> 
> Well, fscrypt was chosen as UBIFS encryption backend because per-file encryption
> with derived keys makes a lot of sense.
> Also the implementation was not super hard, David and I weren't keen to reinvent
> dm-crypt für UBI/MTD.
> 
> That said, I'm happy with fscrypt, it works well in production.

OK, but please note that fscrypt leaks i_size and timestamp
information; dm-crypt doesn't.  An enterprising attacker could very
easily be able to do something interesting with that information, so
be sure you've thought through what the threat model for users of
ubifs is going to be.

If you need per-user keying, and you need to be able to mount the file
system and access some of the files without having any keys, and if
it's useful for an admin to be able to delete files without having the
key, then fscrypt is a great fit.

You are proposing changes that (optionally) eliminate that last
advantage of fscrypt.  So I just wanted to sanity check whether or not
the other advantages are useful to you, and worth the security
tradeoffs that are inherent in such a choice.  If it's worth it, then
great.  But if it isn't, I'd much rather that you appropriately
protect your users and your customers rather than be an additional
user of fscrypt.  :-)

Cheers,

						- Ted

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Richard Weinberger]

Ted,

Am Freitag, 15. März 2019, 14:51:28 CET schrieb Theodore Ts'o:
> On Fri, Mar 15, 2019 at 08:48:10AM +0100, Richard Weinberger wrote:
> > Ted,
> > 
> > Am Freitag, 15. März 2019, 00:07:02 CET schrieb Theodore Ts'o:
> > > Richard --- stepping back for a moment, in your use case, are you
> > > assuming that the encryption key is always going to be present while
> > > the system is running?
> > 
> > it is not a hard requirement, it is something what is common on embedded
> > systems that utilize UBIFS and fscrypt.
> > 
> > Well, fscrypt was chosen as UBIFS encryption backend because per-file encryption
> > with derived keys makes a lot of sense.
> > Also the implementation was not super hard, David and I weren't keen to reinvent
> > dm-crypt für UBI/MTD.
> > 
> > That said, I'm happy with fscrypt, it works well in production.
> 
> OK, but please note that fscrypt leaks i_size and timestamp
> information; dm-crypt doesn't.  An enterprising attacker could very
> easily be able to do something interesting with that information, so
> be sure you've thought through what the threat model for users of
> ubifs is going to be.

No need to worry, I'm fully aware of all this.
 
> If you need per-user keying, and you need to be able to mount the file
> system and access some of the files without having any keys, and if
> it's useful for an admin to be able to delete files without having the
> key, then fscrypt is a great fit.
> 
> You are proposing changes that (optionally) eliminate that last
> advantage of fscrypt.  So I just wanted to sanity check whether or not
> the other advantages are useful to you, and worth the security
> tradeoffs that are inherent in such a choice.  If it's worth it, then
> great.  But if it isn't, I'd much rather that you appropriately
> protect your users and your customers rather than be an additional
> user of fscrypt.  :-)

Like I said, this patch series is an RFC, the mount option was suggested
by Amir and Miklos, so I assumed showing some code is a good base for further
discussion.
For most of *my* use-cases it works but having general support for fscrypt+overlayfs
would be the ultimate goal.

Thanks,
//richard

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[James Bottomley]

On Thu, 2019-03-14 at 18:15 +0100, Richard Weinberger wrote:
> Usually fscrypt allows limited access to encrypted files even
> if no key is available.
> Encrypted filenames are shown and based on this names users
> can unlink and move files.

Shouldn't they be able to read/write and create as well (all with the
ciphertext name and contents, of course) ... otherwise how does backup
of encrypted files by admin without the key ever work?

James

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[Theodore Ts'o]

On Thu, Mar 14, 2019 at 04:15:11PM -0700, James Bottomley wrote:
> On Thu, 2019-03-14 at 18:15 +0100, Richard Weinberger wrote:
> > Usually fscrypt allows limited access to encrypted files even
> > if no key is available.
> > Encrypted filenames are shown and based on this names users
> > can unlink and move files.
> 
> Shouldn't they be able to read/write and create as well (all with the
> ciphertext name and contents, of course) ... otherwise how does backup
> of encrypted files by admin without the key ever work?

That's not currently supported.  Michael Halcrow and I worked out some
designs on how to do this, and I even had some prototype patches, but
it's really, really, messy, and requires a lot of complex machinations
in userspace on the save *and* restore, since there's a lot of crypto
metadata that has to be saved, and you have to handle backing up the
directory per-file keys, and restoring them when you recreate the
directory on a restore, etc.

The simpler approach would have allowed backup of encrypted files
without the key, but would require the user's key to do the restore,
and if we ever actually tried to get this feature supported, that's
the approach I'd suggest.


The fundamental reason why we never went did anything with this was
that the original use case of fscrypt was for Chrome OS, where the
original design premise was that you don't need to do backups, since
everything is in the cloud.  A video from 2010:

	https://www.youtube.com/watch?v=lm-Vnx58UYo

And with Android, backups happen automatically while you have the
encryption key.

There was talk about using fscrypt for Ubuntu desktops as an ecryptfs
replacement, and in that case, we would have use case that would have
required backups of a shared desktop where you don't have all of the
encryption keys for all of the users.  Maybe someday as a Google
Summer of Code project or maybe if some potential corporate user of
fscrypt would be willing to fund the necessary engineering work?

But again, I'll repeat this because it's so important: In the vast
majority of cases, including single-user laptops, desktops, etc., the
real right answer is dm-crypt and *not* fscrypt.  Especially since
time-sharing systems are *so* 1980's.  :-)

So I don't use fscrypt on my upstream development laptop (except for
testing purposes) because it's simply not the right tool for the job.

		      	 	    - Ted

Re: [PATCH 4/4] ubifs: Implement new mount option, fscrypt_key_required

[James Bottomley]

On Thu, 2019-03-14 at 19:42 -0400, Theodore Ts'o wrote:
> On Thu, Mar 14, 2019 at 04:15:11PM -0700, James Bottomley wrote:
> > On Thu, 2019-03-14 at 18:15 +0100, Richard Weinberger wrote:
> > > Usually fscrypt allows limited access to encrypted files even
> > > if no key is available.
> > > Encrypted filenames are shown and based on this names users
> > > can unlink and move files.
> > 
> > Shouldn't they be able to read/write and create as well (all with
> > the ciphertext name and contents, of course) ... otherwise how does
> > backup of encrypted files by admin without the key ever work?
> 
> That's not currently supported.  Michael Halcrow and I worked out
> some designs on how to do this, and I even had some prototype
> patches, but it's really, really, messy, and requires a lot of
> complex machinations in userspace on the save *and* restore, since
> there's a lot of crypto metadata that has to be saved, and you have
> to handle backing up the directory per-file keys, and restoring them
> when you recreate the directory on a restore, etc.
> 
> The simpler approach would have allowed backup of encrypted files
> without the key, but would require the user's key to do the restore,
> and if we ever actually tried to get this feature supported, that's
> the approach I'd suggest.

Well, as I said above encrypted file backup by an admin who doesn't
have the key was what I was thinking of.

> The fundamental reason why we never went did anything with this was
> that the original use case of fscrypt was for Chrome OS, where the
> original design premise was that you don't need to do backups, since
> everything is in the cloud.  A video from 2010:
> 
> 	https://www.youtube.com/watch?v=lm-Vnx58UYo
> 
> And with Android, backups happen automatically while you have the
> encryption key.

Heh, colour me paranoid, but if I backup my sensitive data to any
medium (including the cloud) I *want* the backup to be encrypted so
that if someone is careless with my backup data at least they don't get
the contents.

> There was talk about using fscrypt for Ubuntu desktops as an ecryptfs
> replacement, and in that case, we would have use case that would have
> required backups of a shared desktop where you don't have all of the
> encryption keys for all of the users.  Maybe someday as a Google
> Summer of Code project or maybe if some potential corporate user of
> fscrypt would be willing to fund the necessary engineering work?
> 
> But again, I'll repeat this because it's so important: In the vast
> majority of cases, including single-user laptops, desktops, etc., the
> real right answer is dm-crypt and *not* fscrypt.  Especially since
> time-sharing systems are *so* 1980's.  :-)
> 
> So I don't use fscrypt on my upstream development laptop (except for
> testing purposes) because it's simply not the right tool for the job.

I was thinking of the container use case again.  It's not really backup
per se but it is serialization: if you can't do a backup ... i.e. save
and restore an encrypted tar image, you can't use the filesystem for
container image protection.  But it also strikes me that inability to
do backup and restore without the key really restricts the use cases
for the entire filesystem.

James

Re: overlayfs vs. fscrypt

[Eric Biggers]

Hi Miklos,

On Wed, Mar 13, 2019 at 02:24:47PM +0100, Miklos Szeredi wrote:
> On Wed, Mar 13, 2019 at 2:00 PM Richard Weinberger <richard@nod.at> wrote:
> >
> > Am Mittwoch, 13. März 2019, 13:58:11 CET schrieb Miklos Szeredi:
> > > On Wed, Mar 13, 2019 at 1:47 PM Richard Weinberger <richard@nod.at> wrote:
> > > >
> > > > Am Mittwoch, 13. März 2019, 13:36:02 CET schrieb Miklos Szeredi:
> > > > > I don't get it.  Does fscrypt try to check permissions via
> > > > > ->d_revalidate?  Why is it not doing that via ->permission()?
> > > >
> > > > Please let me explain. Suppose we have a fscrypto directory /mnt and
> > > > I *don't* have the key.
> > > >
> > > > When reading the directory contents of /mnt will return an encrypted filename.
> > > > e.g.
> > > > # ls /mnt
> > > > +mcQ46ne5Y8U6JMV9Wdq2C
> > >
> > > Why does showing the encrypted contents make any sense?  It could just
> > > return -EPERM on all operations?
> >
> > The use case is that you can delete these files if the DAC/MAC permissions allow it.
> > Just like on NTFS. If a user encrypts files, the admin cannot read them but can
> > remove them if the user is gone or loses the key.
> 
> There's the underlying filesystem view where admin can delete files,
> etc.   And there's the fscrypt layer stacked on top of the underlying
> fs, which en/decrypts files *in case the user has the key*.  What if
> one user has a key, but the other one doesn't?  Will d_revalidate
> constantly switch the set of dentries between the encrypted filenames
> and the decrypted ones?  Sounds crazy.  And the fact that NTFS does
> this doesn't make it any less crazy...
> 

fscrypt (aka ext4/f2fs/ubifs encryption) isn't a stacked filesystem.  I think
you're confusing it with eCryptfs.  There's only one "view" of the filesystem.

It's true that different processes can put different keys in their
process-subscribed keyrings, e.g. their session keyrings.  But that doesn't
change the fact that each cached inode either has the key or it doesn't, and all
users share those same cached inodes.  The mistake here is not making the keys
be provided at the filesystem level too, and I've proposed to fix that:
https://patchwork.kernel.org/cover/10821413/

Note that the the key (->i_crypt_info) is never removed from a cached inode
without evicting it.  It used to be done, but it was broken and removed.  Now a
cached inode can only have the key added.  For this reason and others, I think
fscrypt_d_revalidate() contains unneeded checks and can be simplified to this:

	static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
	{
		struct dentry *dir;
		bool valid;

		if (flags & LOOKUP_RCU)
			return -ECHILD;

		if (dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY)
			return 1;

		dir = dget_parent(dentry);
		valid = (d_inode(dir)->i_crypt_info == NULL);
		dput(dir);
		return valid;
	}

I think we can even support RCU mode too.

Then, one possibility is to move fscrypt_d_revalidate() to the VFS.  If we
replace DCACHE_ENCRYPTED_WITH_KEY with the opposite meaning, say
DCACHE_CIPHERTEXT_NAME, then the VFS will have everything it needs to just do
the equivalent of fscrypt_d_revalidate() directly in d_revalidate() in
fs/namei.c.  So fscrypt_d_ops won't be needed at all.  Something like this:

#ifdef CONFIG_FS_ENCRYPTION
static inline int fscrypt_d_revalidate(struct dentry *dentry,
                                       unsigned int flags)
{
	struct dentry *dir;
	struct inode *dir_inode;

	if (!(READ_ONCE(dentry->d_flags) & DCACHE_CIPHERTEXT_NAME))
		return 1;

	dir = READ_ONCE(dentry->d_parent);
	dir_inode = READ_ONCE(dir->d_inode);
	return READ_ONCE(dir_inode->i_crypt_info) == NULL;
}
#else
static inline int fscrypt_d_revalidate(struct dentry *dentry,
                                       unsigned int flags)
{
        return 1;
}
#endif

static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
{
        int status;

        if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
                status = dentry->d_op->d_revalidate(dentry, flags);
        else
                status = 1;

        if (status > 0)
                status = fscrypt_d_revalidate(dentry, flags);
        return status;
}


What do you think about this?

- Eric

Re: overlayfs vs. fscrypt

[Al Viro]

On Wed, Mar 13, 2019 at 08:01:27AM -0700, Eric Biggers wrote:

> What do you think about this?

That fscrypt might have some very deep flaws.  I'll need to RTFS and
review its model, but what I've seen in this thread so far is not
promising anything good.

It's not just overlayfs - there are all kinds of interesting trouble
possible just with fscrypt, unless I'm misparsing what had been said
so far.

Re: overlayfs vs. fscrypt

[Eric Biggers]

On Wed, Mar 13, 2019 at 04:11:48PM +0000, Al Viro wrote:
> On Wed, Mar 13, 2019 at 08:01:27AM -0700, Eric Biggers wrote:
> 
> > What do you think about this?
> 
> That fscrypt might have some very deep flaws.  I'll need to RTFS and
> review its model, but what I've seen in this thread so far is not
> promising anything good.
> 
> It's not just overlayfs - there are all kinds of interesting trouble
> possible just with fscrypt, unless I'm misparsing what had been said
> so far.

FYI, there *is* a known bug I was very recently made aware of and am planning to
fix.  When ->lookup() finds the plaintext name for a directory and the
ciphertext name is already in the dcache, d_splice_alias() will __d_move() the
existing dentry to the plaintext name.  But it doesn't set
DCACHE_ENCRYPTED_WITH_KEY, so the dentry incorrectly is still marked as a
ciphertext name and will be invalidated on the next lookup.  That's especially
problematic if the lookup that caused the __d_move() came from sys_mount().

I'm thinking the best fix is to have __d_move() propagate
DCACHE_ENCRYPTED_WITH_KEY from 'target' to 'dentry'.

- Eric