add config.yml as default config file format

None of the config options in config.py require Python code.  YAML is a
common config data format, and it is also used for build metadata.  It is
also much safer to use since it can be pure data, without anything
executable in it.  This also reduces the attack surface of the fdroid
process by eliminating a guaranteed place to write to get code executed.
With config.py, any exploit that can get local write access can turn that
into execute access by writing to the config.py, then cleaning up after
itself once it has what it needs.  Switching to YAML removes that vector
entirely.

Also, this removes the config_file argument.  It is not used in either
fdroidserver or repomaker.  Also, it probably wouldn't work since so
much of the code assumes that the current working dir is the root of the
repo.
This commit is contained in:
Hans-Christoph Steiner 2018-09-04 11:03:05 +02:00
parent fd41b70e27
commit d3d48dba5e
2 changed files with 93 additions and 8 deletions

View File

@ -39,6 +39,7 @@ import socket
import base64
import urllib.parse
import urllib.request
import yaml
import zipfile
import tempfile
import json
@ -284,14 +285,18 @@ def regsub_file(pattern, repl, path):
f.write(text)
def read_config(opts, config_file='config.py'):
def read_config(opts):
"""Read the repository config
The config is read from config_file, which is in the current
directory when any of the repo management commands are used. If
there is a local metadata file in the git repo, then config.py is
there is a local metadata file in the git repo, then the config is
not required, just use defaults.
config.yml is the preferred form because no code is executed when
reading it. config.py is deprecated and supported for backwards
compatibility.
"""
global config, options
@ -301,12 +306,23 @@ def read_config(opts, config_file='config.py'):
options = opts
config = {}
config_file = 'config.yml'
old_config_file = 'config.py'
if os.path.isfile(config_file):
if os.path.exists(config_file) and os.path.exists(old_config_file):
logging.error(_("""Conflicting config files! Using {newfile}, ignoring {oldfile}!""")
.format(oldfile=old_config_file, newfile=config_file))
if os.path.exists(config_file):
logging.debug(_("Reading '{config_file}'").format(config_file=config_file))
with io.open(config_file, "rb") as f:
code = compile(f.read(), config_file, 'exec')
exec(code, None, config) # nosec TODO switch to YAML file
with open(config_file) as fp:
config = yaml.safe_load(fp)
elif os.path.exists(old_config_file):
logging.warning(_("""{oldfile} is deprecated, use {newfile}""")
.format(oldfile=old_config_file, newfile=config_file))
with io.open(old_config_file, "rb") as fp:
code = compile(fp.read(), old_config_file, 'exec')
exec(code, None, config) # nosec TODO automatically migrate
else:
logging.warning(_("No 'config.py' found, using defaults."))
@ -329,10 +345,14 @@ def read_config(opts, config_file='config.py'):
'-providerArg', 'opensc-fdroid.cfg']
if any(k in config for k in ["keystore", "keystorepass", "keypass"]):
st = os.stat(config_file)
if os.path.exists(config_file):
f = config_file
elif os.path.exists(old_config_file):
f = old_config_file
st = os.stat(f)
if st.st_mode & stat.S_IRWXG or st.st_mode & stat.S_IRWXO:
logging.warning(_("unsafe permissions on '{config_file}' (should be 0600)!")
.format(config_file=config_file))
.format(config_file=f))
fill_config_defaults(config)

View File

@ -47,6 +47,7 @@ class CommonTest(unittest.TestCase):
if not os.path.exists(self.tmpdir):
os.makedirs(self.tmpdir)
os.chdir(self.basedir)
fdroidserver.common.config = None
def test_parse_human_readable_size(self):
for k, v in ((9827, 9827), (123.456, 123), ('123b', 123), ('1.2', 1),
@ -1409,6 +1410,70 @@ class CommonTest(unittest.TestCase):
self.assertIsNotNone(result)
self.assertNotEqual(result, '')
def test_with_no_config(self):
"""It should set defaults if no config file is found"""
testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
os.chdir(testdir)
self.assertFalse(os.path.exists('config.yml'))
self.assertFalse(os.path.exists('config.py'))
config = fdroidserver.common.read_config(fdroidserver.common.options)
self.assertEqual(None, config.get('apksigner'))
self.assertIsNotNone(config.get('char_limits'))
def test_with_config_yml(self):
"""Make sure it is possible to use config.yml alone."""
testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
os.chdir(testdir)
with open('config.yml', 'w') as fp:
fp.write('apksigner: yml')
self.assertTrue(os.path.exists('config.yml'))
self.assertFalse(os.path.exists('config.py'))
config = fdroidserver.common.read_config(fdroidserver.common.options)
self.assertEqual('yml', config.get('apksigner'))
def test_with_config_py(self):
"""Make sure it is still possible to use config.py alone."""
testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
os.chdir(testdir)
with open('config.py', 'w') as fp:
fp.write('apksigner = "py"')
self.assertFalse(os.path.exists('config.yml'))
self.assertTrue(os.path.exists('config.py'))
config = fdroidserver.common.read_config(fdroidserver.common.options)
self.assertEqual("py", config.get('apksigner'))
def test_config_perm_warning(self):
"""Exercise the code path that issues a warning about unsafe permissions."""
testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
os.chdir(testdir)
with open('config.yml', 'w') as fp:
fp.write('keystore: foo.jks')
self.assertTrue(os.path.exists(fp.name))
os.chmod(fp.name, 0o666)
fdroidserver.common.read_config(fdroidserver.common.options)
os.remove(fp.name)
fdroidserver.common.config = None
with open('config.py', 'w') as fp:
fp.write('keystore = "foo.jks"')
self.assertTrue(os.path.exists(fp.name))
os.chmod(fp.name, 0o666)
fdroidserver.common.read_config(fdroidserver.common.options)
def test_with_both_config_yml_py(self):
"""If config.yml and config.py are present, config.py should be ignored."""
testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
os.chdir(testdir)
with open('config.yml', 'w') as fp:
fp.write('apksigner: yml')
with open('config.py', 'w') as fp:
fp.write('apksigner = "py"')
self.assertTrue(os.path.exists('config.yml'))
self.assertTrue(os.path.exists('config.py'))
config = fdroidserver.common.read_config(fdroidserver.common.options)
self.assertEqual('yml', config.get('apksigner'))
if __name__ == "__main__":
os.chdir(os.path.dirname(__file__))