On Sat, Jan 24, 2015 at 10:44:55 CET, Milan Broz wrote: > Hi, > > On 01/23/2015 10:33 PM, Ahmed, Safayet (GE Global Research) wrote: > > I've been looking into the cryptsetup tool and I just wanted to mention some > > bugs/undocumented features I came across. > > > > 1 detached headers > > ------------------ > > > > For a Luks setup that uses a detached header, a lot of the cryptsetup commands > > don't work in an intuitive way. Specifically, the "--header" option is ignored > > and cryptsetup throws an error that the specified device is not a Luks device. > > The cryptsetup commands work when the path to the actual luks device is > > replaced with the path to the header file. This was the case for the following > > commands: > > > > luksDump > > luksAddKey > > luksKillSlot > > > > I didn't check for the other commands. If this is the intended behavior, I > > think it should be mentioned in the doucmentation that the header file should > > be used in place of the <device> argument in the case of detached LUKS headers. > > The detached header was added later and only the commands that require > the --header were initially modified; commands above operates only with header device. > > But they should support --header as well and simply use it if specified, should be > easy to fix. > (The libcryptsetup has universal crypt_set_data_device() call.) > > I do not think we should document it as an exception, the --header should be simply supported > everywhere (despite the data device would be completely ignored, this seems to me like > a simpler way for the user - once you specify --header, it is used. > If not, code will try to use default arg.) Yes, I think that would be better as it does not silently switch between two different semantics of the positional argument. So no changes to the documentation. Arno > > > 2 master-key-file with new-key-file > > ----------------------------------- > > > > For the luksAddKey command, when the "--master-key-file" argument is specified, > > the new-key-file positional argument is ignored. Whether or not a file is > > provided for the new pass phrase, the user is prompted to enter a pass phrase. > > > > The problem is in the implementation of the function "action_luksAddKey" in > > "src/cryptsetup.c". Consider the following lines (916 - 927): > > > > if (opt_master_key_file) { > > r = _read_mk(opt_master_key_file, &key, keysize); > > if (r < 0) > > goto out; > > //FIXME: process keyfile arg > > r = crypt_keyslot_add_by_volume_key(cd, opt_key_slot, > > key, keysize, NULL, 0); > > } else if (opt_key_file || opt_new_key_file) { > > r = crypt_keyslot_add_by_keyfile_offset(cd, opt_key_slot, > > opt_key_file, opt_keyfile_size, opt_keyfile_offset, > > opt_new_key_file, opt_new_keyfile_size, opt_new_keyfile_offset); > > } else { > > > > When the master key is present, the code doesn't check "opt_new_key_file" and > > assumes that the user should be prompted for the pass phrase. > > > > Is this the intended behavior of luksAddKey? > > No, I think it is just missing code here. Only opt_new_keyfile_size should be parsed > here because master-key is provided (opt_key_file would be redundant). > > Milan > _______________________________________________ > dm-crypt mailing list > dm-crypt@xxxxxxxx > http://www.saout.de/mailman/listinfo/dm-crypt -- Arno Wagner, Dr. sc. techn., Dipl. Inform., Email: arno@xxxxxxxxxxx GnuPG: ID: CB5D9718 FP: 12D6 C03B 1B30 33BB 13CF B774 E35C 5FA1 CB5D 9718 ---- A good decision is based on knowledge and not on numbers. -- Plato If it's in the news, don't worry about it. The very definition of "news" is "something that hardly ever happens." -- Bruce Schneier _______________________________________________ dm-crypt mailing list dm-crypt@xxxxxxxx http://www.saout.de/mailman/listinfo/dm-crypt