dyne / tomb

the Crypto Undertaker
https://dyne.org/software/tomb
GNU General Public License v3.0
1.32k stars 150 forks source link

Improve KDF handling #527

Open Narrat opened 1 month ago

Narrat commented 1 month ago

Previously it wasn't possible to use argon2 as KDF function without the tomb tools from extras/kdf-keys being available. To change that behaviour introduce checks on the ARGON2 variable. Additionally add a fallback function to create a salt that is compatible to tomb-kdb-pbkdf2-gensalt.

Options specific for the different supported KDF algorithm are reorganized. Some options align between the various KDF and some are unique to them. The output of -h is enhanced with the various --kdf options and depends on the available optional tools. argon2 specific cli arguments won't be displayed if argon2 is not available.

Add case for results beside argon2 and pbkdf2. Key creation won't be stopped, just a warning is issued that the resulting key won't be protected via KDF.

Regarding the cli options. The argument for the suboption --kdf is made optional. In that regard one needs to make sure, that if --kdf is the last option before an argument one needs to use - to separate or use -k. Example: tomb forge --kdf - testkey.tomb Example: tomb forge --kdf -k testkey.tomb Example: tomb forge -k testkey.tomb --kdf

Additonally the kdf options are reorganized, which is a possible breaking change for scripts or GUI helpers.

Only --kdf is mandatory to get a key which is protected with KDF. For every other option safe defaults are set and can be optionally adjusted. KDF related subcommand options are removed where they don't come into play. gen_key() is only called in forge and passwd.


Description and such will be added if the path forward is clear. This is a WIP draft of the things mentioned in #526

Currently done: Make ARGON2 useful for the usage().

Narrat commented 1 month ago

The general change in behaviour is implemented. Adjustment of docs and tests are next on the agenda. Although I saw the CI also failed for the weblate update? Fails the CI in general? I don't really see a descriptive error from those runs.

And more thoroughly testing. Currently testing was done with the holy trinity of dig, forge and lock in all variations (argon2 only, argon2 and pbkdf2 tools available, pbkdf2, pbkdf2 without tomb-kdb-pbkdf2-gensalt).

jaromil commented 1 month ago

the tests should not fail in general. I see the problem here https://github.com/dyne/tomb/actions/runs/10100450578/job/27931822219?pr=527#step:6:706

Narrat commented 1 month ago

Okay. Hints at least at the expected location if it fails at the kdf tests. But why does it at also happen at the same stage for the weblatePR?

Narrat commented 1 month ago

Tests are adjusted, so this PR should be generally ready for review