random: expose hashed passwords in recipe options

Directly expose all passlib.hash supported hashes, using a `passwd-`
prefix. For example, to access `sha256_crypt`, use `passwd-sha256-crypt`
option name.

  [secret]
  recipe = slapos.cookbook:generate.password

  [config-file]
  hashed-password = ${secret:passwd-sha256-crypt}

This changes the format of storage-path, it used to be the password in
plain text, it is now a mapping also containing hashed passwords, to
have the same hashed passwords for each buildout run.

This needs collaboration from publish_early recipe, because .pop(k) does
raised a KeyError with the dict.__missing__  approach.

setup.py

@@ -72,6 +72,8 @@ setup(name=name,
         'zc.buildout', # plays with buildout
         'zc.recipe.egg', # for scripts generation
         'pytz', # for timezone database
+        'passlib',
+        'bcrypt',
         ],
       zip_safe=True,
       entry_points={

slapos/recipe/publish_early.py

@@ -131,7 +131,9 @@ class Recipe(GenericSlapRecipe):
           new = {}
           for k, v in six.iteritems(init):
             try:
-              options[k] = publish_dict[k] = new[v] = init_section.pop(v)
+              init_section_value = init_section[v]
+              options[k] = publish_dict[k] = new[v] = init_section_value
+              del init_section[v]
             except KeyError:
               pass
           if new != override:

slapos/recipe/random.py

@@ -33,12 +33,16 @@ buildout Software Releases and Instances developments.
 from __future__ import absolute_import
 
 import errno
+import json
 import os
 import random
 import string
+import sys
 from .librecipe import GenericBaseRecipe
 from .publish_early import volatileOptions
-from slapos.util import str2bytes
+
+import passlib.hash
+
 
 class Integer(object):
   """
@@ -113,7 +117,7 @@ def generatePassword(length):
 
 
 class Password(object):
-  """Generate a password that is only composed of lowercase letters
+  """Generate a password.
 
     This recipe only makes sure that ${:passwd} does not end up in `.installed`
     file, which is world-readable by default. So be careful not to spread it
@@ -128,6 +132,11 @@ class Password(object):
     - create-once: boolean value which set if storage-path won't be modified
                    as soon the file is created with the password (not empty).
       (default: True)
+    - passwd: the generated password. Can also be set, to reuse the password
+              hashing capabilities.
+    - passwd-*: the hashed password, using schemes supported by passlib.
+                for example, passwd-sha256-crypt will expose the password hashed
+                with sha256 crypt algorithm.
 
     If storage-path is empty, the recipe does not save the password, which is
     fine it is saved by other means, e.g. using the publish-early recipe.
@@ -141,24 +150,53 @@ class Password(object):
     except KeyError:
       self.storage_path = options['storage-path'] = os.path.join(
         buildout['buildout']['parts-directory'], name)
-    passwd = options.get('passwd')
-    if not passwd:
+    passwd_dict = {
+      '': options.get('passwd')
+    }
+    if not passwd_dict['']:
       if self.storage_path:
+        self._needs_migration = False
         try:
           with open(self.storage_path) as f:
-            passwd = f.read().strip('\n')
+            content = f.read().strip('\n')
+            # new format: the file contains password and hashes in json format
+            try:
+              passwd_dict = json.loads(content)
+              if sys.version_info < (3, ):
+                passwd_dict = {k: v.encode() for k, v in passwd_dict.items()}
+            except ValueError:
+              # old format: the file only contains the password in plain text
+              passwd_dict[''] = content
+              self._needs_migration = True
         except IOError as e:
           if e.errno != errno.ENOENT:
             raise
-      if not passwd:
-        passwd = self.generatePassword(int(options.get('bytes', '16')))
+
+      if not passwd_dict['']:
+        passwd_dict[''] = self.generatePassword(int(options.get('bytes', '16')))
         self.update = self.install
-      options['passwd'] = passwd
+      options['passwd'] = passwd_dict['']
+
+    class HashedPasswordDict(dict):
+      def __missing__(self, key):
+        if not key.startswith('passwd-'):
+          raise KeyError(key)
+        if key in passwd_dict:
+          return passwd_dict[key]
+        handler = getattr(
+          passlib.hash, key[len('passwd-'):].replace('-', '_'), None)
+        if handler is None:
+          raise KeyError(key)
+        hashed = handler.hash(passwd_dict[''])
+        passwd_dict[key] = hashed
+        return hashed
+    options._data = HashedPasswordDict(options._data)
+
     # Password must not go into .installed file, for 2 reasons:
     # security of course but also to prevent buildout to always reinstall.
     # publish_early already does it, but this recipe may also be used alone.
     volatileOptions(options, ('passwd',))
-    self.passwd = passwd
+    self.passwd_dict = passwd_dict
 
   generatePassword = staticmethod(generatePassword)
 
@@ -167,19 +205,14 @@ class Password(object):
       try:
         # The following 2 lines are just an optimization to avoid recreating
         # the file with the same content.
-        if self.create_once and os.stat(self.storage_path).st_size:
+        if self.create_once and os.stat(self.storage_path).st_size and not self._needs_migration:
           return
         os.unlink(self.storage_path)
       except OSError as e:
         if e.errno != errno.ENOENT:
           raise
-
-      fd = os.open(self.storage_path,
-        os.O_CREAT | os.O_EXCL | os.O_WRONLY | os.O_TRUNC, 0o600)
-      try:
-        os.write(fd, str2bytes(self.passwd))
-      finally:
-        os.close(fd)
+      with open(self.storage_path, 'w') as f:
+        json.dump(self.passwd_dict, f)
       if not self.create_once:
         return self.storage_path
 

slapos/test/recipe/test_random.py

+import json
+import os
+import shutil
+import tempfile
+import unittest
+
+import zc.buildout.testing
+import zc.buildout.buildout
+import passlib.hash
+from slapos.recipe import random
+
+
+class TestPassword(unittest.TestCase):
+  def setUp(self):
+    self.buildout = zc.buildout.testing.Buildout()
+    parts_directory = tempfile.mkdtemp()
+    self.buildout['buildout']['parts-directory'] = parts_directory
+    self.addCleanup(shutil.rmtree, parts_directory)
+
+  def _makeRecipe(self, options, section_name="random"):
+    self.buildout[section_name] = options
+    recipe = random.Password(
+      self.buildout, section_name, self.buildout[section_name]
+    )
+    return recipe
+
+  def test_empty_options(self):
+    recipe = self._makeRecipe({})
+    passwd = self.buildout["random"]["passwd"]
+    self.assertEqual(len(passwd), 16)
+    recipe.install()
+    with open(self.buildout["random"]["storage-path"]) as f:
+      self.assertEqual(json.load(f), {'': passwd})
+
+  def test_storage_path(self):
+    tf = tempfile.NamedTemporaryFile(delete=False)
+    self.addCleanup(os.unlink, tf.name)
+    self._makeRecipe({'storage-path': tf.name}).install()
+    passwd = self.buildout["random"]["passwd"]
+    self.assertEqual(len(passwd), 16)
+    with open(tf.name) as f:
+      self.assertEqual(json.load(f), {'': passwd})
+
+    self._makeRecipe({'storage-path': tf.name}, "another").install()
+    self.assertEqual(self.buildout["another"]["passwd"], passwd)
+
+  def test_storage_path_legacy_format(self):
+    with tempfile.NamedTemporaryFile(delete=False) as tf:
+      tf.write(b'secret\n')
+      tf.flush()
+
+      self._makeRecipe({'storage-path': tf.name}).install()
+      passwd = self.buildout["random"]["passwd"]
+      self.assertEqual(passwd, 'secret')
+      tf.flush()
+      with open(tf.name) as f:
+        self.assertEqual(json.load(f), {'': 'secret'})
+
+    self._makeRecipe({'storage-path': tf.name}, "another").install()
+    self.assertEqual(self.buildout["another"]["passwd"], passwd)
+
+  def test_bytes(self):
+    self._makeRecipe({'bytes': '32'}).install()
+    passwd = self.buildout["random"]["passwd"]
+    self.assertEqual(len(passwd), 32)
+    with open(self.buildout["random"]["storage-path"]) as f:
+      self.assertEqual(json.load(f), {'': passwd})
+
+  def test_volatile(self):
+    self._makeRecipe({})
+    options = self.buildout['random']
+    self.assertIn('passwd', options)
+    options_items = [(k, v) for k, v in options.items() if k != 'passwd']
+    copied_options = options.copy()
+    self.assertEqual(list(copied_options.items()), options_items)
+
+  def test_passlib(self):
+    recipe = self._makeRecipe({})
+
+    hashed = self.buildout['random']['passwd-sha256-crypt']
+    self.assertTrue(
+      passlib.hash.sha256_crypt.verify(
+        self.buildout['random']['passwd'], hashed))
+
+    hashed = self.buildout['random']['passwd-md5-crypt']
+    self.assertTrue(
+      passlib.hash.md5_crypt.verify(
+        self.buildout['random']['passwd'], hashed))
+
+    hashed = self.buildout['random']['passwd-bcrypt']
+    self.assertTrue(
+      passlib.hash.bcrypt.verify(
+        self.buildout['random']['passwd'], hashed))
+
+    hashed = self.buildout['random']['passwd-ldap-salted-sha1']
+    self.assertTrue(
+      passlib.hash.ldap_salted_sha1.verify(
+        self.buildout['random']['passwd'], hashed))
+
+    with self.assertRaises(zc.buildout.buildout.MissingOption):
+      self.buildout['random']['passwd-unknown']
+    with self.assertRaises(zc.buildout.buildout.MissingOption):
+      self.buildout['random']['unknown']
+
+    copied_options = self.buildout['random'].copy()
+    self.assertEqual(list(copied_options.keys()), ['storage-path'])
+
+    recipe.install()
+    # when buildout runs again, the values are read from the storage
+    # and even the hashed values are the same
+    self._makeRecipe({'storage-path': self.buildout['random']['storage-path']}, 'reread')
+    self.assertEqual(
+      self.buildout['reread']['passwd'],
+      self.buildout['random']['passwd'])
+    self.assertEqual(
+      self.buildout['reread']['passwd-sha256-crypt'],
+      self.buildout['random']['passwd-sha256-crypt'])
+    self.assertEqual(
+      self.buildout['reread']['passwd-bcrypt'],
+      self.buildout['random']['passwd-bcrypt'])
+    self.assertEqual(
+      self.buildout['reread']['passwd-ldap-salted-sha1'],
+      self.buildout['random']['passwd-ldap-salted-sha1'])
+    # values are strings which is important for python2
+    self.assertIsInstance(self.buildout['reread']['passwd'], str)
+    self.assertIsInstance(self.buildout['reread']['passwd-ldap-salted-sha1'], str)
+
+  def test_passlib_input_passwd(self):
+    self._makeRecipe({'passwd': 'insecure'})
+    self.assertEqual(self.buildout['random']['passwd'], 'insecure')
+
+    hashed = self.buildout['random']['passwd-sha256-crypt']
+    self.assertTrue(passlib.hash.sha256_crypt.verify('insecure', hashed))

[Thomas Gambier]

This commit is introducing the following error when passwd is set:

[2024-10-18 14:35:57,397] INFO     slapos While:
[2024-10-18 14:35:57,397] INFO     slapos   Installing switch_softwaretype.
[2024-10-18 14:35:57,397] INFO     slapos   Installing monitor-htpasswd.
[2024-10-18 14:35:57,397] INFO     slapos 
[2024-10-18 14:35:57,397] INFO     slapos An internal error occurred due to a bug in either zc.buildout or in a
[2024-10-18 14:35:57,397] INFO     slapos recipe being used:
[2024-10-18 14:35:57,397] INFO     slapos Traceback (most recent call last):
[2024-10-18 14:35:57,398] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 2664, in main
[2024-10-18 14:35:57,398] INFO     slapos     getattr(buildout, command)(args)
[2024-10-18 14:35:57,398] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 855, in install
[2024-10-18 14:35:57,398] INFO     slapos     self._install_parts(install_args)
[2024-10-18 14:35:57,398] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 1026, in _install_parts
[2024-10-18 14:35:57,398] INFO     slapos     installed_files = self[part]._call(recipe.install)
[2024-10-18 14:35:57,398] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 1948, in _call
[2024-10-18 14:35:57,398] INFO     slapos     return f()
[2024-10-18 14:35:57,398] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/slapos.cookbook-1.0.369-py3.9.egg/slapos/recipe/switch_softwaretype.py", line 122, in install
[2024-10-18 14:35:57,399] INFO     slapos     sub_buildout.install([])
[2024-10-18 14:35:57,399] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 855, in install[2024-10-18 14:35:57,399] INFO     slapos     self._install_parts(install_args)
[2024-10-18 14:35:57,399] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 1026, in _install_parts
[2024-10-18 14:35:57,399] INFO     slapos     installed_files = self[part]._call(recipe.install)
[2024-10-18 14:35:57,399] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/zc.buildout-3.0.1+slapos005-py3.9.egg/zc/buildout/buildout.py", line 1948, in _call
[2024-10-18 14:35:57,399] INFO     slapos     return f()
[2024-10-18 14:35:57,399] INFO     slapos   File "/opt/slapgrid/639c58b9c93e44c377060b1a1f618575/eggs/slapos.cookbook-1.0.369-py3.9.egg/slapos/recipe/random.py", line 208, in install
[2024-10-18 14:35:57,399] INFO     slapos     if self.create_once and os.stat(self.storage_path).st_size and not self._needs_migration:
[2024-10-18 14:35:57,400] INFO     slapos AttributeError: 'Password' object has no attribute '_needs_migration'

[Jérome Perrin]

This problem was with https://lab.nexedi.com/nexedi/slapos/-/blob/863e9196b40fa558f8e3619228add09cd9321b55/software/kvm/instance-kvm-resilient.cfg.jinja2#L35-44

using publish-early with storage-path does not work here.

I start to believe that this combination can also lead to this other problem we observe with partitions having different monitor passwords. I don't really have a full scenario, but because the password storage file is no longer updated if it has been generated, maybe there is a scenario where passwd is not an option a first time, a partition generates its storage file with a password it generates and during next buildout run, passwd is in option but storage file is not updated.

I thought that maybe this does not happen in the test because unlike "slapos master", slap proxy always returns up to date connection parameters and always returns partitions right after requesting. I tried to run KVM resilient tests with a modified slapos proxy introducing delays a bit similar to slapos master, but it did not reproduce. I did not try all possible timing delays scenarios, I still feel that the difference of timings might be the reason why we don't see that password problems with slapos proxy.

slapos proxy patch for test

diff --git a/slapos/proxy/views.py b/slapos/proxy/views.py
index 19ce0af13..ee477d95b 100644
--- a/slapos/proxy/views.py
+++ b/slapos/proxy/views.py
@@ -462,6 +468,14 @@ def supplySupply():
   return 'Supplied %r to be %s' % (url, state)
 
 
+request_failures_by_partition_reference = {
+  #'PBS (kvm / 1)': 2,
+  'PBS pushing on kvm1': 2,
+  'kvm0': 1,
+  'kvm1': 3,
+}
+
+
 @app.route('/requestComputerPartition', methods=['POST'])
 def requestComputerPartition():
   parsed_request_dict = parseRequestComputerPartitionForm(request.form)
@@ -537,6 +552,18 @@ def requestComputerPartition():
       software_instance = requestSlave(**parsed_request_dict)
     else:
       software_instance = requestNotSlave(**parsed_request_dict)
+    failures_left = request_failures_by_partition_reference.get(
+      parsed_request_dict['partition_reference'],
+      0)
+    print(request_failures_by_partition_reference, slave, matching_partition['reference']
+    , parsed_request_dict.get('partition_parameter_kw', {}).get('monitor-password'), 
+    repr(parsed_request_dict['partition_reference']))
+    #breakpoint()
+    request_failures_by_partition_reference[
+      parsed_request_dict['partition_reference']] = failures_left - 1
+    if failures_left > 0:
+      abort(503)
+
   else:
     # Instance is not yet allocated: try to do it.
     external_master_url = isRequestToBeForwardedToExternalMaster(parsed_request_dict)

This is not the full patch I was trying, I also tried similar approach in other places ( code to emulate frontend requests or getFullComputerInformation )