bug#35540: Installer displays encrypted partition password entry in cleartext

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

When creating an encrypted partition in Manual partitioning (maybe
also Guided?) in the Newt installer, it asks for a password with which
to encrypt the partition.  However only the password confirmation
password entry diplays ******* instead of the typed password, the
password entry before displays the password in cleartext.

bug#35540: Installer displays encrypted partition password entry in cleartext

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

Hi,

On Fri, 3 May 2019 10:54:37 +0200
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:

> When creating an encrypted partition in Manual partitioning (maybe
> also Guided?) in the Newt installer, it asks for a password with which
> to encrypt the partition.  However only the password confirmation
> password entry diplays ******* instead of the typed password, the
> password entry before displays the password in cleartext.

Yes.  What about it is a bug?  It would be very bad if you had a typo
in the partition encryption password, so it's good that it's visible.

If you want, we can make the password visible in both boxes.
But we shouldn't make it invisible in both boxes.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35540: Installer displays encrypted partition password entry in cleartext

[Julien Lepiller]

Le Fri, 3 May 2019 11:30:18 +0200,
Danny Milosavljevic <dannym@scratchpost.org> a écrit :

> Hi,
> 
> On Fri, 3 May 2019 10:54:37 +0200
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
> 
> > When creating an encrypted partition in Manual partitioning (maybe
> > also Guided?) in the Newt installer, it asks for a password with
> > which to encrypt the partition.  However only the password
> > confirmation password entry diplays ******* instead of the typed
> > password, the password entry before displays the password in
> > cleartext.  
> 
> Yes.  What about it is a bug?  It would be very bad if you had a typo
> in the partition encryption password, so it's good that it's visible.
> 
> If you want, we can make the password visible in both boxes.
> But we shouldn't make it invisible in both boxes.

The role of the confirmation is to make sure you didn't make a typo
somewhere. If it's visible from the start, you know you didn't make a
typo, so the confirmation is useless, no?

On android when you enter a wifi password for instance, it's invisible
when you type it (replace by *) by there is a small button on the side
of the text entry to allow you to see it in plaintext to check that you
didn't make a mistake. Could we implement that instead? There won't be
a need for a confirmation either in that case.

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

Julien Lepiller <julien@lepiller.eu> skribis:

> Le Fri, 3 May 2019 11:30:18 +0200,
> Danny Milosavljevic <dannym@scratchpost.org> a écrit :
>
>> Hi,
>> 
>> On Fri, 3 May 2019 10:54:37 +0200
>> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> wrote:
>> 
>> > When creating an encrypted partition in Manual partitioning (maybe
>> > also Guided?) in the Newt installer, it asks for a password with
>> > which to encrypt the partition.  However only the password
>> > confirmation password entry diplays ******* instead of the typed
>> > password, the password entry before displays the password in
>> > cleartext.  
>> 
>> Yes.  What about it is a bug?  It would be very bad if you had a typo
>> in the partition encryption password, so it's good that it's visible.
>> 
>> If you want, we can make the password visible in both boxes.
>> But we shouldn't make it invisible in both boxes.
>
> The role of the confirmation is to make sure you didn't make a typo
> somewhere.

But that’s a different thing.  Suppose you type a passphrase assuming
you have a Dvorak keyboard but it’s actually QWERTY.  You’ll get the
confirmation right.

Then when you boot, if for some reason you get the wrong keyboard
layout, you’re screwed.

That’s why I think that seeing what you actually type is useful.

Other options include:

  1. Hiding the passphrase, but display right above it something like:

     Keyboard layout: <layout name>

  2. Adding a checkbox to toggle password visibility.

#1 is probably not great because it doesn’t help if you don’t know
precisely the layout.

#2 would be nice; not sure how to do it, though.

Ludo’.

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

On Fri, May 03, 2019 at 03:50:52PM +0200, Ludovic Courtès wrote:
> That’s why I think that seeing what you actually type is useful.
> 

Seeing the password is useful, unless someone is shoulder surfing
while you install, which is possible.


> Other options include:
> 
>   1. Hiding the passphrase, but display right above it something like:
> 
>      Keyboard layout: <layout name>
> 
>   2. Adding a checkbox to toggle password visibility.
> 
> #1 is probably not great because it doesn’t help if you don’t know
> precisely the layout.
> 
> #2 would be nice; not sure how to do it, though.
> 
> Ludo’.
> 

#2 would please everybody, but I do not know what widgets Newt
provides for this.  Mathieu, would you know if changing the
visibility with e.g. a checkbox is doable?

Regards,
Florian

bug#35540: Installer displays encrypted partition password entry in cleartext

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]


Hello,

> #2 would please everybody, but I do not know what widgets Newt
> provides for this.  Mathieu, would you know if changing the
> visibility with e.g. a checkbox is doable?

You'll find a patch attached that adds a checkbox to toggle password
hiding. Every password input now has such a checkbox, WDYT?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-installer-Add-password-hide-checkbox.patch --]
[-- Type: text/x-diff, Size: 8017 bytes --]

From fba1d82b2e27917a5efef339b326fe2d98e62bc0 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Sun, 5 May 2019 12:54:40 +0200
Subject: [PATCH] installer: Add password 'hide' checkbox.

* gnu/installer/newt/page.scm (run-input-page)[input-hide-checkbox?]: New
parameter adding a checkbox to toggle password hiding. By default, the
checkbox is active and the password is hence hided.
* gnu/installer/newt/partition.scm (prompt-luks-passwords): Enable the
previous parameter on both password input pages.
* gnu/installer/newt/user.scm (run-root-password-page): Enable the previous
parameter,
(confirm-password): ditto,
(run-user-add-page): add a checkbox to toggle password hiding.
---
 gnu/installer/newt/page.scm      | 34 +++++++++++++++++++++++---------
 gnu/installer/newt/partition.scm |  9 +++------
 gnu/installer/newt/user.scm      | 22 ++++++++++++++++++---
 3 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/gnu/installer/newt/page.scm b/gnu/installer/newt/page.scm
index 8a32c403df..4645486ff7 100644
--- a/gnu/installer/newt/page.scm
+++ b/gnu/installer/newt/page.scm
@@ -75,6 +75,7 @@ this page to TITLE."
                          #:key
                          (allow-empty-input? #f)
                          (default-text #f)
+                         (input-hide-checkbox? #f)
                          (input-field-width 40)
                          (input-flags 0))
   "Run a page to prompt user for an input. The given TEXT will be displayed
@@ -86,22 +87,37 @@ input box, such as FLAG-PASSWORD."
           (make-reflowed-textbox -1 -1 text
                                  input-field-width
                                  #:flags FLAG-BORDER))
-         (grid (make-grid 1 3))
+         (input-visible-cb
+          (make-checkbox -1 -1 (G_ "Hide") #\x "x "))
+         (input-flags* (if input-hide-checkbox?
+                           (logior FLAG-PASSWORD input-flags)
+                           input-flags))
          (input-entry (make-entry -1 -1 20
-                                  #:flags input-flags))
+                                  #:flags input-flags*))
          (ok-button (make-button -1 -1 (G_ "OK")))
+         (grid (vertically-stacked-grid
+                GRID-ELEMENT-COMPONENT text-box
+                GRID-ELEMENT-SUBGRID
+                (apply
+                 horizontal-stacked-grid
+                 GRID-ELEMENT-COMPONENT input-entry
+                 `(,@(if input-hide-checkbox?
+                         (list GRID-ELEMENT-COMPONENT input-visible-cb)
+                         '())))
+                GRID-ELEMENT-COMPONENT ok-button))
          (form (make-form)))
 
+    (add-component-callback
+     input-visible-cb
+     (lambda (component)
+       (set-entry-flags input-entry
+                        FLAG-PASSWORD
+                        FLAG-ROLE-TOGGLE)))
+
     (when default-text
       (set-entry-text input-entry default-text))
 
-    (set-grid-field grid 0 0 GRID-ELEMENT-COMPONENT text-box)
-    (set-grid-field grid 0 1 GRID-ELEMENT-COMPONENT input-entry
-                    #:pad-top 1)
-    (set-grid-field grid 0 2 GRID-ELEMENT-COMPONENT ok-button
-                    #:pad-top 1)
-
-    (add-components-to-form form text-box input-entry ok-button)
+    (add-form-to-grid grid form #t)
     (make-wrapped-grid-window grid title)
     (let ((error-page (lambda ()
                         (run-error-page (G_ "Please enter a non empty input.")
diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scm
index 3fb6c5079e..ecf241b8e2 100644
--- a/gnu/installer/newt/partition.scm
+++ b/gnu/installer/newt/partition.scm
@@ -153,21 +153,18 @@ USER-PARTITIONS list. Return this list with password fields filled-in."
                 (file-name (user-partition-file-name user-part))
                 (password-page
                  (lambda ()
-                   ;; Note: Don't use FLAG-PASSWORD here because this is the
-                   ;; first bit of text that the user types in, so it's
-                   ;; probably safer if they can see that the keyboard layout
-                   ;; they chose is in effect.
                    (run-input-page
                     (format #f (G_ "Please enter the password for the \
 encryption of partition ~a (label: ~a).") file-name crypt-label)
-                    (G_ "Password required"))))
+                    (G_ "Password required")
+                    #:input-hide-checkbox? #t)))
                 (password-confirm-page
                  (lambda ()
                    (run-input-page
                     (format #f (G_ "Please confirm the password for the \
 encryption of partition ~a (label: ~a).") file-name crypt-label)
                     (G_ "Password confirmation required")
-                    #:input-flags FLAG-PASSWORD))))
+                    #:input-hide-checkbox? #t))))
            (if crypt-label
                (let loop ()
                  (let ((password (password-page))
diff --git a/gnu/installer/newt/user.scm b/gnu/installer/newt/user.scm
index deab056e0c..6aa103aa5a 100644
--- a/gnu/installer/newt/user.scm
+++ b/gnu/installer/newt/user.scm
@@ -51,9 +51,11 @@ REAL-NAME, and HOME-DIRECTORY as the initial values in the form."
                                       #:initial-value real-name))
          (entry-home-directory (make-entry -1 -1 entry-width
                                            #:initial-value home-directory))
+         (password-visible-cb
+          (make-checkbox -1 -1 (G_ "Hide") #\x "x "))
          (entry-password (make-entry -1 -1 entry-width
                                      #:flags FLAG-PASSWORD))
-         (entry-grid (make-grid 2 5))
+         (entry-grid (make-grid 3 5))
          (button-grid (make-grid 1 1))
          (ok-button (make-button -1 -1 (G_ "OK")))
          (grid (make-grid 1 2))
@@ -71,6 +73,12 @@ REAL-NAME, and HOME-DIRECTORY as the initial values in the form."
     (set-entry-grid-field 0 3 label-password)
     (set-entry-grid-field 1 3 entry-password)
 
+    (set-grid-field entry-grid
+                    2 3
+                    GRID-ELEMENT-COMPONENT
+                    password-visible-cb
+                    #:pad-left 1)
+
     (set-grid-field button-grid 0 0 GRID-ELEMENT-COMPONENT ok-button)
 
     (add-component-callback
@@ -83,11 +91,19 @@ REAL-NAME, and HOME-DIRECTORY as the initial values in the form."
          (set-entry-text entry-real-name
                          (string-titlecase (entry-value entry-name))))))
 
+    (add-component-callback
+     password-visible-cb
+     (lambda (component)
+       (set-entry-flags entry-password
+                        FLAG-PASSWORD
+                        FLAG-ROLE-TOGGLE)))
+
     (add-components-to-form form
                             label-name label-real-name
                             label-home-directory label-password
                             entry-name entry-real-name
                             entry-home-directory entry-password
+                            password-visible-cb
                             ok-button)
 
     (make-wrapped-grid-window (vertically-stacked-grid
@@ -136,7 +152,7 @@ a thunk, if the confirmation doesn't match PASSWORD, and return its result."
     (run-input-page (G_ "Please confirm the password.")
                     (G_ "Password confirmation required")
                     #:allow-empty-input? #t
-                    #:input-flags FLAG-PASSWORD))
+                    #:input-hide-checkbox? #t))
 
   (if (string=? password confirmation)
       password
@@ -153,7 +169,7 @@ a thunk, if the confirmation doesn't match PASSWORD, and return its result."
     (run-input-page (G_ "Please choose a password for the system \
 administrator (\"root\").")
                     (G_ "System administrator password")
-                    #:input-flags FLAG-PASSWORD))
+                    #:input-hide-checkbox? #t))
 
   (confirm-password password run-root-password-page))
 
-- 
2.17.1

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

On Sun, May 05, 2019 at 01:04:03PM +0200, Mathieu Othacehe wrote:
> 
> Hello,
> 
> > #2 would please everybody, but I do not know what widgets Newt
> > provides for this.  Mathieu, would you know if changing the
> > visibility with e.g. a checkbox is doable?
> 
> You'll find a patch attached that adds a checkbox to toggle password
> hiding. Every password input now has such a checkbox, WDYT?
> 
> Thanks,
> 
> Mathieu


This looks great and appears to work fine (I did not finish the
installation due to the wpa-supplicant bug).  “Hide” of course has no
translations yet.

Regards,
Florian

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

Hello,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> #2 would please everybody, but I do not know what widgets Newt
>> provides for this.  Mathieu, would you know if changing the
>> visibility with e.g. a checkbox is doable?
>
> You'll find a patch attached that adds a checkbox to toggle password
> hiding. Every password input now has such a checkbox, WDYT?

It looks great!

I would perhaps add that checkbox only for the passphrase, in part
because when I test an install I prefer to have fewer keystrokes :-),
but also because this might clutter dialog boxes and cause troubles:
what if the translation of “Hide” is takes up more space? is it still
going to fit?

WDYT?

Thank you!

Ludo’.

bug#35540: Installer displays encrypted partition password entry in cleartext

[Mathieu Othacehe]

Hey Ludo,

> I would perhaps add that checkbox only for the passphrase, in part
> because when I test an install I prefer to have fewer keystrokes :-),

The parameter is disabled by default and enabled only for disk
encryption and root/user passwords so I'm not sure I get what you mean.

> but also because this might clutter dialog boxes and cause troubles:
> what if the translation of “Hide” is takes up more space? is it still
> going to fit?

Hard to say :p, the checkbox is added to widget grids which should adapt
to terminal size. So unless the translation is really long and the
screen really small, I hope it fits!

Mathieu

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> I would perhaps add that checkbox only for the passphrase, in part
>> because when I test an install I prefer to have fewer keystrokes :-),
>
> The parameter is disabled by default and enabled only for disk
> encryption and root/user passwords so I'm not sure I get what you mean.

I mean that when I do a test install, I essentially hit RET RET RET RET,
and sometimes TAB ludo foo, and this change adds an additional TAB into
the mix; but nevermind, my muscle memory will get used to it.  :-)

>> but also because this might clutter dialog boxes and cause troubles:
>> what if the translation of “Hide” is takes up more space? is it still
>> going to fit?
>
> Hard to say :p, the checkbox is added to widget grids which should adapt
> to terminal size. So unless the translation is really long and the
> screen really small, I hope it fits!

Isn’t the grid constrained by the side of the outer box?  Perhaps we
should enlarge that outer box a bit?

Anyway, I think you can push, it’s already a great improvement IMO.

Thanks,
Ludo’.

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

Sorry for being late to the feedback party.

On Mon, May 06, 2019 at 12:02:16PM +0200, Ludovic Courtès wrote:
> I would perhaps add that checkbox only for the passphrase, in part

I disagree, people would expect it (and file bugs) for other passwords
for the same reasons (other people watching while you install Guix
vs. wanting to visually confirm you have not mistyped etc.)


> because when I test an install I prefer to have fewer keystrokes :-),
> but also because this might clutter dialog boxes and cause troubles:
> what if the translation of “Hide” is takes up more space? is it still
> going to fit?
>

German “Verbergen” is somewhat longer than “Hide”; I tried translating
it locally and it still fits perfectly on my low res AMD GPU screen.

When creating a user account, the translation for Home Directory
(“Persönliches Verzeichnis”) gets cut off to “Persönliches Verzeic”,
but I believe this is not important.

Regards,
Florian

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

On Mon, May 06, 2019 at 08:14:46PM +0200, pelzflorian (Florian Pelz) wrote:
> When creating a user account, the translation for Home Directory
> (“Persönliches Verzeichnis”) gets cut off to “Persönliches Verzeic”,
> but I believe this is not important.
> 

This cut-off happens even on higher resolution screens.  Hmm well it
would be nicer if the space for the Home Directory were 4 letters
wider, but it is not all that important.

Regards,
Florian

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> On Mon, May 06, 2019 at 08:14:46PM +0200, pelzflorian (Florian Pelz) wrote:
>> When creating a user account, the translation for Home Directory
>> (“Persönliches Verzeichnis”) gets cut off to “Persönliches Verzeic”,
>> but I believe this is not important.
>> 
>
> This cut-off happens even on higher resolution screens.  Hmm well it
> would be nicer if the space for the Home Directory were 4 letters
> wider, but it is not all that important.

I agree.  I’ll commit the patch below, which works fine for German (at
least before the “Hide” checkbox patch).

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 970 bytes --]

diff --git a/gnu/installer/newt/user.scm b/gnu/installer/newt/user.scm
index deab056e0c..ac07d26c8b 100644
--- a/gnu/installer/newt/user.scm
+++ b/gnu/installer/newt/user.scm
@@ -34,7 +34,7 @@
   "Run a form to enter the user name, home directory, and password.  Use NAME,
 REAL-NAME, and HOME-DIRECTORY as the initial values in the form."
   (define (pad-label label)
-    (string-pad-right label 20))
+    (string-pad-right label 25))
 
   (let* ((label-name
           (make-label -1 -1 (pad-label (G_ "Name"))))
@@ -44,7 +44,7 @@ REAL-NAME, and HOME-DIRECTORY as the initial values in the form."
           (make-label -1 -1 (pad-label (G_ "Home directory"))))
          (label-password
           (make-label -1 -1 (pad-label (G_ "Password"))))
-         (entry-width 30)
+         (entry-width 35)
          (entry-name (make-entry -1 -1 entry-width
                                  #:initial-value name))
          (entry-real-name (make-entry -1 -1 entry-width

bug#35540: Installer displays encrypted partition password entry in cleartext

[pelzflorian (Florian Pelz)]

On Mon, May 06, 2019 at 09:43:55PM +0200, Ludovic Courtès wrote:
>> I agree.  I’ll commit the patch below, which works fine for German (at
> least before the “Hide” checkbox patch).
> 
> Thanks,
> Ludo’.
> 


Fits perfectly.  Thank you all of you!

Regards,
Florian

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> On Mon, May 06, 2019 at 12:02:16PM +0200, Ludovic Courtès wrote:
>> I would perhaps add that checkbox only for the passphrase, in part
>
> I disagree, people would expect it (and file bugs) for other passwords
> for the same reasons (other people watching while you install Guix
> vs. wanting to visually confirm you have not mistyped etc.)

Yeah OK, that makes sense to me.

>> because when I test an install I prefer to have fewer keystrokes :-),
>> but also because this might clutter dialog boxes and cause troubles:
>> what if the translation of “Hide” is takes up more space? is it still
>> going to fit?
>>
>
> German “Verbergen” is somewhat longer than “Hide”; I tried translating
> it locally and it still fits perfectly on my low res AMD GPU screen.

Perfect.

So I guess you can go ahead and push, Mathieu!

Thank you,
Ludo’.

bug#35540: Installer displays encrypted partition password entry in cleartext

[Mathieu Othacehe]

Hey,

> So I guess you can go ahead and push, Mathieu!

Pushed as 445bd4d5e5, thanks to both of you for reviewing it :)

Mathieu

bug#35540: Installer displays encrypted partition password entry in cleartext

[Ludovic Courtès]

Hi,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> When creating an encrypted partition in Manual partitioning (maybe
> also Guided?) in the Newt installer, it asks for a password with which
> to encrypt the partition.  However only the password confirmation
> password entry diplays ******* instead of the typed password, the
> password entry before displays the password in cleartext.

This is done on purpose as I wrote in commit
453c976501bb4d5c4c6b832b7c0c1ec3d493b80f:

         ;; Note: Don't use FLAG-PASSWORD here because this is the
         ;; first bit of text that the user types in, so it's
         ;; probably safer if they can see that the keyboard layout
         ;; they chose is in effect.

I’m not entirely sure this is the right thing to do, but I thought that
as a user I’d want to make sure I really typed what I thought I typed.

WDYT?

Ludo’.