[Config] Add mask_funcs to help mask passwords in logs
Added a new Config class parameter `log_mask_funcs` to enable config instances hide sensitive information that would normally appear in config debug logs. Added mask password function to hostlist to replace passwords with '*'s in logs. Closes: https://github.com/deluge-torrent/deluge/pull/363
This commit is contained in:
parent
a27a77f8c1
commit
bf97bec994
|
@ -107,13 +107,23 @@ class Config:
|
|||
file_version (int): The file format for the default config values when creating
|
||||
a fresh config. This value should be increased whenever a new migration function is
|
||||
setup to convert old config files. (default: 1)
|
||||
log_mask_funcs (dict): A dict of key:function, used to mask sensitive
|
||||
key values (e.g. passwords) when logging is enabled.
|
||||
|
||||
"""
|
||||
|
||||
def __init__(self, filename, defaults=None, config_dir=None, file_version=1):
|
||||
def __init__(
|
||||
self,
|
||||
filename,
|
||||
defaults=None,
|
||||
config_dir=None,
|
||||
file_version=1,
|
||||
log_mask_funcs=None,
|
||||
):
|
||||
self.__config = {}
|
||||
self.__set_functions = {}
|
||||
self.__change_callbacks = []
|
||||
self.__log_mask_funcs = log_mask_funcs if log_mask_funcs else {}
|
||||
|
||||
# These hold the version numbers and they will be set when loaded
|
||||
self.__version = {'format': 1, 'file': file_version}
|
||||
|
@ -187,7 +197,15 @@ class Config:
|
|||
if self.__config[key] == value:
|
||||
return
|
||||
|
||||
log.debug('Setting key "%s" to: %s (of type: %s)', key, value, type(value))
|
||||
if log.isEnabledFor(logging.DEBUG):
|
||||
if key in self.__log_mask_funcs:
|
||||
value = self.__log_mask_funcs[key](value)
|
||||
log.debug(
|
||||
'Setting key "%s" to: %s (of type: %s)',
|
||||
key,
|
||||
value,
|
||||
type(value),
|
||||
)
|
||||
self.__config[key] = value
|
||||
|
||||
# Skip save or func callbacks if setting default value for keys
|
||||
|
@ -334,7 +352,6 @@ class Config:
|
|||
# Run the function now if apply_now is set
|
||||
if apply_now:
|
||||
function(key, self.__config[key])
|
||||
return
|
||||
|
||||
def apply_all(self):
|
||||
"""Calls all set functions.
|
||||
|
@ -409,12 +426,24 @@ class Config:
|
|||
log.exception(ex)
|
||||
log.warning('Unable to load config file: %s', filename)
|
||||
|
||||
if not log.isEnabledFor(logging.DEBUG):
|
||||
return
|
||||
|
||||
config = self.__config
|
||||
if self.__log_mask_funcs:
|
||||
config = {
|
||||
key: self.__log_mask_funcs[key](config[key])
|
||||
if key in self.__log_mask_funcs
|
||||
else config[key]
|
||||
for key in config
|
||||
}
|
||||
|
||||
log.debug(
|
||||
'Config %s version: %s.%s loaded: %s',
|
||||
filename,
|
||||
self.__version['format'],
|
||||
self.__version['file'],
|
||||
self.__config,
|
||||
config,
|
||||
)
|
||||
|
||||
def save(self, filename=None):
|
||||
|
|
|
@ -4,6 +4,8 @@
|
|||
# See LICENSE for more details.
|
||||
#
|
||||
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
from codecs import getwriter
|
||||
|
||||
|
@ -12,6 +14,7 @@ from twisted.internet import task
|
|||
|
||||
from deluge.common import JSON_FORMAT
|
||||
from deluge.config import Config
|
||||
from deluge.ui.hostlist import mask_hosts_password
|
||||
|
||||
DEFAULTS = {
|
||||
'string': 'foobar',
|
||||
|
@ -20,9 +23,16 @@ DEFAULTS = {
|
|||
'bool': True,
|
||||
'unicode': 'foobar',
|
||||
'password': 'abc123*\\[!]?/<>#{@}=|"+$%(^)~',
|
||||
'hosts': [
|
||||
('host1', 'port', '', 'password1234'),
|
||||
('host2', 'port', '', 'password5678'),
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class TestConfig:
|
||||
def test_init(self):
|
||||
config = Config('test.conf', defaults=DEFAULTS, config_dir=self.config_dir)
|
||||
|
@ -82,6 +92,41 @@ class TestConfig:
|
|||
config['foobar'] = 5
|
||||
assert config.get('foobar', 2) == 5
|
||||
|
||||
def test_set_log_mask_funcs(self, caplog):
|
||||
"""Test mask func masks key in log"""
|
||||
caplog.set_level(logging.DEBUG)
|
||||
config = Config(
|
||||
'test.conf',
|
||||
config_dir=self.config_dir,
|
||||
log_mask_funcs={'hosts': mask_hosts_password},
|
||||
)
|
||||
config['hosts'] = DEFAULTS['hosts']
|
||||
assert isinstance(config['hosts'], list)
|
||||
assert 'host1' in caplog.text
|
||||
assert 'host2' in caplog.text
|
||||
assert 'password1234' not in caplog.text
|
||||
assert 'password5678' not in caplog.text
|
||||
assert '*' * 10 in caplog.text
|
||||
|
||||
def test_load_log_mask_funcs(self, caplog):
|
||||
"""Test mask func masks key in log"""
|
||||
with open(os.path.join(self.config_dir, 'test.conf'), 'wb') as _file:
|
||||
json.dump(DEFAULTS, getwriter('utf8')(_file), **JSON_FORMAT)
|
||||
|
||||
config = Config(
|
||||
'test.conf',
|
||||
config_dir=self.config_dir,
|
||||
log_mask_funcs={'hosts': mask_hosts_password},
|
||||
)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
config.load(os.path.join(self.config_dir, 'test.conf'))
|
||||
assert 'host1' in caplog.text
|
||||
assert 'host2' in caplog.text
|
||||
assert 'foobar' in caplog.text
|
||||
assert 'password1234' not in caplog.text
|
||||
assert 'password5678' not in caplog.text
|
||||
assert '*' * 10 in caplog.text
|
||||
|
||||
def test_load(self):
|
||||
def check_config():
|
||||
config = Config('test.conf', config_dir=self.config_dir)
|
||||
|
@ -91,8 +136,6 @@ class TestConfig:
|
|||
assert config['password'] == 'abc123*\\[!]?/<>#{@}=|"+$%(^)~'
|
||||
|
||||
# Test opening a previous 1.2 config file of just a json object
|
||||
import json
|
||||
|
||||
with open(os.path.join(self.config_dir, 'test.conf'), 'wb') as _file:
|
||||
json.dump(DEFAULTS, getwriter('utf8')(_file), **JSON_FORMAT)
|
||||
|
||||
|
|
|
@ -84,6 +84,14 @@ def migrate_config_2_to_3(config):
|
|||
return config
|
||||
|
||||
|
||||
def mask_hosts_password(hosts):
|
||||
"""Replace passwords in hosts list with *'s for log output"""
|
||||
if not hosts:
|
||||
return hosts
|
||||
|
||||
return [list(host)[:-1] + ['*' * 10] for host in hosts]
|
||||
|
||||
|
||||
class HostList:
|
||||
"""This class contains methods for adding, removing and looking up hosts in hostlist.conf."""
|
||||
|
||||
|
@ -94,6 +102,7 @@ class HostList:
|
|||
default_hostlist(),
|
||||
config_dir=get_config_dir(),
|
||||
file_version=3,
|
||||
log_mask_funcs={'hosts': mask_hosts_password},
|
||||
)
|
||||
self.config.run_converter((1, 2), 3, migrate_config_2_to_3)
|
||||
self.config.save()
|
||||
|
|
Loading…
Reference in New Issue