guests/lcitool | 29 +------------------------ guests/playbooks/update/tasks/users.yml | 2 +- 2 files changed, 2 insertions(+), 29 deletions(-)
Since version 1.9 ansible supports password_hash filter that can do that for us.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
guests/lcitool | 29 +------------------------
guests/playbooks/update/tasks/users.yml | 2 +-
2 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 2901a92c507b..ad1eee288620 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -151,34 +151,7 @@ class Config:
return vault_pass_file
def get_root_password_file(self):
- root_pass_file = self._get_config_file("root-password")
- root_hash_file = self._get_config_file(".root-password.hash")
-
- try:
- with open(root_pass_file, "r") as infile:
- root_pass = infile.readline().strip()
- except Exception:
- raise Error(
- "Missing or invalid root password file ({})".format(
- root_pass_file,
- )
- )
-
- # The hash will be different every time we run, but that doesn't
- # matter - it will still validate the correct root password
- root_hash = crypt.crypt(root_pass, Util.mksalt())
-
- try:
- with open(root_hash_file, "w") as infile:
- infile.write("{}\n".format(root_hash))
- except Exception:
- raise Error(
- "Can't write hashed root password file ({})".format(
- root_hash_file,
- )
- )
-
- return root_hash_file
+ return self._get_config_file("root-password")
class Inventory:
diff --git a/guests/playbooks/update/tasks/users.yml b/guests/playbooks/update/tasks/users.yml
index ec7f798a9c00..0a930d6c382c 100644
--- a/guests/playbooks/update/tasks/users.yml
+++ b/guests/playbooks/update/tasks/users.yml
@@ -2,7 +2,7 @@
- name: 'root: Set password'
user:
name: root
- password: '{{ lookup("file", root_password_file) }}'
+ password: '{{ lookup("file", root_password_file)|password_hash("sha512") }}'
shell: '{{ bash }}'
- name: 'root: Configure ssh access'
--
2.18.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote: s/manually/ourselves/ in the subject. [...] > def get_root_password_file(self): > - root_pass_file = self._get_config_file("root-password") > - root_hash_file = self._get_config_file(".root-password.hash") > - > - try: > - with open(root_pass_file, "r") as infile: > - root_pass = infile.readline().strip() > - except Exception: > - raise Error( > - "Missing or invalid root password file ({})".format( > - root_pass_file, > - ) > - ) > - > - # The hash will be different every time we run, but that doesn't > - # matter - it will still validate the correct root password > - root_hash = crypt.crypt(root_pass, Util.mksalt()) > - > - try: > - with open(root_hash_file, "w") as infile: > - infile.write("{}\n".format(root_hash)) > - except Exception: > - raise Error( > - "Can't write hashed root password file ({})".format( > - root_hash_file, > - ) > - ) > - > - return root_hash_file > + return self._get_config_file("root-password") This is a really nice improvement overall, but we can't quite get rid of the entire function: we still need to try and open the file, or at least stat() it, like we do in get_vault_password_file(), so that we can error out early instead of having Ansible bail out on us really late in the game. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 04, 2018 at 01:53:45PM +0200, Andrea Bolognani wrote: >On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote: > >s/manually/ourselves/ in the subject. > >[...] >> def get_root_password_file(self): >> - root_pass_file = self._get_config_file("root-password") >> - root_hash_file = self._get_config_file(".root-password.hash") >> - >> - try: >> - with open(root_pass_file, "r") as infile: >> - root_pass = infile.readline().strip() >> - except Exception: >> - raise Error( >> - "Missing or invalid root password file ({})".format( >> - root_pass_file, >> - ) >> - ) >> - >> - # The hash will be different every time we run, but that doesn't >> - # matter - it will still validate the correct root password >> - root_hash = crypt.crypt(root_pass, Util.mksalt()) >> - >> - try: >> - with open(root_hash_file, "w") as infile: >> - infile.write("{}\n".format(root_hash)) >> - except Exception: >> - raise Error( >> - "Can't write hashed root password file ({})".format( >> - root_hash_file, >> - ) >> - ) >> - >> - return root_hash_file >> + return self._get_config_file("root-password") > >This is a really nice improvement overall, but we can't quite get >rid of the entire function: we still need to try and open the file, >or at least stat() it, like we do in get_vault_password_file(), so >that we can error out early instead of having Ansible bail out on >us really late in the game. > So what you had in mind is something like the following squashed in? diff --git i/guests/lcitool w/guests/lcitool index 609c73c43dbc..2ac98ea69030 100755 --- i/guests/lcitool +++ w/guests/lcitool @@ -151,7 +151,22 @@ class Config: return vault_pass_file def get_root_password_file(self): - return self._get_config_file("root-password") + root_pass_file = None + + root_pass_file = self._get_config_file("root-password") + + try: + with open(root_pass_file, "r") as infile: + if not infile.readline().strip(): + raise ValueError + except Exception: + raise Error( + "Missing or invalid root password file ({})".format( + root_pass_file, + ) + ) + + return root_pass_file class Inventory: -- Or we could have the check in ansible itself, but that would be a bigger change and the codebase is not prepared for that. TLTTIRN, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2018-09-05 at 11:39 +0200, Martin Kletzander wrote: > On Tue, Sep 04, 2018 at 01:53:45PM +0200, Andrea Bolognani wrote: > > On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote: > > > > s/manually/ourselves/ in the subject. [...] > > This is a really nice improvement overall, but we can't quite get > > rid of the entire function: we still need to try and open the file, > > or at least stat() it, like we do in get_vault_password_file(), so > > that we can error out early instead of having Ansible bail out on > > us really late in the game. > > So what you had in mind is something like the following squashed in? Pretty much exactly, yes :) [...] > def get_root_password_file(self): > - return self._get_config_file("root-password") > + root_pass_file = None > + These two lines can be avoided. > + root_pass_file = self._get_config_file("root-password") > + > + try: > + with open(root_pass_file, "r") as infile: > + if not infile.readline().strip(): > + raise ValueError This introduces a *slight* semantic change, in that we won't accept an empty root password anymore. I'd argue it's probably for the best anyway, so let's just go ahead with it. > Or we could have the check in ansible itself, but that would be a bigger change > and the codebase is not prepared for that. It shouldn't be *too* hard, and we could also take the opportunity to fix the long-standing issue of 'update' not working correctly if ~/.ssh/id_rsa.pub doesn't exist. But we can worry about that another day; in the meantime, if you fix the two nits pointed out above you can have my Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.