search

libyal / libfvde

Library and tools to access FileVault Drive Encryption (FVDE) encrypted volumes
GNU Lesser General Public License v3.0
339 stars 34 forks source link

Changes to getopt and added option to prompt for password #30

Closed Rob--W closed 3 years ago

Rob--W commented 6 years ago

I have added two new options, -P and -R to prompt for the password and recovery password, respectively. The ability to specify a password without explicitly passing it through the command-line arguments fixes #16 and Debian bug 867110.

Before adding the feature, I fixed a few bugs, including major issues such undefined memory deallocation behavior on Windows and infinite loops in argument parsing.

I suggest to review each commit in isolation (and read their commit messages to understand the context of each commit).

joachimmetz commented 6 years ago

Rob thx, I have a look to integrate them as soon as time permits. Regarding the -P and -R do you know if they are portable to Windows?

Rob--W commented 6 years ago

Rob thx, I have a look to integrate them as soon as time permits. Regarding the -P and -R do you know if they are portable to Windows?

I tested on Windows (cross-compiled with MinGW-w64) and the behavior was identical to Linux (with the library-provided getopt). I haven't tested with MSVC, but I don't expect any issues since your project already uses parameterless options (e.g. -h, -v, -V).

(At first I tried to do something special upon detecting an empty string parameter. This works as expected on Linux, but not on Windows. So I used -P instead of -p "", which in retrospect looks better too.)

I explicitly accounted for the possibility of not having a password, having empty password and having non-empty passwords. Although I don't do any special input processing, passwords with multi-byte UTF-8 should work too because I don't touch bytes with the high order bit set.

joachimmetz commented 6 years ago

Heap free issue will be corrected by changes in: https://github.com/libyal/libyal/issues/49

Rob--W commented 6 years ago

@joachimmetz Do I need to rebase and remove the commit that fixed the HeapFree issue?

joachimmetz commented 6 years ago

keep the PR as is. I'll merge or integrate changes step by step, the getopt changes also need to go into libyal

joachimmetz commented 6 years ago

small update, I'll try to continue merging these changes shortly

joachimmetz commented 6 years ago

confirmed the infinite loop in the fallback getopt implementation, however the proposed changes do not match the getopt behavior. I'll make some adjustments before merge. Changed added to libyal to generate source files: https://github.com/libyal/libyal/commit/0c062b28e4d2042da72f6dd1146225d5ce608c0c

joachimmetz commented 3 years ago

Password related changes integrated in https://github.com/libyal/libfvde/commit/86e0c5bbbf81a9a9c746744b3fb6dbab700fca03. Note that it was solved differently then proposed. This should cover the proposed functionality. Closing issue.