[dm-crypt] Empty key files vs empty passwords in plain mode

[dm-crypt] Empty key files vs empty passwords in plain mode

[Quentin Lefebvre]

Hi,

I experienced some troubles recently with Debian's cryptsetup package 
(testing version), which version is 1.6.6.
I found out that empty key files get refused by cryptsetup, for example:
cat empty_file | cryptsetup --debug --key-file=- open --type plain 
/test1.loop test1
gets rejected.
The debug output directly leads to a test in utils_crypt.c that, I 
think, should be removed.

Indeed, empty passwords are accepted, so it make sense to accept also 
empty inputs.
Especially in Debian, where cryptdisks_start script calls:
/lib/cryptsetup/askpass | cryptsetup --key-file=- open --type [type] 
[src] [dst]

What do you think about this issue?
Shall I send a patch for that?

Best regards,
Quentin

PS: I checked against the git version, the problem is not solved and 
actually exactly the same.

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Quentin Lefebvre]

Hi,

Any idea about that?
Maybe there's a better place to discuss development issues?

Best regards,
Quentin

Le 19/11/2014 22:24, Quentin Lefebvre a écrit :
> Hi,
>
> I experienced some troubles recently with Debian's cryptsetup package
> (testing version), which version is 1.6.6.
> I found out that empty key files get refused by cryptsetup, for example:
> cat empty_file | cryptsetup --debug --key-file=- open --type plain
> /test1.loop test1
> gets rejected.
> The debug output directly leads to a test in utils_crypt.c that, I
> think, should be removed.
>
> Indeed, empty passwords are accepted, so it make sense to accept also
> empty inputs.
> Especially in Debian, where cryptdisks_start script calls:
> /lib/cryptsetup/askpass | cryptsetup --key-file=- open --type [type]
> [src] [dst]
>
> What do you think about this issue?
> Shall I send a patch for that?
>
> Best regards,
> Quentin
>
> PS: I checked against the git version, the problem is not solved and
> actually exactly the same.

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Milan Broz]

On 11/23/2014 01:44 PM, Quentin Lefebvre wrote:

>> I experienced some troubles recently with Debian's cryptsetup package
>> (testing version), which version is 1.6.6.
>> I found out that empty key files get refused by cryptsetup, for example:
>> cat empty_file | cryptsetup --debug --key-file=- open --type plain
>> /test1.loop test1
>> gets rejected.
>> The debug output directly leads to a test in utils_crypt.c that, I
>> think, should be removed.
>>
>> Indeed, empty passwords are accepted, so it make sense to accept also
>> empty inputs.
>> Especially in Debian, where cryptdisks_start script calls:
>> /lib/cryptsetup/askpass | cryptsetup --key-file=- open --type [type]
>> [src] [dst]
>>
>> What do you think about this issue?
>> Shall I send a patch for that?

Well, logically it should be the same. But reading empty keyfile never worked
AFAIK and IMHO the case that you encrypt device by empty keyfile by mistake
is more common...
I am tempting to say it is a safety feature than bug :-)

Anyway, please create issue on project page, https://code.google.com/p/cryptsetup/issues/list
If you have a patch, attach it there as well.
I will get to it but this is not really urgent issue to solve.

BTW I would better suggest that Debian uses pwquality library with some sane
defaults and will not allow users to enter so weak passwords in the first place...

(There is always --force-password switch so your issue is still kind of problem
for testing though.)

Milan

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Quentin Lefebvre]

On 23/11/2014 14:16, Milan Broz wrote :
> On 11/23/2014 01:44 PM, Quentin Lefebvre wrote:
>
>>> I experienced some troubles recently with Debian's cryptsetup package
>>> (testing version), which version is 1.6.6.
>>> I found out that empty key files get refused by cryptsetup, for example:
>>> cat empty_file | cryptsetup --debug --key-file=- open --type plain
>>> /test1.loop test1
>>> gets rejected.
>>> The debug output directly leads to a test in utils_crypt.c that, I
>>> think, should be removed.
>>>
>>> Indeed, empty passwords are accepted, so it make sense to accept also
>>> empty inputs.
>>> Especially in Debian, where cryptdisks_start script calls:
>>> /lib/cryptsetup/askpass | cryptsetup --key-file=- open --type [type]
>>> [src] [dst]
>>>
>>> What do you think about this issue?
>>> Shall I send a patch for that?
>
> Well, logically it should be the same. But reading empty keyfile never worked AFAIK

Right, and this is just because of a test that returns an error code in 
case the key file is empty.

> and IMHO the case that you encrypt device by empty keyfile by mistake
> is more common...

I agree and I think there should be at least a warning.

> I am tempting to say it is a safety feature than bug :-)
>
> Anyway, please create issue on project page, https://code.google.com/p/cryptsetup/issues/list
> If you have a patch, attach it there as well.

Sure, I'll do that. But which tool is preferred to write a patch for 
cryptsetup?

> I will get to it but this is not really urgent issue to solve.

In my opinion, the problem is, for example, when a user enters a blank 
password (not secured, sure) and then reboot and can't mount his file 
system because of a script piped to cryptsetup.

> BTW I would better suggest that Debian uses pwquality library with some sane
> defaults and will not allow users to enter so weak passwords in the first place...
>
> (There is always --force-password switch so your issue is still kind of problem
> for testing though.)

OK.
Thanks for this answer.

Cheers,
Quentin

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Milan Broz]

On 11/23/2014 03:01 PM, Quentin Lefebvre wrote:
...
>> Well, logically it should be the same. But reading empty keyfile never worked AFAIK
> 
> Right, and this is just because of a test that returns an error code in 
> case the key file is empty.
> 
>> and IMHO the case that you encrypt device by empty keyfile by mistake
>> is more common...
> 
> I agree and I think there should be at least a warning.

Maybe for luksFormat but not for plain case. Otherwise everyone with access
to logs or screen scroll up will see that password is empty.

I have a generic rule that cryptsetup output (even debug log) must not
contain usable information about your password or key.

(The exception is messages produced by pwquality library but these
prevents creating new volume, it will never appear when opening
existing one.)

>> I am tempting to say it is a safety feature than bug :-)
>>
>> Anyway, please create issue on project page, https://code.google.com/p/cryptsetup/issues/list
>> If you have a patch, attach it there as well.
> 
> Sure, I'll do that. But which tool is preferred to write a patch for 
> cryptsetup?

Whatever is applicable. The best is created with "git format-patch" way
so I can simply apply it to git if it is correct.

There is also repository mirror on github so pull request there will work as well.
(I will just not use github directly because it is not primary repo.)

Milan

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Quentin Lefebvre]

Le 23/11/2014 15:57, Milan Broz a écrit :
> On 11/23/2014 03:01 PM, Quentin Lefebvre wrote:
> ...
>>> Well, logically it should be the same. But reading empty keyfile never worked AFAIK
>>
>> Right, and this is just because of a test that returns an error code in
>> case the key file is empty.
>>
>>> and IMHO the case that you encrypt device by empty keyfile by mistake
>>> is more common...
>>
>> I agree and I think there should be at least a warning.
>
> Maybe for luksFormat but not for plain case. Otherwise everyone with access
> to logs or screen scroll up will see that password is empty.
>
> I have a generic rule that cryptsetup output (even debug log) must not
> contain usable information about your password or key.

OK, this makes sense.

>>> I am tempting to say it is a safety feature than bug :-)
>>>
>>> Anyway, please create issue on project page, https://code.google.com/p/cryptsetup/issues/list
>>> If you have a patch, attach it there as well.
>>
>> Sure, I'll do that. But which tool is preferred to write a patch for
>> cryptsetup?
>
> Whatever is applicable. The best is created with "git format-patch" way
> so I can simply apply it to git if it is correct.
>
> There is also repository mirror on github so pull request there will work as well.
> (I will just not use github directly because it is not primary repo.)

Thanks for the advice.

At this point, I think I'll try to write a patch that accepts an empty 
key file, except in the case where --force-password is set (actually I 
didn't know this parameter).

Best,
Quentin

Re: [dm-crypt] Empty key files vs empty passwords in plain mode

[Quentin Lefebvre]

Le 23/11/2014 16:29, Quentin Lefebvre a écrit :

> At this point, I think I'll try to write a patch that accepts an empty
> key file, except in the case where --force-password is set (actually I
> didn't know this parameter).

I just read the meaning of --force-password, which is for LUKS 
partition. So I'll finally ignore that and try to figure out something else.

Quentin