-
-
Notifications
You must be signed in to change notification settings - Fork 739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement an early-decrypting check #5171
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1386,13 +1386,9 @@ def __next__(self): | |
|
||
class ArchiveChecker: | ||
|
||
def __init__(self): | ||
self.error_found = False | ||
self.possibly_superseded = set() | ||
|
||
def check(self, repository, repair=False, archive=None, first=0, last=0, sort_by='', glob=None, | ||
def __init__(self, repository, repair=False, archive=None, first=0, last=0, sort_by='', glob=None, | ||
verify_data=False, save_space=False): | ||
"""Perform a set of checks on 'repository' | ||
"""A checker of repostiory archives | ||
|
||
:param repair: enable repair mode, write updated or corrected data into repository | ||
:param archive: only check this archive | ||
|
@@ -1401,32 +1397,45 @@ def check(self, repository, repair=False, archive=None, first=0, last=0, sort_by | |
:param verify_data: integrity verification of data referenced by archives | ||
:param save_space: Repository.commit(save_space) | ||
""" | ||
logger.info('Starting archive consistency check...') | ||
self.check_all = archive is None and not any((first, last, glob)) | ||
self.error_found = False | ||
self.verify_data = verify_data | ||
self.possibly_superseded = set() | ||
self.repair = repair | ||
self.repository = repository | ||
self.archive = archive | ||
self.first = first | ||
self.last = last | ||
self.glob = glob | ||
self.check_all = archive is None and not any((first, last, glob)) | ||
self.sort_by = sort_by | ||
self.save_space = save_space | ||
self.init_chunks() | ||
self.key = self.identify_key(repository) | ||
Comment on lines
1412
to
+1413
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. guess these 2 need checking. do these have any unwanted consequences when run before the repo check has happened? |
||
logger.info('Initialised archive consistency checker...') | ||
|
||
def check(self): | ||
"""Perform a set of checks on 'repository'""" | ||
logger.info('Starting archive consistency check...') | ||
if not self.chunks: | ||
logger.error('Repository contains no apparent data at all, cannot continue check/repair.') | ||
return False | ||
self.key = self.identify_key(repository) | ||
if verify_data: | ||
self.verify_data() | ||
if self.verify_data: | ||
self.do_verify_data() | ||
if Manifest.MANIFEST_ID not in self.chunks: | ||
logger.error("Repository manifest not found!") | ||
self.error_found = True | ||
self.manifest = self.rebuild_manifest() | ||
else: | ||
try: | ||
self.manifest, _ = Manifest.load(repository, (Manifest.Operation.CHECK,), key=self.key) | ||
self.manifest, _ = Manifest.load(self.repository, (Manifest.Operation.CHECK,), key=self.key) | ||
except IntegrityErrorBase as exc: | ||
logger.error('Repository manifest is corrupted: %s', exc) | ||
self.error_found = True | ||
del self.chunks[Manifest.MANIFEST_ID] | ||
self.manifest = self.rebuild_manifest() | ||
self.rebuild_refcounts(archive=archive, first=first, last=last, sort_by=sort_by, glob=glob) | ||
self.rebuild_refcounts(archive=self.archive, first=self.first, last=self.last, sort_by=self.sort_by, glob=self.glob) | ||
self.orphan_chunks_check() | ||
self.finish(save_space=save_space) | ||
self.finish(save_space=self.save_space) | ||
if self.error_found: | ||
logger.error('Archive consistency check complete, problems found.') | ||
else: | ||
|
@@ -1459,7 +1468,7 @@ def identify_key(self, repository): | |
cdata = repository.get(some_chunkid) | ||
return key_factory(repository, cdata) | ||
|
||
def verify_data(self): | ||
def do_verify_data(self): | ||
Comment on lines
-1462
to
+1471
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did you rename this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as we now have a boolean flag we need to save to
|
||
logger.info('Starting cryptographic data integrity verification...') | ||
chunks_count_index = len(self.chunks) | ||
chunks_count_segments = 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -327,15 +327,21 @@ def do_check(self, args, repository): | |
# also, there is no max_duration support in the archives check code anyway. | ||
self.print_error("--repository-only is required for --max-duration support.") | ||
return EXIT_ERROR | ||
checker = None | ||
if args.prefix is not None: | ||
args.glob_archives = args.prefix + '*' | ||
if not args.repo_only: | ||
try: | ||
checker = ArchiveChecker(repository, repair=args.repair, archive=args.location.archive, first=args.first, | ||
last=args.last, sort_by=args.sort_by or 'ts', glob=args.glob_archives, | ||
verify_data=args.verify_data, save_space=args.save_space) | ||
except Exception as exc: | ||
self.print_error(str(exc)) | ||
return EXIT_WARNING | ||
if not args.archives_only: | ||
if not repository.check(repair=args.repair, save_space=args.save_space, max_duration=args.max_duration): | ||
Comment on lines
+335
to
342
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the potential fundamental problem with first accessing the repo inside The repo check / repair makes sure that the fundamental data is ok (in memory and in repair mode also on disk). So, guess there needs to be pretty much review before touching repo data before the repo has been checked. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, and that's what I feared. Is there some way to verify only the chunks that contain the metadata needed to decrypt in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, a "miniature version" of the borg repo check, that only makes sure that 1 chunk is ok (which could be the manifest (id 0) or any other chunk). And then detect crypto based on that chunk. that might work, needs checking... |
||
return EXIT_WARNING | ||
if args.prefix is not None: | ||
args.glob_archives = args.prefix + '*' | ||
if not args.repo_only and not ArchiveChecker().check( | ||
repository, repair=args.repair, archive=args.location.archive, | ||
first=args.first, last=args.last, sort_by=args.sort_by or 'ts', glob=args.glob_archives, | ||
verify_data=args.verify_data, save_space=args.save_space): | ||
if not args.repo_only and not checker.check(): | ||
return EXIT_WARNING | ||
return EXIT_SUCCESS | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo