autostep and encrypted root password

[Jeffrey Bastian]

If kickstarting a RHEL 5.y system, and the kickstart file contains
    autostep
    rootpw --iscrypted $1$.....
then the kickstart will stop and ask for the root password.  If using a 
plain text root password, it autosteps through the root password screen 
as expected.

See https://bugzilla.redhat.com/show_bug.cgi?id=471122#c4 for a patch 
for RHEL 5.3 Beta.

Below is a similar patch for anaconda-11.4.1.56-1.

Jeff

--- kickstart.py.ORIG   2008-11-11 15:35:29.000000000 -0600
+++ kickstart.py        2008-11-11 15:39:42.000000000 -0600
@@ -1185,6 +1185,11 @@
          dispatch.skipStep("installtype")
          dispatch.skipStep("bootdisk")

+    # if the root password is already encrypted, we cannot fill in
+    # the password field on the accounts screen, so skip it
+    if flags.autostep and self.id.rootPassword["isCrypted"]:
+        dispatch.skipStep("accounts")
+
      dispatch.skipStep("bootdisk")
      dispatch.skipStep("betanag")
      dispatch.skipStep("regkey")

[Chris Lumens]

> If kickstarting a RHEL 5.y system, and the kickstart file contains
>    autostep
>    rootpw --iscrypted $1$.....
> then the kickstart will stop and ask for the root password.  If using a  
> plain text root password, it autosteps through the root password screen  
> as expected.
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=471122#c4 for a patch  
> for RHEL 5.3 Beta.
>
> Below is a similar patch for anaconda-11.4.1.56-1.

The problem here is that now we've broken the assumption that
autostep/interactive modes display every screen.  Now there's this one
special screen that doesn't get displayed, which is going to lead to
questions about where it went.  Also the main reason for autostep is for
taking screenshots and doing documentation, which means that screen's
going to have to get treated specially.

- Chris

[Jeffrey Bastian]

Chris Lumens wrote:
> The problem here is that now we've broken the assumption that
> autostep/interactive modes display every screen.

So, a better solution would be to set a dummy password, e.g., xxxxxxxx, 
let it take the screenshot with the bullets in the fields, then replace 
it with the encrypted password from the kickstart file?  Let me take a 
stab at that.

Jeff

[Jeffrey Bastian]

Jeffrey Bastian wrote:
> So, a better solution would be to set a dummy password, e.g., xxxxxxxx, 
> let it take the screenshot with the bullets in the fields, then replace 
> it with the encrypted password from the kickstart file?  Let me take a 
> stab at that.

This patch does the above.  What do you think about this approach?  It 
fixed the problem in my limited testing.

--- account_gui.py.ORIG 2008-11-11 01:56:39.000000000 -0600
+++ account_gui.py      2008-11-11 17:28:55.000000000 -0600
@@ -68,8 +68,13 @@
                                          custom_icon="error")
                  passwordError()

-        self.rootPassword["password"] = self.pw.get_text()
-        self.rootPassword["isCrypted"] = False
+        if self.isCrypted:
+            self.rootPassword["password"] = self.cryptedPassword
+            self.rootPassword["isCrypted"] = True
+        else:
+            self.rootPassword["password"] = self.pw.get_text()
+            self.rootPassword["isCrypted"] = False
+
          return None

def setFocus (self, area, data):
@@ -81,6 +86,8 @@
          self.intf = anaconda.intf

self.passwords = {}
+        self.isCrypted = False
+        self.cryptedPassword = ""

box = gtk.VBox ()
          box.set_border_width(5)
@@ -136,8 +143,13 @@
          wrapper.pack_start (self.rootStatus)
          box.pack_start (wrapper, False)

-        if not self.rootPassword["isCrypted"]:
-           self.pw.set_text(self.rootPassword["password"])
-           self.confirm.set_text(self.rootPassword["password"])
+        if self.rootPassword["isCrypted"]:
+            self.isCrypted = True
+            self.cryptedPassword = self.rootPassword["password"]
+            self.pw.set_text("xxxxxxxx")
+            self.confirm.set_text("xxxxxxxx")
+        else:
+            self.pw.set_text(self.rootPassword["password"])
+            self.confirm.set_text(self.rootPassword["password"])

return box

[Jeffrey Bastian]

Jeffrey Bastian wrote:
> Jeffrey Bastian wrote:
> > So, a better solution would be to set a dummy password, e.g., 
> > xxxxxxxx, let it take the screenshot with the bullets in the fields, 
> > then replace it with the encrypted password from the kickstart file?  
> > Let me take a stab at that.
>
> This patch does the above.  What do you think about this approach?  It 
> fixed the problem in my limited testing.

Yesterday's patch was for RHEL 5.3 Beta.  Attached is a patch for git 
master (anaconda-11.4.1.57-1-1-gc6cf36a).  It's almost the same except 
that it skips the weak password (cracklib) check if the kickstart file 
contains a crypted password (since the password it would be checking -- 
"xxxxxxxx" -- is not the real password, and it is very weak!).

I have not tested this patch, but I did test the RHEL 5.3 Beta patch.

Jeff

[Jeffrey Bastian]

I've been looking at the code more closely and found a way to make the 
patch less intrusive and more efficient.

Attached are new patches for both anaconda-11.1.2.155 (RHEL 5.3 Beta) 
(tested) and anaconda-11.4.1.57-1-1-gc6cf36a (not tested).

Jeff

[Chris Lumens]

> I've been looking at the code more closely and found a way to make the  
> patch less intrusive and more efficient.
>
> Attached are new patches for both anaconda-11.1.2.155 (RHEL 5.3 Beta)  
> (tested) and anaconda-11.4.1.57-1-1-gc6cf36a (not tested).

Sorry for not getting back to you sooner on this.
> diff --git a/iw/account_gui.py b/iw/account_gui.py
> index 45f396b..035ebab 100644
> --- a/iw/account_gui.py
> +++ b/iw/account_gui.py
> @@ -59,9 +59,15 @@ class AccountWindow (InstallWindow):
>              lambda w, e: self.handleCapsLockRelease(w, e, self.capslock))
>      # we might have a root password already
>    # 1. if it's not encrypted, just use it
>    # 2. if it is encrypted, set text in password fields to "xxxxxxxx"
>    #    for use in autostep screenshots
>    if not self.rootPassword['isCrypted']:
>        self.pw.set_text(self.rootPassword['password'])
>        self.confirm.set_text(self.rootPassword['password'])
>    else:
>        self.pw.set_text("xxxxxxxx")
>        self.confirm.set_text("xxxxxxxx")
>
>    # make sure pw has the focus when we enter the screen
>    vbox = self.xml.get_widget("account_box")
>
> @@ -100,6 +106,9 @@ class AccountWindow (InstallWindow):
>              self.capslock.set_text("")
>  def getNext (self):
>    # check if we already have a crypted root password from kickstart
>    if self.rootPassword["isCrypted"]: return None
>    pw = self.pw.get_text()
>    confirm = self.confirm.get_text()

I think using the XXXXXXXX trick is really cheesy, but I support it will
work.  At the least, it puts something into the box so the user knows
they can continue without typing anything in.  I suppose that's the best
that can be hoped for here.

Have you gotten a chance to try against rawhide?  If so and this works
for you, I'm okay with taking it.

We can pursue the RHEL5 one separately through bugzilla I guess.

- Chris

[Jeffrey Bastian]

Chris Lumens wrote:
> I think using the XXXXXXXX trick is really cheesy, but I support it will
> work.  At the least, it puts something into the box so the user knows
> they can continue without typing anything in.  I suppose that's the best
> that can be hoped for here.

Agreed, it is cheesy.  The user shouldn't have to worry, though, since 
this is used for kickstart + autostep so it's not interactive.  It just 
puts something in the box in case screenshots are being taken so it 
doesn't look like root has a blank password.
> We can pursue the RHEL5 one separately through bugzilla I guess.

See bug 471122.

Jeff