Merge pull request #742 from YunoHost/redact-secrets-from-logs

[enh] Redact secrets from logs

data/helpers.d/postgresql

@@ -265,8 +265,8 @@ ynh_psql_test_if_first_run() {
 	if [ -f "$PSQL_ROOT_PWD_FILE" ]; then
 		echo "PostgreSQL is already installed, no need to create master password"
 	else
-		local pgsql="$(ynh_string_random)"
+		local psql_root_password="$(ynh_string_random)"
-		echo "$pgsql" >/etc/yunohost/psql
+		echo "$psql_root_password" >$PSQL_ROOT_PWD_FILE
 
 		if [ -e /etc/postgresql/9.4/ ]; then
 			local pg_hba=/etc/postgresql/9.4/main/pg_hba.conf
@@ -280,7 +280,7 @@ ynh_psql_test_if_first_run() {
 
 		ynh_systemd_action --service_name=postgresql --action=start
 
-		sudo --login --user=postgres psql -c"ALTER user postgres WITH PASSWORD '$pgsql'" postgres
+		sudo --login --user=postgres psql -c"ALTER user postgres WITH PASSWORD '$psql_root_password'" postgres
 
 		# force all user to connect to local database using passwords
 		# https://www.postgresql.org/docs/current/static/auth-pg-hba-conf.html#EXAMPLE-PG-HBA.CONF

src/yunohost/app.py

@@ -491,7 +491,7 @@ def app_change_url(operation_logger, app, domain, path):
     # Retrieve arguments list for change_url script
     # TODO: Allow to specify arguments
     args_odict = _parse_args_from_manifest(manifest, 'change_url')
-    args_list = args_odict.values()
+    args_list = [ value[0] for value in args_odict.values() ]
     args_list.append(app)
 
     # Prepare env. var. to pass to script
@@ -640,7 +640,7 @@ def app_upgrade(app=[], url=None, file=None):
         # Retrieve arguments list for upgrade script
         # TODO: Allow to specify arguments
         args_odict = _parse_args_from_manifest(manifest, 'upgrade')
-        args_list = args_odict.values()
+        args_list = [ value[0] for value in args_odict.values() ]
         args_list.append(app_instance_name)
 
         # Prepare env. var. to pass to script
@@ -798,7 +798,7 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
     args_dict = {} if not args else \
         dict(urlparse.parse_qsl(args, keep_blank_values=True))
     args_odict = _parse_args_from_manifest(manifest, 'install', args=args_dict)
-    args_list = args_odict.values()
+    args_list = [ value[0] for value in args_odict.values() ]
     args_list.append(app_instance_name)
 
     # Prepare env. var. to pass to script
@@ -809,6 +809,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu
 
     # Start register change on system
     operation_logger.extra.update({'env': env_dict})
+    # Tell the operation_logger to redact all password-type args
+    data_to_redact = [ value[0] for value in args_odict.values() if value[1] == "password" ]
+    operation_logger.data_to_redact.extend(data_to_redact)
     operation_logger.related_to = [s for s in operation_logger.related_to if s[0] != "app"]
     operation_logger.related_to.append(("app", app_id))
     operation_logger.start()
@@ -1536,7 +1539,7 @@ def app_action_run(app, action, args=None):
     # Retrieve arguments list for install script
     args_dict = dict(urlparse.parse_qsl(args, keep_blank_values=True)) if args else {}
     args_odict = _parse_args_for_action(actions[action], args=args_dict)
-    args_list = args_odict.values()
+    args_list = [ value[0] for value in args_odict.values() ]
 
     app_id, app_instance_nb = _parse_app_instance_name(app)
 
@@ -2464,7 +2467,7 @@ def _parse_args_in_yunohost_format(args, action_args):
             if arg.get("optional", False):
                 # Argument is optional, keep an empty value
                 # and that's all for this arg !
-                args_dict[arg_name] = ''
+                args_dict[arg_name] = ('', arg_type)
                 continue
             else:
                 # The argument is required !
@@ -2502,22 +2505,20 @@ def _parse_args_in_yunohost_format(args, action_args):
                 raise YunohostError('pattern_password_app', forbidden_chars=forbidden_chars)
             from yunohost.utils.password import assert_password_is_strong_enough
             assert_password_is_strong_enough('user', arg_value)
-        args_dict[arg_name] = arg_value
+        args_dict[arg_name] = (arg_value, arg_type)
 
     # END loop over action_args...
 
     # If there's only one "domain" and "path", validate that domain/path
     # is an available url and normalize the path.
 
-    domain_args = [arg["name"] for arg in action_args
+    domain_args = [ (name, value[0]) for name, value in args_dict.items() if value[1] == "domain" ]
-                   if arg.get("type", "string") == "domain"]
+    path_args = [ (name, value[0]) for name, value in args_dict.items() if value[1] == "path" ]
-    path_args = [arg["name"] for arg in action_args
-                 if arg.get("type", "string") == "path"]
 
     if len(domain_args) == 1 and len(path_args) == 1:
 
-        domain = args_dict[domain_args[0]]
+        domain = domain_args[0][1]
-        path = args_dict[path_args[0]]
+        path = path_args[0][1]
         domain, path = _normalize_domain_path(domain, path)
 
         # Check the url is available
@@ -2536,7 +2537,7 @@ def _parse_args_in_yunohost_format(args, action_args):
 
         # (We save this normalized path so that the install script have a
         # standard path format to deal with no matter what the user inputted)
-        args_dict[path_args[0]] = path
+        args_dict[path_args[0][0]] = (path, "path")
 
     return args_dict
 
@@ -2551,8 +2552,8 @@ def _make_environment_dict(args_dict, prefix="APP_ARG_"):
 
     """
     env_dict = {}
-    for arg_name, arg_value in args_dict.items():
+    for arg_name, arg_value_and_type in args_dict.items():
-        env_dict["YNH_%s%s" % (prefix, arg_name.upper())] = arg_value
+        env_dict["YNH_%s%s" % (prefix, arg_name.upper())] = arg_value_and_type[0]
     return env_dict
 
 

src/yunohost/log.py

@@ -25,6 +25,7 @@
 """
 
 import os
+import re
 import yaml
 import collections
 
@@ -289,6 +290,33 @@ def is_unit_operation(entities=['app', 'domain', 'service', 'user'],
     return decorate
 
 
+class RedactingFormatter(Formatter):
+
+    def __init__(self, format_string, data_to_redact):
+        super(RedactingFormatter, self).__init__(format_string)
+        self.data_to_redact = data_to_redact
+
+    def format(self, record):
+        msg = super(RedactingFormatter, self).format(record)
+        self.identify_data_to_redact(msg)
+        for data in self.data_to_redact:
+            msg = msg.replace(data, "**********")
+        return msg
+
+    def identify_data_to_redact(self, record):
+
+        # Wrapping this in a try/except because we don't want this to
+        # break everything in case it fails miserably for some reason :s
+        try:
+            # This matches stuff like db_pwd=the_secret or admin_password=other_secret
+            # (the secret part being at least 3 chars to avoid catching some lines like just "db_pwd=")
+            match = re.search(r'(pwd|pass|password)=(\S{3,})$', record.strip())
+            if match and match.group(2) not in self.data_to_redact:
+                self.data_to_redact.append(match.group(2))
+        except Exception as e:
+            logger.warning("Failed to parse line to try to identify data to redact ... : %s" % e)
+
+
 class OperationLogger(object):
 
     """
@@ -309,6 +337,11 @@ class OperationLogger(object):
         self.ended_at = None
         self.logger = None
         self._name = None
+        self.data_to_redact = []
+
+        for filename in ["/etc/yunohost/mysql", "/etc/yunohost/psql"]:
+            if os.path.exists(filename):
+                self.data_to_redact.append(read_file(filename).strip())
 
         self.path = OPERATIONS_PATH
 
@@ -345,9 +378,12 @@ class OperationLogger(object):
         Register log with a handler connected on log system
         """
 
-        # TODO add a way to not save password on app installation
         self.file_handler = FileHandler(self.log_path)
-        self.file_handler.formatter = Formatter('%(asctime)s: %(levelname)s - %(message)s')
+        # We use a custom formatter that's able to redact all stuff in self.data_to_redact
+        # N.B. : the subtle thing here is that the class will remember a pointer to the list,
+        # so we can directly append stuff to self.data_to_redact and that'll be automatically
+        # propagated to the RedactingFormatter
+        self.file_handler.formatter = RedactingFormatter('%(asctime)s: %(levelname)s - %(message)s', self.data_to_redact)
 
         # Listen to the root logger
         self.logger = getLogger('yunohost')
@@ -358,8 +394,11 @@ class OperationLogger(object):
         Write or rewrite the metadata file with all metadata known
         """
 
+        dump = yaml.safe_dump(self.metadata, default_flow_style=False)
+        for data in self.data_to_redact:
+            dump = dump.replace(data, "**********")
         with open(self.md_path, 'w') as outfile:
-            yaml.safe_dump(self.metadata, outfile, default_flow_style=False)
+            outfile.write(dump)
 
     @property
     def name(self):