Bug 12817

Created attachment 1022 [details]
Backup from CU163

Created attachment 1023 [details]
Backup from CU164

Created attachment 1024 [details]
Backup from CU164 after leading / added back in to include file

I had submitted bug 12816 earlier which may be related.

Function 'process_includes' in backup.pl should do a

pushd /

at begin and a 

popd

at end.

This reflects the invocation of tar

tar cvfz "${filename}" -C / \
		--exclude-from="/var/ipfire/backup/exclude" \
		--exclude-from="/var/ipfire/backup/exclude.user" \
		$(process_includes "/var/ipfire/backup/include") \
		$(process_includes "/var/ipfire/backup/include.user") \
		"$@"


The tar command isn't completely symmetric, --exclude-from accepts pattern in the file, the correspondig -T option accepts filenames only.

The pushd and popd you mention are bash commands correct?

I was able to find pushd available as a perl module but not popd but both pushd and popd as bash commands.

So the commands would need to be run as system calls in perl but I think the devs prefer to avoid that as much as possible as system calls have some security weakness.

I will compare the bash and perl pushd commands and see what perl uses at the end of running a pushd

Hi,

Am 24.03.2022 um 15:55 schrieb IPFire Bugzilla:
> *Comment # 6 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817#c6> on 
> bug 12817 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817> from Adolf 
> Belka <mailto:adolf.belka@ipfire.org> *
> 
> The pushd and popd you mention are bash commands correct?
> 

Correct!

> I was able to find pushd available as a perl module but not popd but both pushd
> and popd as bash commands.
> > So the commands would need to be run as system calls in perl but I 
think the
> devs prefer to avoid that as much as possible as system calls have some
> security weakness.
> 
> I will compare the bash and perl pushd commands and see what perl uses at the
> end of running a pushd
> 

Not necessary. ;) Despite the name 'backup.pl' is a bash script. So the 
commands can be used.

Regards,
Bernhard

> ------------------------------------------------------------------------
> You are receiving this mail because:
> 
>   * You are on the CC list for the bug.
>

Thanks Bernhard. I had just seen the .pl and made the assumption (which is never good) that it was a perl file and didn't look closer at the content.

I will give it a test in my vm testbed system and report back how it goes.

It didn't work as it ended up also trying to add all the existing *.ipf backup files in /var/ipfire/backup and it should be ignoring them.

Just to check this is what I changed the function in backup.pl to. Maybe I did it incorrectly, not fully understanding this yet.

process_includes() {
        local include
    pushd /
        for include in $@; do
                local file
                while read -r file; do
                        for file in ${file}; do
                                if [ -e "/${file}" ]; then
                                        echo "${file}"
                                fi
                        done
                done < "${include}"
        done | sort -u
    popd
}

Am 24.03.2022 um 16:15 schrieb IPFire Bugzilla:
> *Comment # 9 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817#c9> on 
> bug 12817 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817> from Adolf 
> Belka <mailto:adolf.belka@ipfire.org> *
> 
> It didn't work as it ended up also trying to add all the existing *.ipf backup
> files in /var/ipfire/backup and it should be ignoring them.
> 
> Just to check this is what I changed the function in backup.pl to. Maybe I did
> it incorrectly, not fully understanding this yet.
> 
> process_includes() {
>          local include
>      pushd /
>          for include in $@; do
>                  local file
>                  while read -r file; do
>                          for file in ${file}; do
>                                  if [ -e "/${file}" ]; then
>                                          echo "${file}"
>                                  fi
>                          done
>                  done < "${include}"
>          done | sort -u
>      popd
> }
> 

This implemented right. In investigation of your report I found that 
pushd and popd output the dir stack. This shouldn't go to the file list, 
so the exact invocation should be
pushd / >/dev/null
popd >/dev/null

I've tried the modification with execution of
/var/ipfire/backup.pl list
There are no '.ipf files in the included file list.

(In reply to Bernhard Bitsch from comment #10)

> so the exact invocation should be
> pushd / >/dev/null
> popd >/dev/null
> 

I tried that in my vm testbed and the backup worked as expected.

Thanks very much Bernhard.

Will you raise a patch for this or should I do it on your behalf?

Hi Adolf,

Am 24.03.2022 um 17:51 schrieb IPFire Bugzilla:
> *Comment # 11 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817#c11> on 
> bug 12817 <https://bugzilla.ipfire.org/show_bug.cgi?id=12817> from Adolf 
> Belka <mailto:adolf.belka@ipfire.org> *
> 
> (In reply to Bernhard Bitsch fromcomment #10  <show_bug.cgi?id=12817#c10>)
> 
>> so the exact invocation should be  > pushd / >/dev/null > popd >/dev/null >
> 
> I tried that in my vm testbed and the backup worked as expected.
> 

Maybe you were in the backup directory. Thus the commands report this 
and the dir is included.

> Thanks very much Bernhard.
> 
> Will you raise a patch for this or should I do it on your behalf?
> 
I think it's just easier, if you do that. At the moment I don't have an 
environment for sending patches.
Should do it. ;)

Thx,
Bernhard

BTW:
is there a possibility to introduce this change into the running systems?
Either by replacing backup.pl or changing all entries in include and include.user to absolute paths.

This would make the 'brute-force repair method' backup-reinstall-restore functioning again.

(In reply to Bernhard Bitsch from comment #13)
> BTW:
> is there a possibility to introduce this change into the running systems?
> Either by replacing backup.pl or changing all entries in include and
> include.user to absolute paths.
> 
> This would make the 'brute-force repair method' backup-reinstall-restore
> functioning again.

This can be done but only manually by each individual as far as I can tell.

I had changed the include file back to absolute paths on my running system but now I have moved include back to the relative paths and added the pushd/popd fix into my backup.pl file and that is running well.

You may have to redo that after any Core Update until the Core Update that ends up with the patch fix on it. I suspect this will be CU166

Patch to fix this bug based on Bernhard Bitsch input has been submitted to the Dev Mailing list and patchwork.

https://patchwork.ipfire.org/project/ipfire/patch/20220325122247.7036-1-adolf.belka@ipfire.org/

@Michael has submitted an updated patch.

https://patchwork.ipfire.org/project/ipfire/patch/20220329122711.558230-1-michael.tremer@ipfire.org/

Patch has been released in CU166

https://www.ipfire.org/download/ipfire-2.27-core166

Fix confirmed on production machine and two vm's.

*** Bug 12816 has been marked as a duplicate of this bug. ***