read raw key from stdin if --key isn't specified

[myomv100]

Hello everyone,

This pull request contains some additions to fscrypt so you can now pass base64 encoded raw key and use it without the need to create the raw key file on the file system and worry about its security.

Best

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[myomv100]

@googlebot I signed it!

[ebiggers]

Sorry for the slow response. A couple comments:

Storing a "raw key type" on disk feels like the wrong approach. This is really just another way of passing a raw key to the program, so there shouldn't be any need to store anything on-disk. Why not just make fscrypt read the key from standard input if it needs a raw key, is being run in noninteractive mode, and the --key argument isn't specified?

Also, please ensure that all tests are passing. See CONTRIBUTING.md for how to run them yourself. E.g., make format needs to be run.

[myomv100]

Sorry for the slow response. A couple comments:

Storing a "raw key type" on disk feels like the wrong approach. This is really just another way of passing a raw key to the program, so there shouldn't be any need to store anything on-disk. Why not just make fscrypt read the key from standard input if it needs a raw key, is being run in noninteractive mode, and the --key argument isn't specified?

Also, please ensure that all tests are passing. See CONTRIBUTING.md for how to run them yourself. E.g., make format needs to be run.

You mean we should remove the currently supported option of passing a raw key file that's stored on disk ?

Also writing to standard input a raw data will mess up bash environment. The terminals are not optimized to process binary data.
So --key shall get the key from stdin. And to make sure we don't mess up the terminal, we need to remove it from the menu of raw key prompt in the interactive mode.

Is this what you mean?

[ebiggers]

You mean we should remove the currently supported option of passing a raw key file that's stored on disk ?

No, that's not necessary. In noninteractive mode, if --key is specified then read the key from the specified file, otherwise read the key from stdin.

Also writing to standard input a raw data will mess up bash environment. The terminals are not optimized to process binary data.

This is only true in interactive mode.

It would be possible to make the interactive mode ask whether the user wants to provide the key as a key file or as a base64-encoded string. However, would this actually be useful? I think the main benefit of supporting providing the key on stdin is for noninteractive mode.

[myomv100]

You mean we should remove the currently supported option of passing a raw key file that's stored on disk ?

No, that's not necessary. In noninteractive mode, if --key is specified then read the key from the specified file, otherwise read the key from stdin.

Also writing to standard input a raw data will mess up bash environment. The terminals are not optimized to process binary data.

This is only true in interactive mode.

It would be possible to make the interactive mode ask whether the user wants to provide the key as a key file or as a base64-encoded string. However, would this actually be useful? I think the main benefit of supporting providing the key on stdin is for noninteractive mode.

Last commit includes reading raw key from stdin but you need to pass --key=- so we know you want to read it from stdin .

The dash character argument to point to stdin is barrowed from cryptsetup for exactly the same purpose which is reading key files.

I think we should change --key to a more specific raw key type. It may look like:

--keyfile interactive mode with file path and noninteractive from stdin
--keyb64str noninteractive accepts only b64 encoded keys from stdin.

[ebiggers]

Last commit includes reading raw key from stdin but you need to pass --key=- so we know you want to read it from stdin .

Any reason not to just read the key from stdin if --key isn't specified, as I suggested?

Also, please drop any obsolete changes from the pull request (and squash together commits as needed). There are still changes to add RawKeyType, but that shouldn't be needed as I explained earlier, right?

[myomv100]

Last commit includes reading raw key from stdin but you need to pass --key=- so we know you want to read it from stdin .

Any reason not to just read the key from stdin if --key isn't specified, as I suggested?

Also, please drop any obsolete changes from the pull request (and squash together commits as needed). There are still changes to add RawKeyType, but that shouldn't be needed as I explained earlier, right?

Done !

[ebiggers]

Can you please update cli-tests/t_encrypt_raw_key.sh to test this new feature?

Also, this breaks interactive use of raw_key:

$ fscrypt encrypt dir/
Should we create a new protector? [y/N] y
The following protector sources are available:
1 - Your login passphrase (pam_passphrase)
2 - A custom passphrase (custom_passphrase)
3 - A raw 256-bit key (raw_key)
Enter the source number for the new protector [2 - custom_passphrase]: 3
Enter a name for the new protector: raw_key

The program just hangs after that because it is reading from stdin.

As I suggested earlier, reading the key from stdin should only happen in noninteractive mode.

[ebiggers]

Why not just:

return crypto.NewFixedLengthKeyFromReader(bufio.NewReader(os.Stdin), metadata.InternalKeyLen)

[dimitry-ishenko]

This is so close to being done, but @myomv100 seems to have given up. I am not a Go guy, but I can give it a go 😉 to see if I can push it over the hump.

@ebiggers is there a preferred way of detecting non-interactive mode in Go? I've found the go-isatty package. Will that work?

[ebiggers]

@ebiggers is there a preferred way of detecting non-interactive mode in Go? I've found the go-isatty package. Will that work?

I think you can just use term.IsTerminal(), like cmd/fscrypt/keys.go uses.

[dimitry-ishenko]

@ebiggers 👍🏻 👍🏻 👍🏻 Please see PR #326 and feel free to close this one.