*: switch to new slapos.cookbook random API for passwords

software/dufs/instance.cfg.in

@@ -158,7 +158,7 @@ command-line =
     --render-try-index
     --allow-all
     --auth-method basic
-    --auth ${admin-password:user}:${admin-password:passwd}@/:rw
+    --auth ${admin-password:user}:${admin-password:passwd-sha512-crypt}@/:rw
     --auth @/pub
     --tls-cert ${dufs-certificate:cert-file}
     --tls-key ${dufs-certificate:key-file}

software/erp5/test/test/test_erp5.py

@@ -32,6 +32,7 @@ import glob
 import http.client
 import json
 import os
+import pathlib
 import resource
 import shutil
 import socket
@@ -1100,6 +1101,22 @@ class TestNEO(ZopeSkinsMixin, CrontabMixin, ERP5InstanceTestCase):
           'log',
           f))
 
+class TestPassword(ERP5InstanceTestCase, TestPublishedURLIsReachableMixin):
+  __partition_reference__ = 'p'
+
+  def test_no_plain_text_password_in_files(self):
+    inituser_password = self.getRootPartitionConnectionParameterDict()[
+      'inituser-password'].encode()
+    self.assertFalse(
+      [f for f in pathlib.Path(self.slap._instance_root).glob('**/*')
+        if f.is_file() and inituser_password in f.read_bytes()])
+    # the hashed password is present in some files
+    inituser_password_hashed = self.getRootPartitionConnectionParameterDict()[
+      'inituser-password-hashed'].encode()
+    self.assertTrue(
+      [f for f in pathlib.Path(self.slap._instance_root).glob('**/*')
+        if f.is_file() and inituser_password_hashed in f.read_bytes()])
+
 
 class TestWithMaxRlimitNofileParameter(ERP5InstanceTestCase, TestPublishedURLIsReachableMixin):
   """Test setting the with-max-rlimit-nofile parameter sets the open fd soft limit to the hard limit.

software/restic-rest-server/buildout.hash.cfg

@@ -15,4 +15,4 @@
 
 [instance.cfg.in]
 filename = instance.cfg.in
-md5sum = 65d2254369eb5aaaaaf488a56c120ed6
+md5sum = b5c479ebb4cf2fd2f63623af88b95078

software/restic-rest-server/instance.cfg.in

@@ -147,18 +147,10 @@ recipe = slapos.cookbook:generate.password
 user = backup
 
 [rest-server-htpassword]
-recipe = plone.recipe.command
-command =
-  if [ ! -f '${:htpassword}' ] ; then
-    {{ htpasswd_bin }} \
-      -b \
-      -B \
-      -c ${:htpassword} \
-      ${rest-server-password:user} \
-      ${rest-server-password:passwd}
-  fi
-htpassword = ${directory:rest-server-data-dir}/.htpasswd
-stop-on-error = true
+recipe = slapos.recipe.template
+inline =
+  ${rest-server-password:user}:${rest-server-password:passwd-bcrypt}
+output = ${directory:rest-server-data-dir}/.htpasswd
 
 [rest-server]
 recipe = slapos.cookbook:wrapper

software/restic-rest-server/software.cfg

@@ -35,7 +35,6 @@ context =
   section buildout buildout
   key gowork_bin gowork:bin
   raw openssl_bin ${openssl:location}/bin/openssl
-  raw htpasswd_bin ${apache:location}/bin/htpasswd
   raw dash_bin ${dash:location}/bin/dash
   raw curl_bin ${curl:location}/bin/curl
   key template_monitor monitor2-template:output

software/seleniumserver/buildout.hash.cfg

@@ -19,4 +19,4 @@ md5sum = 10e19df182c692b71ea552da183a0bcf
 
 [template-selenium]
 filename = instance-selenium.cfg.in
-md5sum = 5a7abfff9f9d7898620f8c7fc1e6f488
\ No newline at end of file
+md5sum = 7239845e758b2d10299699e061b0fc75
\ No newline at end of file

software/seleniumserver/instance-selenium.cfg.in

@@ -170,14 +170,14 @@ content =
     use_backend admin if { path_beg $${selenium-server-frontend-configuration:path-admin} }
 
   userlist hub
-    user $${selenium-server-selenium-password:username} insecure-password $${selenium-server-selenium-password:passwd}
+    user $${selenium-server-selenium-password:username} password $${selenium-server-selenium-password:passwd-sha256-crypt}
   backend hub
     acl auth_ok http_auth(hub)
     http-request auth realm "Selenium Server" unless auth_ok
     server hub $${selenium-server-hub-instance:hostname}:$${selenium-server-hub-instance:port}
 
   userlist admin
-    user $${selenium-server-admin-password:username} insecure-password $${selenium-server-admin-password:passwd}
+    user $${selenium-server-admin-password:username} password $${selenium-server-admin-password:passwd-sha256-crypt}
   backend admin
     acl auth_ok http_auth(admin)
     http-request auth realm "Grid Admin" unless auth_ok

software/slapos-master/buildout.hash.cfg

@@ -14,7 +14,7 @@
 # not need these here).
 [template-erp5]
 filename = instance-erp5.cfg.in
-md5sum = 1fbfca2d64a9824054f7a3281e71efdc
+md5sum = ba46a66da1c834df14a80a20b21e4a96
 
 [template-balancer]
 filename = instance-balancer.cfg.in

software/slapos-master/instance-erp5.cfg.in

@@ -247,7 +247,7 @@ config-id-store-interval = {{ dumps(slapparameter_dict.get('id-store-interval'))
 config-zope-longrequest-logger-error-threshold = {{ dumps(monitor_dict.get('zope-longrequest-logger-error-threshold', 20)) }}
 config-zope-longrequest-logger-maximum-delay = {{ dumps(monitor_dict.get('zope-longrequest-logger-maximum-delay', 0)) }}
 config-inituser-login = {{ dumps(inituser_login) }}
-config-inituser-password = ${publish-early:inituser-password}
+config-inituser-password-hashed = ${publish-early:inituser-password-hashed}
 config-kumofs-url = ${request-memcached-persistent:connection-url}
 config-memcached-url = ${request-memcached-volatile:connection-url}
 config-monitor-passwd = ${monitor-htpasswd:passwd}
@@ -515,6 +515,7 @@ hosts-dict = {{ '${' ~ zope_address_list_id_dict.keys()[0] ~ ':connection-hosts-
 recipe = slapos.cookbook:publish-early
 -init =
   inituser-password gen-password:passwd
+  inituser-password-hashed gen-password:passwd-ldap-salted-sha1
   deadlock-debugger-password gen-deadlock-debugger-password:passwd
 {%- if has_posftix %}
   smtpd-sasl-password gen-smtpd-sasl-password:passwd
@@ -532,10 +533,6 @@ recipe = slapos.cookbook:publish-early
 neo-cluster = {{ dumps(neo[0]) }}
 {%-  endif %}
 {%- endif %}
-{%- set inituser_password = slapparameter_dict.get('inituser-password') %}
-{%- if inituser_password %}
-inituser-password = {{ dumps(inituser_password) }}
-{%- endif %}
 {%- set deadlock_debugger_password = slapparameter_dict.get('deadlock-debugger-password') -%}
 {%- if deadlock_debugger_password %}
 deadlock-debugger-password = {{ dumps(deadlock_debugger_password) }}
@@ -552,6 +549,10 @@ recipe =
 [gen-password]
 recipe = slapos.cookbook:generate.password
 storage-path =
+{%- set inituser_password = slapparameter_dict.get('inituser-password') %}
+{%- if inituser_password %}
+passwd = {{ dumps(inituser_password) }}
+{%- endif %}
 
 [gen-deadlock-debugger-password]
 <= gen-password

software/theia/buildout.hash.cfg

@@ -15,7 +15,7 @@
 
 [instance-theia]
 _update_hash_filename_ = instance-theia.cfg.jinja.in
-md5sum = 43b3435b3bc08db42335c03b5b8fe465
+md5sum = 07b222d8c29d446fc0957e4e37706585
 
 [instance]
 _update_hash_filename_ = instance.cfg.in

software/theia/instance-theia.cfg.jinja.in

@@ -255,7 +255,7 @@ context =
   key content :content
 content =
   userlist basic-auth-list
-    user $${frontend-instance-password:username} insecure-password $${frontend-instance-password:passwd}
+    user $${frontend-instance-password:username} password $${frontend-instance-password:passwd-sha256-crypt}
 
   frontend app
     log global

stack/erp5/buildout.hash.cfg

@@ -70,11 +70,11 @@ md5sum = b95084ae9eed95a68eada45e28ef0c04
 
 [template]
 filename = instance.cfg.in
-md5sum = 55232eae0bcdb68a7cb2598d2ba9d60c
+md5sum = 5e0e9565227fe190c420a7bbcd0f7b93
 
 [template-erp5]
 filename = instance-erp5.cfg.in
-md5sum = 359bab24aec7772adb5d822c1389b1bd
+md5sum = 2b91528d3a77a64714e4295a84c1d71b
 
 [template-zeo]
 filename = instance-zeo.cfg.in
@@ -86,7 +86,7 @@ md5sum = 0ac4b74436f554cd677f19275d18d880
 
 [template-zope]
 filename = instance-zope.cfg.in
-md5sum = 2439b90d6f707f47050fc9074fa4d810
+md5sum = 41709f47e5a9051ca4a9c943859f589b
 
 [template-balancer]
 filename = instance-balancer.cfg.in

stack/erp5/instance-erp5.cfg.in

@@ -254,7 +254,7 @@ config-id-store-interval = {{ dumps(slapparameter_dict.get('id-store-interval'))
 config-zope-longrequest-logger-error-threshold = {{ dumps(monitor_dict.get('zope-longrequest-logger-error-threshold', 20)) }}
 config-zope-longrequest-logger-maximum-delay = {{ dumps(monitor_dict.get('zope-longrequest-logger-maximum-delay', 0)) }}
 config-inituser-login = {{ dumps(inituser_login) }}
-config-inituser-password = ${publish-early:inituser-password}
+config-inituser-password-hashed = ${publish-early:inituser-password-hashed}
 config-kumofs-url = ${request-memcached-persistent:connection-url}
 config-memcached-url = ${request-memcached-volatile:connection-url}
 config-monitor-passwd = ${monitor-htpasswd:passwd}
@@ -515,6 +515,7 @@ hosts-dict = {{ '${' ~ next(iter(zope_address_list_id_dict)) ~ ':connection-host
 recipe = slapos.cookbook:publish-early
 -init =
   inituser-password gen-password:passwd
+  inituser-password-hashed gen-password:passwd-ldap-salted-sha1
   deadlock-debugger-password gen-deadlock-debugger-password:passwd
 {%- if has_posftix %}
   smtpd-sasl-password gen-smtpd-sasl-password:passwd
@@ -532,10 +533,6 @@ recipe = slapos.cookbook:publish-early
 neo-cluster = {{ dumps(neo[0]) }}
 {%-  endif %}
 {%- endif %}
-{%- set inituser_password = slapparameter_dict.get('inituser-password') %}
-{%- if inituser_password %}
-inituser-password = {{ dumps(inituser_password) }}
-{%- endif %}
 {%- set deadlock_debugger_password = slapparameter_dict.get('deadlock-debugger-password') -%}
 {%- if deadlock_debugger_password %}
 deadlock-debugger-password = {{ dumps(deadlock_debugger_password) }}
@@ -552,6 +549,10 @@ recipe =
 [gen-password]
 recipe = slapos.cookbook:generate.password
 storage-path =
+{%- set inituser_password = slapparameter_dict.get('inituser-password') %}
+{%- if inituser_password %}
+passwd = {{ dumps(inituser_password) }}
+{%- endif %}
 
 [gen-deadlock-debugger-password]
 <= gen-password

stack/erp5/instance-zope.cfg.in

@@ -252,7 +252,7 @@ file-list = {{ parameter_dict['site-zcml'] }}
 [{{ section('zope-inituser') }}]
 < = jinja2-template-base
 output = ${directory:instance}/inituser
-inline = {{ slapparameter_dict['inituser-login'] }}:{SHA}{{ base64.b64encode(hashlib.sha1(slapparameter_dict['inituser-password'].encode('utf-8')).digest()) }}
+inline = {{ slapparameter_dict['inituser-login'] }}:{{ slapparameter_dict['inituser-password-hashed'] }}
 once = ${:output}_done
 
 [zope-conf-parameter-base]

stack/erp5/instance.cfg.in

@@ -143,9 +143,7 @@ extra-context =
     key buildout_directory buildout:directory
     key root_common context:root-common
     section parameter_dict dynamic-template-zope-parameters
-    import base64 base64
     import urllib_parse six.moves.urllib.parse
-    import hashlib hashlib
     import itertools itertools
     import json json
 import-list =

stack/monitor/buildout.hash.cfg

@@ -14,7 +14,7 @@
 # not need these here).
 [monitor2-template]
 filename = instance-monitor.cfg.jinja2.in
-md5sum = 3850140a4e61349cc64fa924ce410803
+md5sum = 24c7f5527d994e231b4c2bf9fecb68a6
 
 [monitor-httpd-conf]
 _update_hash_filename_ = templates/monitor-httpd.conf.in

stack/monitor/instance-monitor.cfg.jinja2.in

@@ -112,7 +112,6 @@ parameter-list =
   htpasswd monitor-password ${httpd-monitor-htpasswd:password-file} ${monitor-instance-parameter:username} ${httpd-monitor-htpasswd:htpasswd-path}
   file min-free-disk-MB ${promise-check-free-disk-space:config-threshold-file}
   ${monitor-instance-parameter:instance-configuration}
-# htpasswd entry:  htpasswd key password-file username htpasswd-file
 
 promise-output-file = ${directory:monitor}/monitor-bootstrap-status
 
@@ -157,14 +156,11 @@ storage-path = ${directory:etc}/.monitor_pwd
 [httpd-monitor-htpasswd]
 recipe = plone.recipe.command
 stop-on-error = true
-password-file = ${directory:etc}/.monitor_pwd
+password-file = ${monitor-directory:etc}/.monitor-password
 htpasswd-path = ${monitor-directory:etc}/monitor-htpasswd
 command =
   echo "${monitor-instance-parameter:password}" >${:password-file}
-  [ -s "${:htpasswd-path}" ] ||
-    {{ apache_location }}/bin/htpasswd -ci ${:htpasswd-path} "${monitor-instance-parameter:username}" <${:password-file}
-update-command =
-  [ -s "${:password-file}" ] || ${:command}
+  {{ apache_location }}/bin/htpasswd -cib ${:htpasswd-path} "${monitor-instance-parameter:username}" "${monitor-instance-parameter:password}"
 
 [monitor-symlink]
 recipe = cns.recipe.symlink
@@ -343,8 +339,6 @@ collector-db = /srv/slapgrid/var/data-log/collector.db
 # Credentials
 password = ${monitor-htpasswd:passwd}
 username = admin
-# XXX: type key value
-# ex raw monitor-password resqdsdsd34
 instance-configuration =
 
 configuration-file-path = ${monitor-directory:etc}/monitor_knowledge0.cfg

[Xavier Thompson]

I think this might have worsened the situation with monitor passwords: now this does not converge anymore; if etc/.monitor_pwd is deleted for any reason, it will never be recreated, because buildout does not know this file is installed by the recipe, so calls update instead of install even when the file is missing, and so the file is never re-created.

However, changing

[ -s "${:htpasswd-path}" ] ||
    {{ apache_location }}/bin/htpasswd -ci ${:htpasswd-path} "${monitor-instance-parameter:username}" 

to unconditional

{{ apache_location }}/bin/htpasswd -cib ${:htpasswd-path} "${monitor-instance-parameter:username}" 

Is a good thing in itself: before, if etc/.monitor_pwd was recreated (e.g. because ${monitor-instance-parameter:password} changed, triggering re-install) but etc/monitor-htpasswd had not been deleted, etc/monitor-htpasswd was not recreated, thus keeping the signature of the old password and becoming out of sync with etc/.monitor_pwd. This led to recurring issues in resilient instances (where the password of subinstances is passed by the root instance to enable aggregation of monitoring across the whole instance tree), where operators had to manually delete etc/monitor-htpasswd if it became out-of-sync. This could e.g. happen if the root instance was moved to another machine.

I think now the only thing missing to have a good converging situation is to let buildout know it should install if either etc/.monitor_pwd or etc/monitor-htpasswd is missing, either by using a recipe that lets us specify those two files as installed by the recipe (maybe slapos.recipe.build, but I'm not sure it supports more than one file or a whole directory), or by re-adding a:

update-command =
  [ -s "${:password-file}" ] && [ -s "${:htpasswd-path}" ] || ${:command}

EDIT:

plone.recipe.command supports location option with multiple paths, so a much better solution is to add

location =
  ${:password-file}
  ${:htpasswd-path}

[Xavier Thompson]

/cc @rporchetto

[Roque]

Related issue: https://www.erp5.com/project_section/vifib/forum/monitor-stack---bug--monitor-htpasswd-and.monitor_pwd-out-of-sync-mTyxn6gw4R

Thanks for the analysis, I'll check it in MR !1631 (merged)

[Xavier Thompson]

In the meantime I've committed this patch in a test branch:

location =
  ${:password-file}
  ${:htpasswd-path}

and if all goes well I'll just push on master.

[Romain Courteaud]

With a test to prevent any regression?