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

Cleanup #27

Merged
merged 2 commits into from
Dec 12, 2017
Merged

Cleanup #27

merged 2 commits into from
Dec 12, 2017

Conversation

jnvsor
Copy link
Contributor

@jnvsor jnvsor commented Dec 3, 2017

Breaking change: Now takes the file contents verbatim. You don't
need the 's:' prefix any more, and the file can contain binary
data such as \0

You can simply remove the s: prefix from your salt file to get the
same decryption key after this patch.

Depends on #25 (I "cleaned up" the code mtab depended on)

@jnvsor jnvsor force-pushed the cleanup branch 2 times, most recently from 1b8921d to be45fe5 Compare December 3, 2017 12:10
@jnvsor jnvsor mentioned this pull request Dec 4, 2017
@neithernut
Copy link
Owner

Looks like this is not conflicting with changes introduced by #25. Could you resolve those conflicts? Also, I'd appreciate if you could split up the clean-ups into multiple commits. It would be much easier to review.

@jnvsor
Copy link
Contributor Author

jnvsor commented Dec 11, 2017

Here's a branch that will merge - I'm going to start working on splitting it up now

The salt struct exists mostly for e4crypt's input handling and to
keep a single type across multiple subcommands. It's of no use to
pam_e4crypt so we get rid of it.

Since we're not parsing user input and not using a salt we'll also
get rid of salt_parse and just use the raw contents of the salt
file as a salt.
We're storing salts in files, and we don't have mtab keys any more,
so we can ignore the filesystem encryption salt.
@jnvsor
Copy link
Contributor Author

jnvsor commented Dec 11, 2017

That should do it. Almost all of it was removing the user input parsing and refactoring related boilerplate

@jnvsor jnvsor mentioned this pull request Dec 11, 2017
@neithernut
Copy link
Owner

LGTM

@neithernut neithernut merged commit 912c0ed into neithernut:master Dec 12, 2017
@fbarthelery
Copy link

You may want to rewrite the salt generation commands in README.md

@jnvsor
Copy link
Contributor Author

jnvsor commented Mar 6, 2019

Honestly at this point I think we're better off using fscrypt since that seems to have a large userbase and more features.

The only features it's missing that are blocking me from using it is the (planned) ability to backup keys and to store pam keys on a separate home partition.

@neithernut
Copy link
Owner

neithernut commented Mar 7, 2019

I do agree with @jnvsor: fscrypt is far better maintained and has more features. Still, I'll keep maintaining this module in the future. As for the pace... well by know you probably can tell that my maintainership of this module is rather lacking.

I'll do some updates to the README during the next few days, including a reference to fscrypt.

@fbarthelery
Copy link

Yeah that's probably true. But pam_e4crypt works with a simple setup. And I'll need some investments to migrate to fscrypt (aka works for me now, I'll check that in the far future ^^)

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.

3 participants