Bug 818131 - No password check when TUI upgrade.
No password check when TUI upgrade.
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: ovirt-node (Show other bugs)
6.3
x86_64 Linux
high Severity high
: rc
: 6.3
Assigned To: Fabian Deutsch
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-02 06:08 EDT by cshao
Modified: 2016-04-26 09:46 EDT (History)
9 users (show)

See Also:
Fixed In Version: ovirt-node-2.3.0-6.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-07-19 10:12:49 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description cshao 2012-05-02 06:08:01 EDT
Description of problem:
No password check when TUI upgrade.

Version-Release number of selected component (if applicable):
rhev-hypervisor6-6.3-20120426.2.el6

How reproducible:
100%

Steps to Reproduce:
1. Install RHEV-H first.
2. Boot from PXE choose the latest RHEV-H and press Enter key enter TUI upgrade process.
3. Keep current password and new password to blank, press continue button.
  
Actual results:
1. No password check, upgrade can successful.
2. After reboot, login RHEV-H only need to provide the username(admin), and no password is required.

Expected results:
RHEV-H can do password check when TUI upgrade.

Additional info:
Comment 2 Fabian Deutsch 2012-05-03 14:31:01 EDT
If i see it correctly we could add a password confirmation page before the upgrade process can be started?
Comment 4 Fabian Deutsch 2012-05-04 07:34:05 EDT
The following patch adds the same password check which can be found during the
installation to the upgrade flow.

http://gerrit.ovirt.org/4137
Comment 5 Mike Burns 2012-05-04 07:47:32 EDT
I think we should change the flow a bit here:

if admin password is not set:
  upgrade
else:
  if not existing password matches:
    fail (good message, stay on page, etc)
  else:
    if len(new_password) == 0:
      don't change password, but run upgrade
    else
      if validate_new_password:
        upgrade, change password
      else
        fail with good message, stay on current page


Thoughts?
Comment 6 Fabian Deutsch 2012-05-04 07:53:26 EDT
(In reply to comment #5)
> I think we should change the flow a bit here:
> 
> if admin password is not set:
>   upgrade
> else:
>   if not existing password matches:
>     fail (good message, stay on page, etc)
>   else:
>     if len(new_password) == 0:
>       don't change password, but run upgrade

Wasn't this one the reason of the bug? To prevent using an empty password?
Or was it just checking the password if one is given.
In general I'd apply the same rules which are used when we are doing a fresh install.
Thus not allowing an empty password.

>     else
>       if validate_new_password:
>         upgrade, change password
>       else
>         fail with good message, stay on current page
Comment 7 Mike Burns 2012-05-04 08:14:14 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > I think we should change the flow a bit here:
> > 
> > if admin password is not set:
> >   upgrade
> > else:
> >   if not existing password matches:
> >     fail (good message, stay on page, etc)
> >   else:
> >     if len(new_password) == 0:
> >       don't change password, but run upgrade
> 
> Wasn't this one the reason of the bug? To prevent using an empty password?
> Or was it just checking the password if one is given.
> In general I'd apply the same rules which are used when we are doing a fresh
> install.
> Thus not allowing an empty password.

We need to validate that the existing password matches, but if new password isn't passed, we should keep the old password around.  Currently, it will wipe the current password.  

Example:

current password = password
user enters:

current_password:  password
new password:
confirm new password:

Right now, with no patch, when you login next time, admin will have no password set and you can login with just the username

With ^^, the password will still be "password"


From implementation point of view, it could be that we do:

if length(new_password) == 0 and len(new_password_confirm) == 0:
  new_password = old_password
  new_password_confirm = old_password
Comment 8 Fabian Deutsch 2012-05-04 08:18:59 EDT
(In reply to comment #7)
> We need to validate that the existing password matches, but if new password
> isn't passed, we should keep the old password around.  Currently, it will wipe
> the current password.  
> 
> Example:
> 
> current password = password
> user enters:
> 
> current_password:  password
> new password:
> confirm new password:
> 
> Right now, with no patch, when you login next time, admin will have no password
> set and you can login with just the username
> 
> With ^^, the password will still be "password"

Right. That sounds reasonable. But we should at least tell the use that we are (re-)using the current password if no password is given.

> From implementation point of view, it could be that we do:
> 
> if length(new_password) == 0 and len(new_password_confirm) == 0:
>   new_password = old_password
>   new_password_confirm = old_password
Comment 9 Mike Burns 2012-05-04 08:21:22 EDT
(In reply to comment #8)

> Right. That sounds reasonable. But we should at least tell the use that we are
> (re-)using the current password if no password is given.
> 

Sure, we can alter the text on that page.  Something along the lines of:

Please enter the current admin password.  You can also change the password here, if you wish.  If you leave the new password fields blank, the admin password will remain the same.
Comment 10 Fabian Deutsch 2012-05-07 06:41:41 EDT
This patch introduces the required changes:
http://gerrit.ovirt.org/4137

The password is checked when the user enters a new password.

The current admin password (of the installed node) will remain the same, if the new-password field is left blank. There is also a feedback given to the user in this case.
Comment 11 Fabian Deutsch 2012-05-07 07:07:00 EDT
It would be nice if a native speaker could revise the texts.
Comment 14 yuanquan chen 2012-05-15 00:03:44 EDT
Verified with rhevh-6.3-20120509.1. The password check works. If you leave the  current password and new password blank, it will prompt that "You have not provide a new password, current admin password will be used", the updated rhev-h will use the original admin password.
Comment 16 errata-xmlrpc 2012-07-19 10:12:49 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2012-0741.html

Note You need to log in before you can comment on or make changes to this bug.