Re: [dm-crypt] Bugs/Undoucmented features

[Arno Wagner <arno@wagner.name>]

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@saout.de
> http://www.saout.de/mailman/listinfo/dm-crypt

-- 
Arno Wagner,     Dr. sc. techn., Dipl. Inform.,    Email: arno@wagner.name
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