3:49 a.m.
Since version 1.9 ansible supports password_hash filter that can do that for us.
Signed-off-by: Martin Kletzander <mkletzan(a)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
6:53 a.m.
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
4:39 a.m.
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
8:38 a.m.
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2306
days inactive
2307
days old
3 comments
2 participants
participants (2)
-
Andrea Bolognani -
Martin Kletzander