neithernut / pam_e4crypt

PAM module for unlocking transparently encrypted directories on ext4
GNU General Public License v2.0
18 stars 8 forks source link

Passphrase #28

Open jnvsor opened 6 years ago

jnvsor commented 6 years ago

Depends on #25 and #27

The entire wrapper system is brand new, but there are no breaking changes besides the ones already in #25 and #27

All told this PR and #25, #27 were thrown together in a few days so it needs a bit more thorough testing, but I feel confident it won't explode unless you do something strange with it.

I might push a commit later rewriting a large portion of the readme to give more baby-steps instructions. Perhaps the init script shoudl be part of the install target in the makefile? (No idea how to do that in cmake)

samuelsadok commented 6 years ago

I'd like to mention 2 scenarios that I think would be useful to consider:

So I suggest the following directory structure:

[...]/wrap/1111111111111111/auth0
[...]/wrap/1111111111111111/salt0
[...]/wrap/1111111111111111/auth1
[...]/wrap/1111111111111111/salt1
[...]/wrap/2222222222222222/auth0
[...]/wrap/2222222222222222/salt0

where 1111111111111111 and 2222222222222222 are the wrap-key identifiers as shown by keyctl show. Optionally we could add a description[n] for each auth[n] and another description for each wrap key.

As an illustration, I wrote a small script (based off your init_wrapper_files.sh) to handle the directory structure I just described. I didn't integrate this with the pam_e4crypt.so module yet but if we want to go this path maybe we should do the utility in C too and share most of the code between the utility and the module.

jnvsor commented 6 years ago

A single user might want to keep more than one encryption key (for different directories).

If they're being unlocked by the same passphrase there's no practical security there. As a placebo it might even be worse.

maybe we should do the utility in C too and share most of the code between the utility and the module

Not much to share. You'd be replacing a 54 line shell script with an extra ~1000 lines of C

A single directory encryption key might need to be accessible for multiple wrap keys (for different users).

Possible. Perhaps the whole wrapped decryption thing should just be a separate utility loaded into the so as a library. But that's backwards compatible with this so I don't think it's worth worrying about until this is tested and approved.

samuelsadok commented 6 years ago

If they're being unlocked by the same passphrase there's no practical security there. As a placebo it might even be worse.

Key wrapping in general does not add security, but it improves usability. Consider Alice and Bob both encrypt their home directory but they also share a photos directory. Clearly all three directories should have a distinct encryption key, otherwise either Alice or Bob need to reveal their home directory's key.

This demonstrates both use cases:

Not much to share. You'd be replacing a 54 line shell script with an extra ~1000 lines of C

Well 1000 lines is overstated. Regarding code sharing, here's what comes to my mind:

If you share the code you can have a high confidence that both the module and the utility behave the same and you can easily debug stuff if the module fails. Besides shell code is fragile and very easy to get wrong.

But that's backwards compatible with this so I don't think it's worth worrying about until this is tested and approved.

To make future changes easy I just suggest to replace all occurrences of $WRAPPATH/$USER by $WRAPPATH/$outerkeyhash (and the corresponding tweak in the module). I can make a PR against your branch if you want. Also you might wanna put most variables in double quotes (e.g. $SALTPATH/$USER => "$SALTPATH/$USER") in case they contain spaces.

jnvsor commented 6 years ago

Well 1000 lines is overstated

Not really. All of the C code that's there right now does is read files and handle encryption. All the shell file does is write files and encrypt a folder (Using a pre-existing tool for that matter)

You'd need to write from scratch:

1000 lines seems about right

samuelsadok commented 6 years ago

User input: scanf_s(). Permission elevation: setuid(). If you include the boilerplate code that's four lines each. I would have accepted the challenge, however now I first want to evaluate if fscrypt does what I need.

neithernut commented 6 years ago

@jnvsor It's been a while. Sorry for neglecting my duties.

I've finally decided to go with this PR. However, I do want to get myself a proper test-setup already. Hence, it may take a bit until I can give you a proper review.

jnvsor commented 6 years ago

Oh yeah I agree, encryption and PAM are two things you don't want to wing it with :)