Empty key files vs empty passwords in plain mode
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.
Hi,
Any idea about that?
Maybe there's a better place to discuss development issues?
Best regards,
Quentin
Quoted 25 lines · 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:
catempty_file| cryptsetup --debug --key-file=- open --type plain /test1.loop test1
gets rejected.
The debug output directly leads to a test inutils_crypt.cthat, I think, should be removed.Indeed, empty passwords are accepted, so it make sense to accept also empty inputs.
Especially in Debian, wherecryptdisks_startscript 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,
QuentinPS: I checked against the git version, the problem is not solved and actually exactly the same.
Quoted 9 lines · Quentin Lefebvre
Quoted 9 lines
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:
catempty_file| cryptsetup --debug --key-file=- open --type plain /test1.loop test1
gets rejected.
The debug output directly leads to a test inutils_crypt.cthat, I think, should be removed.
Quoted 8 lines
Quoted 8 lines
Indeed, empty passwords are accepted, so it make sense to accept also empty inputs.
Especially in Debian, wherecryptdisks_startscript 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
Quoted 17 lines · Milan Broz
Quoted 15 lines · Quentin Lefebvre
Quoted 15 lines
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:
catempty_file| cryptsetup --debug --key-file=- open --type plain /test1.loop test1
gets rejected.
The debug output directly leads to a test inutils_crypt.cthat, I think, should be removed.Indeed, empty passwords are accepted, so it make sense to accept also empty inputs.
Especially in Debian, wherecryptdisks_startscript calls:/lib/cryptsetup/askpass| cryptsetup --key-file=- open --type [type] [src] [dst]
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
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.
Quoted 2 lines
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.
Quoted 4 lines
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?
Quoted 1 line
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.
Quoted 5 lines
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
On 11/23/2014 03:01 PM, Quentin Lefebvre wrote:
...
Quoted 9 lines
Quoted 1 line
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.
Quoted 2 lines
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.)
Quoted 7 lines
Quoted 4 lines
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
Quoted 10 lines · Milan Broz
On 11/23/2014 03:01 PM, Quentin Lefebvre wrote:
...Quoted 8 lines
Quoted 1 line
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.
Quoted 2 lines
and IMHO the case that you encrypt device by empty keyfile by mistake is more common...
Quoted 7 lines
Quoted 1 line
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.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 2 lines
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
Quoted 3 lines · Quentin Lefebvre
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