[dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Ludwig Nussel]

Hi,

There's no way to determine whether e.g. the keymap on the console is
the same as in X. Ie a key with umlauts added in an xterm may not be
usable during boot. So when using e.g. an encrypted root partition
users could lock themselves out. So I wonder whether a patch like
the following would be acceptable?

cu
Ludwig

---
 lib/utils.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index d23619e..df9c9a5 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -3,6 +3,7 @@
 #include <stdlib.h>
 #include <stddef.h>
 #include <stdarg.h>
+#include <ctype.h>
 #include <errno.h>
 #include <linux/fs.h>
 #include <sys/types.h>
@@ -378,6 +379,20 @@ out_err:
 	return failed;
 }
 
+static void warnifnotascii(struct crypt_device *ctx, const char *str)
+{
+	for(;*str; ++str) {
+		if(!isascii(*str)) {
+			log_std(ctx,
+				"* Warning: Entering non-ASCII passwords\n"
+				"* may not be possible on all systems.\n"
+				"* Make sure you can unlock the volume in\n"
+				"* the intended environment!\n");
+			break;
+		}
+	}
+}
+
 /*
  * Password reading behaviour matrix of get_key
  * 
@@ -443,6 +458,7 @@ void get_key(char *prompt, char **key, unsigned int *passLen, int key_size,
 				goto out_err;
 			}
 			memset(pass_verify, 0, sizeof(pass_verify));
+			warnifnotascii(cd, pass);
 		}
 		*passLen = strlen(pass);
 		*key = pass;
-- 
1.6.4.2

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Ludwig Nussel]

Ludwig Nussel wrote:
> There's no way to determine whether e.g. the keymap on the console is
> the same as in X. Ie a key with umlauts added in an xterm may not be
> usable during boot. So when using e.g. an encrypted root partition
> users could lock themselves out. So I wonder whether a patch like
> the following would be acceptable?

Any opinions?

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\   
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Ian McDonald]

On Tue, Nov 3, 2009 at 8:47 AM, Ludwig Nussel <ludwig.nussel@suse.de> wrote:
> Ludwig Nussel wrote:
>> There's no way to determine whether e.g. the keymap on the console is
>> the same as in X. Ie a key with umlauts added in an xterm may not be
>> usable during boot. So when using e.g. an encrypted root partition
>> users could lock themselves out. So I wonder whether a patch like
>> the following would be acceptable?
>
> Any opinions?

I see it this way: a user setting up disk encryption or even an encrypted root
file system should already know that there is usually only a US keyboard
layout available at boot time (or whatever the kernel is configured to use);
thus he would pick his password accordingly.  If someone really uses a
password he cannot enter at boot time he will have to fix the problem from a
rescue disc or consult a more experienced user to help him with that.  No data
is actually lost as the password is still usable.

Furthermore, I think that what you have described here is a scenario that
would only happen to users with an extremely hazardous behaviour.  When a
newbie sets up his disk for encryption he will try it out on a non-critical
system just to see how it works.  I know no one who would go on to encrypt his
data with the dm-crypt infrastructure if he hasn't a single clue about how it
all works.  The instinctive fear of losing one's own data should form a
reasonable protection against this kind of behaviour.

I think that it would be sufficient if the cryptsetup manpage explained the
dangers of differring key maps in detail and recommended that passwords should
always be entered from the console and not from terminals under X.  A patch
like the one you provided would be overkill in my opinion as it alters the
behaviour of cryptsetup where improved documentation and user education should
be the way to go.

Regards,
Ian.

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Arno Wagner]

On Tue, Nov 03, 2009 at 08:47:49AM +0100, Ludwig Nussel wrote:
> Ludwig Nussel wrote:
> > There's no way to determine whether e.g. the keymap on the console is
> > the same as in X. Ie a key with umlauts added in an xterm may not be
> > usable during boot. So when using e.g. an encrypted root partition
> > users could lock themselves out. So I wonder whether a patch like
> > the following would be acceptable?
> 
> Any opinions?

I think this is the wrong approach. Having a warning prominently
in the man-page is better. Although an option to echo input
back at the user, clearly marked "debug only" in man-page and
--help output would occasionally be useful.

Arno
-- 
Arno Wagner, Dr. sc. techn., Dipl. Inform., CISSP -- Email: arno@wagner.name 
GnuPG:  ID: 1E25338F  FP: 0C30 5782 9D93 F785 E79C  0296 797F 6B50 1E25 338F
----
Cuddly UI's are the manifestation of wishful thinking. -- Dylan Evans

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] [PATCH] print warning when adding a key that contains non-ASCII characters

[Uwe Menges]

Ludwig Nussel wrote:
> Any opinions?

Your patch uses the isascii() call, but the problem "different keymap at
different password entering times" is IMHO not really related to the
question "is the character ascii or not". It's all too easy to run into
exact the same situation you want to prevent, with just plain ascii
chars in the password.

Yours, Uwe

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Heinz Diehl]

On 04.11.2009, Uwe Menges wrote: 

> Your patch uses the isascii() call, but the problem "different keymap at
> different password entering times" is IMHO not really related to the
> question "is the character ascii or not". It's all too easy to run into
> exact the same situation you want to prevent, with just plain ascii
> chars in the password.

Yes, of course, it can be quite a little bit of a problem if the charset of a let's say
Norwegian keyboard (in my case) is turned into an US mapped one at booting
time.

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Milan Broz]

On 10/22/2009 04:16 PM, Ludwig Nussel wrote:
> There's no way to determine whether e.g. the keymap on the console is
> the same as in X. Ie a key with umlauts added in an xterm may not be
> usable during boot. So when using e.g. an encrypted root partition
> users could lock themselves out. So I wonder whether a patch like
> the following would be acceptable?

> +				"* Warning: Entering non-ASCII passwords\n"
> +				"* may not be possible on all systems.\n"
> +				"* Make sure you can unlock the volume in\n"
> +				"* the intended environment!\n");

Hi,

I don't think this in good idea. Information that user entered non-ASCII
character is useful for attacker, why display it to terminal?
(That problem exist in all password entry dialogs - why cryptsetup should
be special here?)

This should be solved in environment before cryptsetup starts - set proper
keymap (even during boot). IIRC Fedora support setting keymap in boot environment now,
there were similar bug reports already:-)

Also I expect that in future libcryptsetup will be used more (instead
of wrapper over cryptsetup binary and the whole internal code
for reading password will be moved to caller application which is then responsible
for password from terminal reading stuff.

Milan
--
mbroz@redhat.com

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Ludwig Nussel]

Milan Broz wrote:
> On 10/22/2009 04:16 PM, Ludwig Nussel wrote:
> > There's no way to determine whether e.g. the keymap on the console is
> > the same as in X. Ie a key with umlauts added in an xterm may not be
> > usable during boot. So when using e.g. an encrypted root partition
> > users could lock themselves out. So I wonder whether a patch like
> > the following would be acceptable?
> 
> > +				"* Warning: Entering non-ASCII passwords\n"
> > +				"* may not be possible on all systems.\n"
> > +				"* Make sure you can unlock the volume in\n"
> > +				"* the intended environment!\n");
> 
> I don't think this in good idea. Information that user entered non-ASCII
> character is useful for attacker, why display it to terminal?
> (That problem exist in all password entry dialogs - why cryptsetup should
> be special here?)

It's only displayed if you add a new password. I doubt you usually
do that if someone's looking at your screen. He could just look at
your keyboard while you are typing as well then, right? :-)
Anyways, if that's a concern for you the warning could be displayed
unconditionally before prompting for a password.

> This should be solved in environment before cryptsetup starts - set proper
> keymap (even during boot). IIRC Fedora support setting keymap in boot environment now,
> there were similar bug reports already:-)

I agree, but that only solves part of the problem. Nowaday users
apparently are encouraged to use desktop apps to change the keymap
for their session (no system wide xorg.conf anymore). There's no
relation between console keymap and X keymap. So if one uses
cryptsetup in an xterm to enter umlauts there's no way to guarantee
that the password will work in initrd, even if initrd supports
keymaps.

I suppose in all distros the console and X keymaps are the same
after installation as the installer sets a proper default so one
could argue that the above situation is unlikely. However the guy
who complained to me is pre-installing systems for his customers. So
there's a chance that he uses a different keyboard layout for
installation than his customer later. I've convinced him to tell his
customers about the problem but I still think it's a mean trap.
Calling people morons because they didn't read the manpage
thoroughly worked when encrypted root was a feature for freaks but
now it's only a matter of enabling a checkbox.

> Also I expect that in future libcryptsetup will be used more (instead
> of wrapper over cryptsetup binary and the whole internal code
> for reading password will be moved to caller application which is then responsible
> for password from terminal reading stuff.

Frontends will have to implement the same warning then.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\   
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg)

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Milan Broz]

On 11/03/2009 11:08 AM, Ludwig Nussel wrote:
>> I don't think this in good idea. Information that user entered non-ASCII
>> character is useful for attacker, why display it to terminal?
>> (That problem exist in all password entry dialogs - why cryptsetup should
>> be special here?)
> 
> It's only displayed if you add a new password. I doubt you usually
> do that if someone's looking at your screen. He could just look at
> your keyboard while you are typing as well then, right? :-)

No. Output can be logged (installer debug output for example).
If the warning is not there, I know that user have only ascii chars
in password.

Milan
--
mbroz@redhat.com

Re: [dm-crypt] [PATCH] print warning when adding a key that contains non-ASCII characters

[Pasi Kärkkäinen]

On Thu, Oct 22, 2009 at 04:16:08PM +0200, Ludwig Nussel wrote:
> Hi,
> 
> There's no way to determine whether e.g. the keymap on the console is
> the same as in X. Ie a key with umlauts added in an xterm may not be
> usable during boot. So when using e.g. an encrypted root partition
> users could lock themselves out. So I wonder whether a patch like
> the following would be acceptable?
> 

Hmm.. at least rhel5/centos5 version of cryptsetup fails to use
multiline keyfiles.. wondering if this also affects those? 

ie. if the keyfile has multiple lines, it'll only use the first line
(until newline).

-- Pasi

> cu
> Ludwig
> 
> ---
>  lib/utils.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/utils.c b/lib/utils.c
> index d23619e..df9c9a5 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -3,6 +3,7 @@
>  #include <stdlib.h>
>  #include <stddef.h>
>  #include <stdarg.h>
> +#include <ctype.h>
>  #include <errno.h>
>  #include <linux/fs.h>
>  #include <sys/types.h>
> @@ -378,6 +379,20 @@ out_err:
>  	return failed;
>  }
>  
> +static void warnifnotascii(struct crypt_device *ctx, const char *str)
> +{
> +	for(;*str; ++str) {
> +		if(!isascii(*str)) {
> +			log_std(ctx,
> +				"* Warning: Entering non-ASCII passwords\n"
> +				"* may not be possible on all systems.\n"
> +				"* Make sure you can unlock the volume in\n"
> +				"* the intended environment!\n");
> +			break;
> +		}
> +	}
> +}
> +
>  /*
>   * Password reading behaviour matrix of get_key
>   * 
> @@ -443,6 +458,7 @@ void get_key(char *prompt, char **key, unsigned int *passLen, int key_size,
>  				goto out_err;
>  			}
>  			memset(pass_verify, 0, sizeof(pass_verify));
> +			warnifnotascii(cd, pass);
>  		}
>  		*passLen = strlen(pass);
>  		*key = pass;
> -- 
> 1.6.4.2
> 
> _______________________________________________
> dm-crypt mailing list
> dm-crypt@saout.de
> http://www.saout.de/mailman/listinfo/dm-crypt