[dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Quentin Lefebvre]

Dear cryptsetup maintainer,

I discovered a functional bug in the recently added BitLocker support 
(thanks by the way!)
Not sure whether it is the place to report it...

Summary:
cryptsetup 2.3.0 cannot open a bitlk device when I provide a file 
containing its password (fails with error -2).

Command failing:

cryptsetup open --type bitlk --key-file <my_password_file> 
/dev/<bitlocker_device> bitlocker_mapping

It is very easy to reproduce, so I don't copy paste the --debug output.

I digged in the source code and found the problem in 
src/cryptsetup.c:543-545, where opt_keyfile_size is used when calling 
tools_get_key().

The workaround is actually to provide the exact password size on the 
command line, e.g.:

cryptsetup open --type bitlk --keyfile-size 55 --key-file 
<my_recovery_password_file> /dev/<bitlocker_device> bitlocker_mapping

Sorry not to provide a patch but I'm sure you will make it work in next 
releases.

Hope this report helps a bit.

Regards,
Quentin

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Milan Broz]

Hi,

On 19/02/2020 17:05, Quentin Lefebvre wrote:
> Dear cryptsetup maintainer,
> 
> I discovered a functional bug in the recently added BitLocker support 
> (thanks by the way!)
> Not sure whether it is the place to report it...

The best is to report issue here
  https://gitlab.com/cryptsetup/cryptsetup/issues
but this list works as well.

> 
> Summary:
> cryptsetup 2.3.0 cannot open a bitlk device when I provide a file 
> containing its password (fails with error -2).
> 
> Command failing:
> 
> cryptsetup open --type bitlk --key-file <my_password_file> 
> /dev/<bitlocker_device> bitlocker_mapping
> 
> It is very easy to reproduce, so I don't copy paste the --debug output.

The debug output provides information of all subsystem versions,
this could be important. This is why I require it on all reports.

> 
> I digged in the source code and found the problem in 
> src/cryptsetup.c:543-545, where opt_keyfile_size is used when calling 
> tools_get_key().

And default is 0, so the full keyfile is read (limited by maximal keyfile size).
So it should work...

> The workaround is actually to provide the exact password size on the 
> command line, e.g.:
> 
> cryptsetup open --type bitlk --keyfile-size 55 --key-file 
> <my_recovery_password_file> /dev/<bitlocker_device> bitlocker_mapping

Hm, so what is the on-disk keyfile size here? In cryptsetup, the whole
keyfile is read, unless you limit it (as in your workaround).

So what is the exact content of your keyfile? Is there additional EOL,
or it had some more characters there?

Milan

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Quentin Lefebvre]

Hi Milan,

Actually there will be no debug output as I found the problem (lying 
between the chair and the computer), thanks to your help.
See below for the explanation.

Le 19/02/2020 à 20:30, Milan Broz a écrit :> Hi,
 >
 > On 19/02/2020 17:05, Quentin Lefebvre wrote:
 >> Dear cryptsetup maintainer,
 >>
 >> I discovered a functional bug in the recently added BitLocker support
 >> (thanks by the way!)
 >> Not sure whether it is the place to report it...
 >
 > The best is to report issue here
 >    https://gitlab.com/cryptsetup/cryptsetup/issues
 > but this list works as well.

Ok, it may be useful another day.

 >> Summary:
 >> cryptsetup 2.3.0 cannot open a bitlk device when I provide a file
 >> containing its password (fails with error -2).
 >>
 >> Command failing:
 >>
 >> cryptsetup open --type bitlk --key-file <my_password_file>
 >> /dev/<bitlocker_device> bitlocker_mapping
 >>
 >> It is very easy to reproduce, so I don't copy paste the --debug output.
 >
 > The debug output provides information of all subsystem versions,
 > this could be important. This is why I require it on all reports.

That's fair ;-)

 >> I digged in the source code and found the problem in
 >> src/cryptsetup.c:543-545, where opt_keyfile_size is used when calling
 >> tools_get_key().
 >
 > And default is 0, so the full keyfile is read (limited by maximal 
keyfile size).
 > So it should work...
 >
 >> The workaround is actually to provide the exact password size on the
 >> command line, e.g.:
 >>
 >> cryptsetup open --type bitlk --keyfile-size 55 --key-file
 >> <my_recovery_password_file> /dev/<bitlocker_device> bitlocker_mapping
 >
 > Hm, so what is the on-disk keyfile size here? In cryptsetup, the whole
 > keyfile is read, unless you limit it (as in your workaround).

The on-disk keyfile was the printed recovery passphrase provided by 
Windows...

 > So what is the exact content of your keyfile? Is there additional EOL,
 > or it had some more characters there?

ls -l <my_recovery_password_file>
showed a 56 bytes file and

hexdump -C <my_recovery_password_file>
showed a trailing '\x0a' character added by my favorite editor.

Removing it with dd(!) solved my problem...

Sorry for the useless message (I should have triple-checked) and thanks 
again for the new feature.

Regards,
Quentin

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Milan Broz]

On 19/02/2020 21:13, Quentin Lefebvre wrote:
> The on-disk keyfile was the printed recovery passphrase provided by 
> Windows...
> 
>  > So what is the exact content of your keyfile? Is there additional EOL,
>  > or it had some more characters there?
> 
> ls -l <my_recovery_password_file>
> showed a 56 bytes file and
> 
> hexdump -C <my_recovery_password_file>
> showed a trailing '\x0a' character added by my favorite editor.
> 
> Removing it with dd(!) solved my problem...

ok, thanks for the info.

But I think we should perhaps process even this file (recovery
passphrase has a special format - if it is detected, we try to use it,
otherwise we parse it as a normal passphrase).
I think we can do this even if it is has an additional EOL there.

Thanks,
Milan

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Arno Wagner]

On Wed, Feb 19, 2020 at 21:57:23 CET, Milan Broz wrote:
> On 19/02/2020 21:13, Quentin Lefebvre wrote:
> > The on-disk keyfile was the printed recovery passphrase provided by 
> > Windows...
> > 
> >  > So what is the exact content of your keyfile? Is there additional EOL,
> >  > or it had some more characters there?
> > 
> > ls -l <my_recovery_password_file>
> > showed a 56 bytes file and
> > 
> > hexdump -C <my_recovery_password_file>
> > showed a trailing '\x0a' character added by my favorite editor.
> > 
> > Removing it with dd(!) solved my problem...
> 
> ok, thanks for the info.
> 
> But I think we should perhaps process even this file (recovery
> passphrase has a special format - if it is detected, we try to use it,
> otherwise we parse it as a normal passphrase).
> I think we can do this even if it is has an additional EOL there.

I would suggest giving a verbose warning in this case.
Something like, "recovery passphrase detected, extra EOL chars stripped."

Regards,
Arno

-- 
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

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Milan Broz]

On 20/02/2020 03:23, Arno Wagner wrote:
> On Wed, Feb 19, 2020 at 21:57:23 CET, Milan Broz wrote:
>> On 19/02/2020 21:13, Quentin Lefebvre wrote:
>>> The on-disk keyfile was the printed recovery passphrase provided by 
>>> Windows...
>>>
>>>  > So what is the exact content of your keyfile? Is there additional EOL,
>>>  > or it had some more characters there?
>>>
>>> ls -l <my_recovery_password_file>
>>> showed a 56 bytes file and
>>>
>>> hexdump -C <my_recovery_password_file>
>>> showed a trailing '\x0a' character added by my favorite editor.
>>>
>>> Removing it with dd(!) solved my problem...
>>
>> ok, thanks for the info.
>>
>> But I think we should perhaps process even this file (recovery
>> passphrase has a special format - if it is detected, we try to use it,
>> otherwise we parse it as a normal passphrase).
>> I think we can do this even if it is has an additional EOL there.
> 
> I would suggest giving a verbose warning in this case.
> Something like, "recovery passphrase detected, extra EOL chars stripped."

If it fails to open through a recovery passphrase slot, then code tries it as a normal
one (key derivation is quite fast here, so it is just a small delay) - so this
is really not necessary.

m.

Re: [dm-crypt] Bug in cryptsetup 2.3.0 (BitLocker key-file support)

[Quentin Lefebvre]

Hi Milan, Arno,

Thanks for your answers.
Below, my humble point of view...

Le 20/02/2020 à 07:37, Milan Broz a écrit :
> On 20/02/2020 03:23, Arno Wagner wrote:
>> On Wed, Feb 19, 2020 at 21:57:23 CET, Milan Broz wrote:
>>> On 19/02/2020 21:13, Quentin Lefebvre wrote:
>>>> The on-disk keyfile was the printed recovery passphrase provided by
>>>> Windows...
>>>>
>>>>   > So what is the exact content of your keyfile? Is there additional EOL,
>>>>   > or it had some more characters there?
>>>>
>>>> ls -l <my_recovery_password_file>
>>>> showed a 56 bytes file and
>>>>
>>>> hexdump -C <my_recovery_password_file>
>>>> showed a trailing '\x0a' character added by my favorite editor.
>>>>
>>>> Removing it with dd(!) solved my problem...
>>>
>>> ok, thanks for the info.
>>>
>>> But I think we should perhaps process even this file (recovery
>>> passphrase has a special format - if it is detected, we try to use it,
>>> otherwise we parse it as a normal passphrase).
>>> I think we can do this even if it is has an additional EOL there.
>>
>> I would suggest giving a verbose warning in this case.
>> Something like, "recovery passphrase detected, extra EOL chars stripped."
> 
> If it fails to open through a recovery passphrase slot, then code tries it as a normal
> one (key derivation is quite fast here, so it is just a small delay) - so this
> is really not necessary.

I think it would be a bit surprising (and a bit dangerous maybe) not to 
have a consistent behavior of cryptsetup between a bitlk recovery 
passphrase and a bitlk "normal" passphrase.

Although it is possible to add code to detect a bitlk recovery 
passphrase, it may be confusing for the user if cryptsetup is not 
consistent for all kinds of passphrases (bitlocker, luks, ...)

But suggestions are always good and here is mine:
When cryptsetup attemps to use a passphrase provided in a file:
1) try it,
2) only if the provided passphrase file fails, test whether there is a 
trailing EOL (maybe Unix/Windows-like) and try again cutting it (here I 
agree with Arno the user should be warned).

IMHO, this solution may be easier to implement and it has the benefits 
to be consistent for all kind of encrypted containers, and also to make 
the user aware of the EOL problem in his file (and learn more about his 
favorite text editor ;-).

Have a nice day,
Quentin