fscrypt
fscrypt copied to clipboard
google
Don't loop forever if `fscrypt unlock` doesn't have a tty
This fixes the bug in two places:
- Don't retry passwords if stdin isn't a tty.
- Notice and error out when no input is available when reading a passphrase. Technically this isn't necessary now that we're not retrying, but it avoids needlessly spending time trying to unlock the protector.
This still retries empty passwords when interactive.
Fixes https://github.com/google/fscrypt/issues/429
Sorry, I've been busy. I'm taking a look at this again.
If I understand correctly, removing support for empty passwords is actually unnecessary to fix the issue. Would you mind splitting that change into a separate commit, or even a separate pull request?
Also, I tested fscrypt encrypt --quiet, and it does actually accept empty passwords currently, so removing support for them would technically be a breaking change. The use case for an empty password would be people setting a default password. For example, a system administrator could set up a new encrypted directory on behalf of a user using a default password initially, and the user could optionally set a real password later.
Empty passwords aren't actually necessary to support that use case, as people can just choose any other default password, like "default" or whatever. Empty passwords should not have been allowed.
But since they were allowed, I'm not sure we can rule out that someone is using them.
If I understand correctly, removing support for empty passwords is actually unnecessary to fix the issue. Would you mind splitting that change into a separate commit, or even a separate pull request?
i have removed all the stuff about empty passwords. i don't feel strongly about disallowing empty passwords so i'm not going to open a separate PR.
i do want to note that this is now more prone to the original bug than my original PR—if stdin is a tty, but no input is available (maybe because we're running under expect ), we'll still loop forever trying to read input. i can fix that without disallowing empty passwords, but you didn't like my fix (#430), so i'm not sure what to do there.