Skip to content

Commit 5b2114b

Browse files
fmoessbauerjan-kiszka
authored andcommitted
repos: verify signatures prior to checkout
The current repo verification logic is vulnerable to TOCTOU errors, as the validation only happens after all repos have been fetched and the final configuration is built. To mitigate this, we now validate the already fetched repos prior to each checkout and prior to updates of the configuration dict. By that, we ensure that all intermediate states of the checkout are valid as well and cannot tamper the configuration which is used on the next iteration. To implement this, we move validation from a command to a library function. Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
1 parent 897dcd2 commit 5b2114b

2 files changed

Lines changed: 53 additions & 53 deletions

File tree

kas/libcmds.py

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040
from .context import get_context
4141
from .includehandler import IncludeException
4242
from .kasusererror import EnvSetButNotFoundError, ArgsCombinationError
43-
from .keyhandler import GPGKeyHandler, SSHKeyHandler
44-
from .repos import RepoRefError
43+
from .repos import SignatureValidator
4544

4645
__license__ = 'MIT'
4746
__copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
@@ -83,7 +82,6 @@ def __init__(self, use_common_setup=True):
8382
repo_loop,
8483
FinishSetupRepos(),
8584
ReposCheckout(),
86-
ReposCheckSignatures(),
8785
ReposApplyPatches(),
8886
SetupEnviron(),
8987
WriteBBConfig(),
@@ -609,7 +607,12 @@ def execute(self, ctx):
609607

610608
repos_fetch([v for k, v in ctx.missing_repos])
611609

610+
# import keys from old config and validate against this state
611+
SignatureValidator.import_keys(ctx)
612612
for _, repo in ctx.missing_repos:
613+
# check signature prior to checkout and config dict update
614+
# to avoid TOCTOU issues
615+
SignatureValidator.ensure_valid_if_signed(ctx, repo)
613616
repo.checkout()
614617

615618
ctx.config.repo_dict.update(
@@ -650,55 +653,7 @@ def __str__(self):
650653
return 'repos_checkout'
651654

652655
def execute(self, ctx):
656+
SignatureValidator.import_keys(ctx)
653657
for repo in ctx.config.get_repos():
658+
SignatureValidator.ensure_valid_if_signed(ctx, repo)
654659
repo.checkout()
655-
656-
657-
class ReposCheckSignatures(Command):
658-
"""
659-
Imports the keys defined in the configuration and checks the
660-
signatures of the repositories.
661-
"""
662-
663-
def __str__(self):
664-
return 'repos_check_signatures'
665-
666-
def execute(self, ctx):
667-
self._import_keys(ctx)
668-
self._check_signatures(ctx)
669-
670-
def _import_keys(self, ctx):
671-
handler_cfg = {
672-
'gpg': (GPGKeyHandler,
673-
Path(ctx.kas_work_dir) / '.kas_gnupg'),
674-
'ssh': (SSHKeyHandler,
675-
Path(ctx.kas_work_dir) / '.kas_ssh-handler'),
676-
}
677-
for name, (handler_cls, dir) in handler_cfg.items():
678-
signers_cfg = ctx.config.get_signers_config(name)
679-
if not signers_cfg:
680-
continue
681-
dir.mkdir(exist_ok=True)
682-
dir.chmod(0o700)
683-
ctx.managed_paths.add(dir)
684-
ctx.keyhandler[name] = handler_cls(dir, signers_cfg, ctx.config)
685-
686-
for keyhandler in ctx.keyhandler.values():
687-
ctx.environ.update(keyhandler.env)
688-
689-
def _check_signatures(self, ctx):
690-
for repo in ctx.config.get_repos():
691-
if not repo.signed:
692-
continue
693-
valid, keyid = repo.check_signature()
694-
keyhandler = ctx.keyhandler[repo.signers_type]
695-
info = keyhandler.get_key_repr(keyid) if keyid else 'No info'
696-
if valid:
697-
logging.info(f'Repository {repo.name} signature valid: {info}')
698-
continue
699-
elif keyid:
700-
raise RepoRefError(f'Repository {repo.name} is not signed '
701-
f'with a trusted key: {info}')
702-
703-
raise RepoRefError(f'Repository {repo.name} is not signed '
704-
'with a trusted key.')

kas/repos.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
from tempfile import TemporaryDirectory
3636

3737
from kas.configschema import CONFIGSCHEMA
38+
from kas.keyhandler import GPGKeyHandler, SSHKeyHandler
3839
from .context import get_context
3940
from .libkas import run_cmd_async, run_cmd
4041
from .kasusererror import KasUserError
@@ -957,3 +958,47 @@ def __lt__(self, other):
957958
return (-self.priority, self.repo_name, self.name) < \
958959
(-other.priority, other.repo_name, other.name)
959960
return NotImplemented
961+
962+
963+
class SignatureValidator:
964+
"""
965+
Handles key loading and signature validation of repos
966+
based on the configuration.
967+
"""
968+
969+
@staticmethod
970+
def import_keys(ctx):
971+
handler_cfg = {
972+
'gpg': (GPGKeyHandler,
973+
Path(ctx.kas_work_dir) / '.kas_gnupg'),
974+
'ssh': (SSHKeyHandler,
975+
Path(ctx.kas_work_dir) / '.kas_ssh-handler'),
976+
}
977+
for name, (handler_cls, dir) in handler_cfg.items():
978+
signers_cfg = ctx.config.get_signers_config(name)
979+
if not signers_cfg:
980+
continue
981+
dir.mkdir(exist_ok=True)
982+
dir.chmod(0o700)
983+
ctx.managed_paths.add(dir)
984+
ctx.keyhandler[name] = handler_cls(dir, signers_cfg, ctx.config)
985+
986+
for keyhandler in ctx.keyhandler.values():
987+
ctx.environ.update(keyhandler.env)
988+
989+
@staticmethod
990+
def ensure_valid_if_signed(ctx, repo):
991+
if not repo.signed:
992+
return
993+
valid, keyid = repo.check_signature()
994+
keyhandler = ctx.keyhandler[repo.signers_type]
995+
info = keyhandler.get_key_repr(keyid) if keyid else 'No info'
996+
if valid:
997+
logging.info(f'Repository {repo.name} signature valid: {info}')
998+
return
999+
elif keyid:
1000+
raise RepoRefError(f'Repository {repo.name} is not signed '
1001+
f'with a trusted key: {info}')
1002+
1003+
raise RepoRefError(f'Repository {repo.name} is not signed '
1004+
'with a trusted key.')

0 commit comments

Comments
 (0)