Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filesystem keyring and v2 encryption policy support #148

Merged
merged 10 commits into from
Jan 23, 2020
Merged

Filesystem keyring and v2 encryption policy support #148

merged 10 commits into from
Jan 23, 2020

Conversation

ebiggers
Copy link
Collaborator

@ebiggers ebiggers commented Sep 18, 2019

This pull request adds support for the filesystem keyring and v2 encryption policies, which are new features in the kernel part of fscrypt in Linux v5.4. It also adds an fscrypt lock command, to mirror fscrypt unlock.

Users can opt into using v2 policies on new encrypted directories by setting "policy_version": "2" in /etc/fscrypt.conf. These directories always use the filesystem keyring, and they can be unlocked and locked by non-root users.

Alternatively, users can opt into using the filesystem keyring for existing directories by setting "use_fs_keyring_for_v1_policies": true. This keeps the directories compatible with older kernels, but makes unlocking and locking them start requiring root.

The filesystem keyring solves the issues users have been running into where processes unexpectedly don't get access to unlocked encrypted files. It also makes locking directories faster, more reliable, and not always require root.

For more information, see the cover letter of the kernel patchset, the updated kernel documentation, and the kernel pull request. Similar changes are being made to Android too.

@josephlr
Copy link
Member

Eric, this is great!! This will take me some time to get through, but thank you so much for splitting it up into individual commits.

A quick question. How much complexity is reduced if we don't bother with use_fs_keyring_for_v1_policies? We could either add this feature in later, or just make it easier for users to update their directories.

@josephlr josephlr self-requested a review September 18, 2019 09:58
@ebiggers
Copy link
Collaborator Author

A quick question. How much complexity is reduced if we don't bother with use_fs_keyring_for_v1_policies? We could either add this feature in later, or just make it easier for users to update their directories.

The diff from what I have now would look something like this: https://github.com/ebiggers/fscrypt/commit/f05f15ae873245f32f1fb15b842a5e2c2a44a46c

It's not a huge difference since we have to support the filesystem keyring anyway. But supporting use_fs_keyring_for_v1_policies does mean we have to document that setting, handle re-elevating to root to access the keyring in the PAM module, and a few other small things.

My main concern with not having this setting is that fscrypt will still be creating v1 encrypted directories by default for some time, and it's rather unfriendly to users to say "hey, you created the wrong type of encrypted directory, please delete everything and start over".

(I think I did screw it up slightly, though. As I've currently written it, "use_fs_keyring_for_v1_policies": true will break fscrypt if you downgrade your kernel. We should check that the ioctls are supported before we honor the setting.)

@Maryse47
Copy link

@josephlr could you consider merging this one? Linux 5.4 is around the corner and it would be good to have userspace that has v2 encryption policy support.

@ebiggers
Copy link
Collaborator Author

(I think I did screw it up slightly, though. As I've currently written it, "use_fs_keyring_for_v1_policies": true will break fscrypt if you downgrade your kernel. We should check that the ioctls are supported before we honor the setting.)

I've fixed this now: use_fs_keyring_for_v1_policies is only honored if the kernel supports it.

Also, Linux v5.4 has been released now, and I've updated the sys/unix package upstream with the new fscrypt declarations. I've rebased this onto #169 which does the sys/unix upgrade.

@josephlr can you review this when you have a chance? Thanks!

@josephlr
Copy link
Member

@josephlr can you review this when you have a chance? Thanks!

I’m currently in India, so I might not get to this until after Christmas, but will review as soon as I can.

@ebiggers
Copy link
Collaborator Author

@josephlr can you review this when you have a chance? Thanks!

I’m currently in India, so I might not get to this until after Christmas, but will review as soon as I can.

Okay, in the mean time do you mind if I start merging preparatory commits and some of my other pull requests that should be non-controversial? I'm thinking these commits:

  • travis: set GO111MODULE=on for 'go get'
  • Allow filesystem links to contain leading/trailing whitespace
  • cmd/fscrypt: adjust message when listing protector sources
  • Upgrade to latest golang.org/x/sys module
  • Use latest fscrypt declarations from sys/unix
  • Rename some variables from 'target' to 'targetUser'
  • cmd/fscrypt: preserve paragraphs in wrapText()

@josephlr
Copy link
Member

@ebiggers that seems reasonable to me

@ebiggers
Copy link
Collaborator Author

ebiggers commented Dec 3, 2019

Rebased onto latest master branch, and updated to make fscrypt status on an incompletely locked directory show Unlocked: Partially (incompletely locked) rather than Unlocked: No, as suggested by @mhogomchungu at https://github.com/ebiggers/fscrypt/commit/e4de7b4a524eef89eaf4279a05306e094a3539f6#commitcomment-36229352.

@josephlr josephlr mentioned this pull request Dec 4, 2019
In preparation for introducing support for the new filesystem-level
keyrings, move the existing user keyring management code from
security/keyring.go and crypto/crypto.go into a new package, 'keyring'.

This package provides functions AddEncryptionKey, RemoveEncryptionKey,
and GetEncryptionKeyStatus which delegate to either the filesystem
keyring (added by a later patch) or to the user keyring.  This provides
a common interface to both types of keyrings, to the extent possible.
Linux v5.4 and later allows fscrypt keys to be added/removed directly
to/from the filesystem via the new ioctls FS_IOC_ADD_ENCRYPTION_KEY and
FS_IOC_REMOVE_ENCRYPTION_KEY.  Among other benefits, these fix the key
visibility problems that many users have been running into, where system
services and containers can't access encrypted files.

Allow the user to opt-in to using these new ioctls for their existing
encrypted directories by setting in their /etc/fscrypt.conf:

	"use_fs_keyring_for_v1_policies": true

Note that it can't really be on by default, since for v1 policies the
ioctls require root, whereas user keyrings don't.  I.e., setting this to
true means that users will need to use 'sudo fscrypt unlock', not
'fscrypt unlock'.  v2 policies won't have this restriction.
Add support for 'fscrypt lock'.  This command "locks" a directory,
undoing 'fscrypt unlock'.

When the filesystem keyring is used, 'fscrypt lock' also detects when a
directory wasn't fully locked due to some files still being in-use.  It
can then be run again later to try to finish locking the files.
Don't force the user to provide a --user argument when running fscrypt
as root if they're doing something where the TargetUser isn't actually
needed, such as provisioning/deprovisioning a v1 encryption policy
to/from the filesystem keyring, or creating a non-login protector.

Also don't set up the user keyring (or check for it being set up) if it
won't actually be used.

Finally, if we'll be provisioning/deprovisioning a v1 encryption policy
to/from the filesystem keyring, make sure the command is running as
root, since the kernel requires this.
FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY require root
for v1 policy keys, so update the PAM module to re-acquire root
privileges while provisioning/deprovisioning policies that need this.

Also, only set up the user keyring if it will actually be used.
Linux v5.4 and later supports v2 encryption policies.  These have
several advantages over v1 encryption policies:

- Their encryption keys can be added/removed to/from the filesystem by
  non-root users, thus gaining the benefits of the filesystem keyring
  while also retaining support for non-root use.

- They use a more standard, secure, and flexible key derivation
  function.  Because of this, some future kernel-level fscrypt features
  will be implemented for v2 policies only.

- They prevent a denial-of-service attack where a user could associate
  the wrong key with another user's encrypted files.

Prepare the fscrypt tool to support v2 encryption policies by:

- Adding a policy_version field to the EncryptionOptions, i.e. to the
  config file and to the policy metadata files.

- Using the kernel-specified algorithm to compute the key descriptor for
  v2 policies.

- Handling setting and getting v2 policies.

Actually adding/removing the keys for v2 policies to/from the kernel is
left for the next patch.
Implement adding/removing v2 encryption policy keys to/from the kernel.
The kernel requires that the new ioctls FS_IOC_ADD_ENCRYPTION_KEY and
FS_IOC_REMOVE_ENCRYPTION_KEY be used for this.  Root is not required.

However, non-root support brings an extra complication: the kernel keeps
track of which users have called FS_IOC_ADD_ENCRYPTION_KEY for the same
key.  FS_IOC_REMOVE_ENCRYPTION_KEY only works as one of these users, and
it only removes the calling user's claim to the key; the key is only
truly removed when the last claim is removed.

Implement the following behavior:

- 'fscrypt unlock' and pam_fscrypt add the key for the user, even if
  other user(s) have it added already.  This behavior is needed so that
  another user can't remove the key out from under the user.

- 'fscrypt lock' and pam_fscrypt remove the key for the user.  However,
  if the key wasn't truly removed because other users still have it
  added, 'fscrypt lock' prints a warning.

- 'fscrypt status' shows whether the directory is unlocked for anyone.
Allow root to provide the --all-users option to 'fscrypt lock' to force
an encryption key to be removed from the filesystem (i.e., force an
encrypted directory to be locked), even if other users have added it.

To implement this option, we just need to use the
FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS ioctl rather than
FS_IOC_REMOVE_ENCRYPTION_KEY.

In theory this option could be implemented for the user keyrings case
too, but it would be difficult and the user keyrings are being
deprecated for fscrypt, so don't bother.
Document the new /etc/fscrypt.conf settings for the filesystem keyring
and v2 encryption policies, and add a new subsection for troubleshooting
key access problems.
@ebiggers
Copy link
Collaborator Author

ebiggers commented Jan 5, 2020

@josephlr can you please review this?

FYI, I'm also thinking of adding support for setting policy_version: "auto" in /etc/fscrypt.conf, which would make v2 policies be used iff the kernel supports them; and possibly making that the default setting. That can be done later, though.

@ebiggers
Copy link
Collaborator Author

This has been out for review for 4 months now, and people keep running into the problems this fixes. So I'm going ahead and merging it. @josephlr please feel free to still review it later, and I'd be glad to address any suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants