From 0845027e12bcb65389d28bb87e3a2ba5f862a78d Mon Sep 17 00:00:00 2001 From: bescoto Date: Sun, 28 Sep 2003 05:01:57 +0000 Subject: Fixed restore/regress permissions bug git-svn-id: http://svn.savannah.nongnu.org/svn/rdiff-backup/trunk@459 2b77aa54-bcbc-44c9-a7ec-4f6cf2b41109 --- rdiff-backup/CHANGELOG | 11 +++++ rdiff-backup/rdiff-backup.1 | 8 ++-- rdiff-backup/rdiff_backup/Security.py | 1 + rdiff-backup/rdiff_backup/regress.py | 17 ++++++- rdiff-backup/rdiff_backup/restore.py | 84 +++++++++++++++++++++++++++++++++-- rdiff-backup/testing/regresstest.py | 45 +++++++++++++++++++ rdiff-backup/testing/roottest.py | 46 +++++++++++++------ 7 files changed, 191 insertions(+), 21 deletions(-) diff --git a/rdiff-backup/CHANGELOG b/rdiff-backup/CHANGELOG index 93aacd6..7a7c0f1 100644 --- a/rdiff-backup/CHANGELOG +++ b/rdiff-backup/CHANGELOG @@ -7,6 +7,17 @@ violation errors. --list-changed-since and --list-at-time now work remotely. Thanks to Morten Werner Olsen for bug report. +Fixed logic bug that could make restoring extremely slow and waste +memory. Thanks for Jacques Botha for report. + +Fixed bug restoring some directories when mirror_metadata file was +missing (as when made by 0.10.x version). + +Regressing and restoring as non-root user now works on directories +that contain unreadable files and directories as long as they are +owned by that user. Bug report by Arkadiusz Miskiewicz. Hopefully +this is the last of the unreadable file bugs... + New in v0.13.2 (2003/09/16) --------------------------- diff --git a/rdiff-backup/rdiff-backup.1 b/rdiff-backup/rdiff-backup.1 index cb80810..4e14812 100644 --- a/rdiff-backup/rdiff-backup.1 +++ b/rdiff-backup/rdiff-backup.1 @@ -470,10 +470,10 @@ splits the filename into host_info::pathname. It then substitutes host_info into the remote schema, and runs the resulting command, reading its input and output. .PP -The default remote schema is 'ssh %s rdiff-backup --server' meaning if -the host_info is user@host.net, then rdiff-backup runs 'ssh -user@host.net rdiff-backup --server'. The '%s' keyword is substituted -with the host_info. Using --remote-schema, rdiff-backup can invoke an +The default remote schema is 'ssh -C %s rdiff-backup --server' where +host_info is substituted for '%s'. So if the host_info is +user@host.net, then rdiff-backup runs 'ssh user@host.net rdiff-backup +--server'. Using --remote-schema, rdiff-backup can invoke an arbitrary command in order to open up a remote pipe. For instance, .RS rdiff-backup --remote-schema 'cd /usr; %s' foo 'rdiff-backup diff --git a/rdiff-backup/rdiff_backup/Security.py b/rdiff-backup/rdiff_backup/Security.py index c3059d3..1805703 100644 --- a/rdiff-backup/rdiff_backup/Security.py +++ b/rdiff-backup/rdiff_backup/Security.py @@ -140,6 +140,7 @@ def set_allowed_requests(sec_level): "fs_abilities.get_fsabilities_restoresource", "restore.MirrorStruct.set_mirror_and_rest_times", "restore.MirrorStruct.initialize_rf_cache", + "restore.MirrorStruct.close_rf_cache", "restore.MirrorStruct.get_diffs", "backup.SourceStruct.get_source_select", "backup.SourceStruct.set_source_select", diff --git a/rdiff-backup/rdiff_backup/regress.py b/rdiff-backup/rdiff_backup/regress.py index 32fcddb..b508138 100644 --- a/rdiff-backup/rdiff_backup/regress.py +++ b/rdiff-backup/rdiff_backup/regress.py @@ -112,14 +112,29 @@ def remove_rbdir_increments(): old_current_mirror.delete() def iterate_raw_rfs(mirror_rp, inc_rp): - """Iterate all RegressFile objects in mirror/inc directory""" + """Iterate all RegressFile objects in mirror/inc directory + + Also changes permissions of unreadable files to allow access and + then changes them back later. + + """ root_rf = RegressFile(mirror_rp, inc_rp, restore.get_inclist(inc_rp)) def helper(rf): + mirror_rp = rf.mirror_rp + if (Globals.process_uid != 0 and + ((mirror_rp.isreg() and not mirror_rp.readable()) or + (mirror_rp.isdir() and not mirror_rp.hasfullperms()))): + unreadable, old_perms = 1, mirror_rp.getperms() + if mirror_rp.isreg(): mirror_rp.chmod(0400 | old_perms) + else: mirror_rp.chmod(0700 | old_perms) + else: unreadable = 0 yield rf + if unreadable and mirror_rp.isreg(): mirror_rp.chmod(old_perms) if rf.mirror_rp.isdir() or rf.inc_rp.isdir(): for sub_rf in rf.yield_sub_rfs(): for sub_sub_rf in helper(sub_rf): yield sub_sub_rf + if unreadable and mirror_rp.isdir(): mirror_rp.chmod(old_perms) return helper(root_rf) def yield_metadata(): diff --git a/rdiff-backup/rdiff_backup/restore.py b/rdiff-backup/rdiff_backup/restore.py index 2e8bf35..8d64c36 100644 --- a/rdiff-backup/rdiff_backup/restore.py +++ b/rdiff-backup/rdiff_backup/restore.py @@ -48,6 +48,7 @@ def Restore(mirror_rp, inc_rpath, target, restore_to_time): target_iter = TargetS.get_initial_iter(target) diff_iter = MirrorS.get_diffs(target_iter) TargetS.patch(target, diff_iter) + MirrorS.close_rf_cache() def get_inclist(inc_rpath): """Returns increments with given base""" @@ -87,7 +88,7 @@ def ListChangedSince(mirror_rp, inc_rp, restore_to_time): path_desc = (old_rorp and old_rorp.get_indexpath() or cur_rorp.get_indexpath()) yield rpath.RORPath(("%-7s %s" % (change, path_desc),)) - + MirrorStruct.close_rf_cache() def ListAtTime(mirror_rp, inc_rp, time): """List the files in archive at the given time @@ -158,6 +159,10 @@ class MirrorStruct: cls.root_rf = rf cls.rf_cache = CachedRF(rf) + def close_rf_cache(cls): + """Run anything remaining on CachedRF object""" + cls.rf_cache.close() + def get_mirror_rorp_iter(cls, rest_time = None, require_metadata = None): """Return iter of mirror rps at given restore time @@ -195,7 +200,9 @@ class MirrorStruct: rorp = rf.get_attribs() yield rorp if rorp.isdir(): - for sub_rf in rf.yield_sub_rfs(): yield sub_rf.get_attribs() + for sub_rf in rf.yield_sub_rfs(): + for attribs in cls.get_rorp_iter_from_rf(sub_rf): + yield attribs def subtract_indicies(cls, index, rorp_iter): """Subtract index from index of each rorp in rorp_iter @@ -295,6 +302,8 @@ class CachedRF: """Initialize CachedRF, self.rf_list variable""" self.root_rf = root_rf self.rf_list = [] # list should filled in index order + if Globals.process_uid != 0: + self.perm_changer = PermissionChanger(root_rf.mirror_rp) def list_rfs_in_cache(self, index): """Used for debugging, return indicies of cache rfs for printing""" @@ -309,7 +318,9 @@ class CachedRF: if not self.rf_list: if not self.add_rfs(index): return None rf = self.rf_list[0] - if rf.index == index: return rf + if rf.index == index: + if Globals.process_uid != 0: self.perm_changer(rf.mirror_rp) + return rf elif rf.index > index: # Try to add earlier indicies. But if first is # already from same directory, or we can't find any @@ -339,6 +350,7 @@ The cause is probably data loss from the destination directory.""" % parent_index = index[:-1] temp_rf = RestoreFile(self.root_rf.mirror_rp.new_index(parent_index), self.root_rf.inc_rp.new_index(parent_index), []) + if Globals.process_uid != 0: self.perm_changer(temp_rf.mirror_rp) new_rfs = list(temp_rf.yield_sub_rfs()) if not new_rfs: log.Log("Warning: No RFs added for index %s" % (index,), 2) @@ -346,6 +358,10 @@ The cause is probably data loss from the destination directory.""" % self.rf_list[0:0] = new_rfs return 1 + def close(self): + """Finish remaining rps in PermissionChanger""" + if Globals.process_uid != 0: self.perm_changer.finish() + class RestoreFile: """Hold data about a single mirror file and its related increments @@ -617,3 +633,65 @@ class PatchITRB(rorpiter.ITRBranch): self.base_rp.rmdir() if self.dir_replacement.lstat(): rpath.rename(self.dir_replacement, self.base_rp) + + +class PermissionChanger: + """Change the permission of mirror files and directories + + The problem is that mirror files and directories may need their + permissions changed in order to be read and listed, and then + changed back when we are done. This class hooks into the CachedRF + object to know when an rp is needed. + + """ + def __init__(self, root_rp): + self.root_rp = root_rp + self.current_index = () + # Below is a list of (index, rp, old_perm) triples in reverse + # order that need clearing + self.open_index_list = [] + + def __call__(self, rp): + """Given rpath, change permissions up and including rp""" + index, old_index = rp.index, self.current_index + self.current_index = index + if not index or index == old_index: return + assert index > old_index, (index, old_index) + self.restore_old(rp, index) + self.add_new(rp, old_index, index) + + def restore_old(self, rp, index): + """Restore permissions for indicies we are done with""" + while self.open_index_list: + old_index, old_rp, old_perms = self.open_index_list[0] + if index[:len(old_index)] > old_index: old_rp.chmod(old_perms) + else: break + del self.open_index_list[0] + + def add_new(self, rp, old_index, index): + """Change permissions of directories between old_index and index""" + for rp in self.get_new_rp_list(rp, old_index, index): + if ((rp.isreg() and not rp.readable()) or + (rp.isdir() and not rp.hasfullperms())): + old_perms = rp.getperms() + self.open_index_list.insert(0, (index, rp, old_perms)) + if rp.isreg(): rp.chmod(0400 | old_perms) + else: rp.chmod(0700 | old_perms) + + def get_new_rp_list(self, rp, old_index, index): + """Return list of new rp's between old_index and index""" + for i in range(len(index)-1, -1, -1): + if old_index[:i] == index[:i]: + common_prefix_len = i + break + else: assert 0 + + new_rps = [] + for total_len in range(common_prefix_len+1, len(index)): + new_rps.append(self.root_rp.new_index(index[:total_len])) + new_rps.append(rp) + return new_rps + + def finish(self): + """Restore any remaining rps""" + for index, rp, perms in self.open_index_list: rp.chmod(perms) diff --git a/rdiff-backup/testing/regresstest.py b/rdiff-backup/testing/regresstest.py index be02e67..a94f4ae 100644 --- a/rdiff-backup/testing/regresstest.py +++ b/rdiff-backup/testing/regresstest.py @@ -93,4 +93,49 @@ class RegressTest(unittest.TestCase): """Run regress test remotely""" self.runtest(self.regress_to_time_remote) + def test_unreadable(self): + """Run regress test when regular file is unreadable""" + self.output_rp.setdata() + if self.output_rp.lstat(): Myrm(self.output_rp.path) + unreadable_rp = self.make_unreadable() + + rdiff_backup(1, 1, unreadable_rp.path, self.output_rp.path, + current_time = 1) + rbdir = self.output_rp.append('rdiff-backup-data') + marker = rbdir.append('current_mirror.2000-12-31T21:33:20-07:00.data') + marker.touch() + self.change_unreadable() + + cmd = "rdiff-backup --check-destination-dir " + self.output_rp.path + print "Executing:", cmd + assert not os.system(cmd) + + def make_unreadable(self): + """Make unreadable input directory + + The directory needs to be readable initially (otherwise it + just won't get backed up, and then later we will turn it + unreadable. + + """ + rp = rpath.RPath(Globals.local_connection, "testfiles/regress") + if rp.lstat(): Myrm(rp.path) + rp.setdata() + rp.mkdir() + rp1 = rp.append('unreadable_dir') + rp1.mkdir() + rp1_1 = rp1.append('to_be_unreadable') + rp1_1.write_string('aensuthaoeustnahoeu') + return rp + + def change_unreadable(self): + """Change attributes in directory, so regress will request fp""" + subdir = self.output_rp.append('unreadable_dir') + assert subdir.lstat() + filerp = subdir.append('to_be_unreadable') + filerp.chmod(0) + subdir.chmod(0) + + if __name__ == "__main__": unittest.main() + diff --git a/rdiff-backup/testing/roottest.py b/rdiff-backup/testing/roottest.py index 176d4ad..a498ff2 100644 --- a/rdiff-backup/testing/roottest.py +++ b/rdiff-backup/testing/roottest.py @@ -11,7 +11,7 @@ if you aren't me, check out the 'user' global variable. Globals.set('change_source_perms', None) Globals.counter = 0 -verbosity = 6 +verbosity = 5 log.Log.setverbosity(verbosity) user = 'ben' # Non-root user to su to userid = 500 # id of user above @@ -95,6 +95,13 @@ class HalfRoot(unittest.TestCase): rp1_3.mkdir() rp1_3_1 = rp1_3.append('file_inside') rp1_3_1.write_string('blah') + rp1_3_1.chmod(0) + rp1_3_2 = rp1_3.append('subdir_inside') + rp1_3_2.mkdir() + rp1_3_2_1 = rp1_3_2.append('foo') + rp1_3_2_1.write_string('saotnhu') + rp1_3_2_1.chmod(0) + rp1_3_2.chmod(0) rp1_3.chmod(0) rp2 = rpath.RPath(Globals.local_connection, "testfiles/root_half2") @@ -105,8 +112,17 @@ class HalfRoot(unittest.TestCase): rp2_1.chmod(0) rp2_3 = rp2.append('unreadable_dir') rp2_3.mkdir() - rp2_3_2 = rp2_3.append('file2') - rp2_3_2.touch() + rp2_3_1 = rp2_3.append('file_inside') + rp2_3_1.write_string('new string') + rp2_3_1.chmod(0) + rp2_3_2 = rp2_3.append('subdir_inside') + rp2_3_2.mkdir() + rp2_3_2_1 = rp2_3_2.append('foo') + rp2_3_2_1.write_string('asoetn;oet') + rp2_3_2_1.chmod(0) + rp2_3_2.chmod(0) + rp2_3_3 = rp2_3.append('file2') + rp2_3_3.touch() rp2_3.chmod(0) return rp1, rp2 @@ -135,23 +151,27 @@ class HalfRoot(unittest.TestCase): rout_rp = rpath.RPath(Globals.local_connection, "testfiles/restore_out") + restore_schema = ("rdiff-backup -v" + str(verbosity) + + " -r %s --remote-schema '%%s' '%s'::%s %s") Myrm(rout_rp.path) - cmd3 = cmd_schema % (10000, - "-r 10000 %s/unreadable_dir" % (outrp.path,), - remote_schema, rout_rp.path) + cmd3 = restore_schema % (10000, remote_schema, outrp.path, + rout_rp.path) print "Executing restore: ", cmd3 assert not os.system(cmd3) - rout_rp.setdata() - assert rout_rp.getperms() == 0, rout_rp.getperms() + rout_perms = rout_rp.append('unreadable_dir').getperms() + outrp_perms = outrp.append('unreadable_dir').getperms() + assert rout_perms == 0, rout_perms + assert outrp_perms == 0, outrp_perms Myrm(rout_rp.path) - cmd4 = cmd_schema % (10000, - "-r now %s/unreadable_dir" % (outrp.path,), - remote_schema, rout_rp.path) + cmd4 = restore_schema % ("now", remote_schema, outrp.path, + rout_rp.path) print "Executing restore: ", cmd4 assert not os.system(cmd4) - rout_rp.setdata() - assert rout_rp.getperms() == 0, rout_rp.getperms() + rout_perms = rout_rp.append('unreadable_dir').getperms() + outrp_perms = outrp.append('unreadable_dir').getperms() + assert rout_perms == 0, rout_perms + assert outrp_perms == 0, outrp_perms class NonRoot(unittest.TestCase): -- cgit v1.2.1