only show "unsafe perms on config.yml" when secrets are present

This should make for fewer false positives.
This commit is contained in:
Hans-Christoph Steiner 2025-03-05 12:22:22 +01:00
parent 36007d50e5
commit 858068c64b
2 changed files with 53 additions and 19 deletions

View file

@ -598,15 +598,6 @@ def read_config():
'sun.security.pkcs11.SunPKCS11', 'sun.security.pkcs11.SunPKCS11',
'-providerArg', 'opensc-fdroid.cfg'] '-providerArg', 'opensc-fdroid.cfg']
if any(k in config for k in ["keystore", "keystorepass", "keypass"]):
st = os.stat(CONFIG_FILE)
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
)
)
fill_config_defaults(config) fill_config_defaults(config)
if 'serverwebroot' in config: if 'serverwebroot' in config:
@ -666,6 +657,18 @@ def read_config():
for configname in confignames_to_delete: for configname in confignames_to_delete:
del config[configname] del config[configname]
if any(
k in config and config.get(k)
for k in ["awssecretkey", "keystorepass", "keypass"]
):
st = os.stat(CONFIG_FILE)
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
)
)
return config return config

View file

@ -1964,16 +1964,6 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase):
with self.assertRaises(ruamel.yaml.scanner.ScannerError): with self.assertRaises(ruamel.yaml.scanner.ScannerError):
fdroidserver.common.read_config() fdroidserver.common.read_config()
def test_config_perm_warning(self):
"""Exercise the code path that issues a warning about unsafe permissions."""
os.chdir(self.tmpdir)
fdroidserver.common.write_config_file('keystore: foo.jks')
self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE))
os.chmod(fdroidserver.common.CONFIG_FILE, 0o666) # nosec B103
fdroidserver.common.read_config()
os.remove(fdroidserver.common.CONFIG_FILE)
fdroidserver.common.config = None
def test_config_repo_url(self): def test_config_repo_url(self):
"""repo_url ends in /repo, archive_url ends in /archive.""" """repo_url ends in /repo, archive_url ends in /archive."""
os.chdir(self.tmpdir) os.chdir(self.tmpdir)
@ -3444,3 +3434,44 @@ class ConfigOptionsScopeTest(unittest.TestCase):
'config' not in vars() and 'config' not in globals(), 'config' not in vars() and 'config' not in globals(),
"The config should not be set in the global context, only module-level.", "The config should not be set in the global context, only module-level.",
) )
class UnsafePermissionsTest(SetUpTearDownMixin, unittest.TestCase):
def setUp(self):
config = dict()
fdroidserver.common.find_apksigner(config)
if not config.get('apksigner'):
self.skipTest('SKIPPING, apksigner not installed!')
super().setUp()
os.chdir(self.testdir)
fdroidserver.common.write_config_file('keypass: {env: keypass}')
os.chmod(fdroidserver.common.CONFIG_FILE, 0o666) # nosec B103
def test_config_perm_no_warning(self):
fdroidserver.common.write_config_file('keystore: foo.jks')
with self.assertNoLogs(level=logging.WARNING):
fdroidserver.common.read_config()
def test_config_perm_keypass_warning(self):
fdroidserver.common.write_config_file('keypass: supersecret')
with self.assertLogs(level=logging.WARNING) as lw:
fdroidserver.common.read_config()
self.assertTrue('unsafe' in lw.output[0])
@mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
def test_config_perm_env_warning(self):
os.environ['keypass'] = 'supersecret'
fdroidserver.common.write_config_file('keypass: {env: keypass}')
with self.assertLogs(level=logging.WARNING) as lw:
fdroidserver.common.read_config()
self.assertTrue('unsafe' in lw.output[0])
self.assertEqual(1, len(lw.output))
@mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
def test_config_perm_unset_env_no_warning(self):
fdroidserver.common.write_config_file('keypass: {env: keypass}')
with self.assertLogs(level=logging.WARNING) as lw:
fdroidserver.common.read_config()
self.assertTrue('unsafe' not in lw.output[0])
self.assertEqual(1, len(lw.output))