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

Allow fscrypt to work in containers #213

Merged
merged 1 commit into from
Apr 17, 2020
Merged

Allow fscrypt to work in containers #213

merged 1 commit into from
Apr 17, 2020

Conversation

ebiggers
Copy link
Collaborator

@ebiggers ebiggers commented Apr 1, 2020

Update the /proc/self/mountinfo parsing code to allow selecting a Mount
with Subtree != "/", i.e. a Mount not of the full filesystem. This is
needed to allow fscrypt to work in containers, where the root of the
filesystem may not be mounted.

See findMainMount() for details about the algorithm used.

Resolves #211

Update the /proc/self/mountinfo parsing code to allow selecting a Mount
with Subtree != "/", i.e. a Mount not of the full filesystem.  This is
needed to allow fscrypt to work in containers, where the root of the
filesystem may not be mounted.

See findMainMount() for details about the algorithm used.

Resolves #211
@ebiggers
Copy link
Collaborator Author

ebiggers commented Apr 8, 2020

@josephlr, are you planning to review this?

@ebiggers ebiggers merged commit fe86093 into google:master Apr 17, 2020
@ebiggers ebiggers deleted the container-support branch April 17, 2020 03:44
@josephlr
Copy link
Member

Hey, sorry for not looking at this sooner. My main concern with any change like this is that we would be too permissive and we would put the .fscrypt directory at a bad location, potentially confusing users or leading to data loss.

What about the following algorithm (for when a device has multiple mounts)?

  1. If one of the mountpoints contains all the other mountpoints, choose that one
  2. If one of the subtrees contains all the other subtrees, choose that one

Note: For all of these (as before) we prefer R/W to R/O if things are otherwise identical.

If neither (1) nor (2) identity a unique mount (or they disagree), we say the situation is ambiguous. This also seems easier to explain/document, and is still a strict extension of the existing behavior, as only picking non-bind mounts (i.e. Subtree = "/") is a subset of (2).

With this simplification, all the tests would stay passing except for TestLoadContainedSubtreesAndMountpoints. But looking at mountinfo for that test:

222 15 259:3 /0   /tpm/a/b rw shared:1 - ext4 /dev/root rw
222 15 259:3 /1   /tpm/a   rw shared:1 - ext4 /dev/root rw
222 15 259:3 /2   /tpm/a/c rw shared:1 - ext4 /dev/root rw
222 15 259:3 /1/3 /tpm/d   rw shared:1 - ext4 /dev/root rw
222 15 259:3 /2/4 /tpm/e   rw shared:1 - ext4 /dev/root rw

It's not clear to me that any of those are the "main" mountpoint (despite the algorithm choosing /1).

Would we lose anything by implementing this simplification?

@ebiggers
Copy link
Collaborator Author

The problem is that if someone had independent subtrees in one mountpoint tree (like the first three entries above, or like the example from #211 but rooted somewhere other than /), and then they bind-mounted some file or directory in that mountpoint tree to outside it (like entries 4-5 above), then with your proposal fscrypt would suddenly break as it would suddenly consider the mountpoints to be ambiguous. I don't think that's desirable. Instead, we should detect that the additional mounts are just bind mounts that are contained in what we had before. (In the above example, entries 4 and 5 are contained in 2 and 3 respectively.) Does that make sense?

@josephlr
Copy link
Member

@ebiggers that makes sense to me. We take the mounts, discard contained subtrees, then find the the mountpoint that contains all remaining mounts (if it exists).

Thanks again for dealing with the complexity in fixing this!!

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.

[Question] [lxc] possible to use fscrypt inside a Linux Container (lxc)
2 participants