Commit bb841a7b authored by Jérome Perrin's avatar Jérome Perrin

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.
parent 00ebaac8
Pipeline #32290 failed with stage
...@@ -72,6 +72,8 @@ setup(name=name, ...@@ -72,6 +72,8 @@ setup(name=name,
'zc.buildout', # plays with buildout 'zc.buildout', # plays with buildout
'zc.recipe.egg', # for scripts generation 'zc.recipe.egg', # for scripts generation
'pytz', # for timezone database 'pytz', # for timezone database
'passlib',
'bcrypt',
], ],
zip_safe=True, zip_safe=True,
entry_points={ entry_points={
......
...@@ -131,7 +131,9 @@ class Recipe(GenericSlapRecipe): ...@@ -131,7 +131,9 @@ class Recipe(GenericSlapRecipe):
new = {} new = {}
for k, v in six.iteritems(init): for k, v in six.iteritems(init):
try: 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: except KeyError:
pass pass
if new != override: if new != override:
......
...@@ -33,12 +33,16 @@ buildout Software Releases and Instances developments. ...@@ -33,12 +33,16 @@ buildout Software Releases and Instances developments.
from __future__ import absolute_import from __future__ import absolute_import
import errno import errno
import json
import os import os
import random import random
import string import string
import sys
from .librecipe import GenericBaseRecipe from .librecipe import GenericBaseRecipe
from .publish_early import volatileOptions from .publish_early import volatileOptions
from slapos.util import str2bytes
import passlib.hash
class Integer(object): class Integer(object):
""" """
...@@ -113,7 +117,7 @@ def generatePassword(length): ...@@ -113,7 +117,7 @@ def generatePassword(length):
class Password(object): 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` 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 file, which is world-readable by default. So be careful not to spread it
...@@ -128,6 +132,11 @@ class Password(object): ...@@ -128,6 +132,11 @@ class Password(object):
- create-once: boolean value which set if storage-path won't be modified - create-once: boolean value which set if storage-path won't be modified
as soon the file is created with the password (not empty). as soon the file is created with the password (not empty).
(default: True) (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 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. fine it is saved by other means, e.g. using the publish-early recipe.
...@@ -141,24 +150,53 @@ class Password(object): ...@@ -141,24 +150,53 @@ class Password(object):
except KeyError: except KeyError:
self.storage_path = options['storage-path'] = os.path.join( self.storage_path = options['storage-path'] = os.path.join(
buildout['buildout']['parts-directory'], name) buildout['buildout']['parts-directory'], name)
passwd = options.get('passwd') passwd_dict = {
if not passwd: '': options.get('passwd')
}
if not passwd_dict['']:
if self.storage_path: if self.storage_path:
self._needs_migration = False
try: try:
with open(self.storage_path) as f: 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: except IOError as e:
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
raise 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 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: # Password must not go into .installed file, for 2 reasons:
# security of course but also to prevent buildout to always reinstall. # security of course but also to prevent buildout to always reinstall.
# publish_early already does it, but this recipe may also be used alone. # publish_early already does it, but this recipe may also be used alone.
volatileOptions(options, ('passwd',)) volatileOptions(options, ('passwd',))
self.passwd = passwd self.passwd_dict = passwd_dict
generatePassword = staticmethod(generatePassword) generatePassword = staticmethod(generatePassword)
...@@ -167,19 +205,14 @@ class Password(object): ...@@ -167,19 +205,14 @@ class Password(object):
try: try:
# The following 2 lines are just an optimization to avoid recreating # The following 2 lines are just an optimization to avoid recreating
# the file with the same content. # 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 return
os.unlink(self.storage_path) os.unlink(self.storage_path)
except OSError as e: except OSError as e:
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
raise raise
with open(self.storage_path, 'w') as f:
fd = os.open(self.storage_path, json.dump(self.passwd_dict, f)
os.O_CREAT | os.O_EXCL | os.O_WRONLY | os.O_TRUNC, 0o600)
try:
os.write(fd, str2bytes(self.passwd))
finally:
os.close(fd)
if not self.create_once: if not self.create_once:
return self.storage_path return self.storage_path
......
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))
  • 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'
  • 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 )

  • mentioned in commit ca009301

    Toggle commit list
  • mentioned in merge request !1671 (merged)

    Toggle commit list
  • mentioned in commit 86045425

    Toggle commit list
  • mentioned in commit 280370c7

    Toggle commit list
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment