Skip to content
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

Added a very simply way to backup up capabilities. #771

Merged
merged 2 commits into from Feb 17, 2016

Conversation

mattihautameki
Copy link
Contributor

Since capabilities are not backuped with tar I implemented a file based backup using getcap and setcap.This commit was tested on SLES12 and RHEL7. Both use capabilities instead of SUID for ping, arping, etc.

getcap and setcap are used to backup and restore.
setcap $cap ${TARGET_FS_ROOT}/${file}
done < <(cat $VAR_DIR/recovery/capabilities | sed 's/=//')
else
Log "setcap binary not found."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you observe this? Better rely on REQUIRED_PROGS and simply assume that it works. And if not then we should urgently fix REQUIRED_PROGS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you added setcap and getcap to the REQUIRED_PROGS array remove the if which check as it is unneeded (rest assure the test for REQUIRED_PROGS works fine).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion the REQUIRED_PROGS in prep/default/95_check_missing_programs.sh is not working correctly. Bash is not my favorite thing so maybe I am wrong.
Due to MISSING_PROGS=("") the later test -n "$MISSING_PROGS" always detects the first element as null.

[root@cen2 rear]# MISSING_PROGS=("")
[root@cen2 rear]# MISSING_PROGS=( "${MISSING_PROGS[@]}" "missing_bin" )
[root@cen2 rear]# if test -n "$MISSING_PROGS" ; then echo "something is missing" ;fi

What works for me is

if [ ${#MISSING_PROGS[@]} -ne 1 ] ; then echo "something is missing" ;fi

@schlomo
Copy link
Member

schlomo commented Feb 12, 2016

Hi @mattihautameki,

thanks a lot for this pull request! This is indeed a missing feature and thanks a lot for providing us with a first implementation.

I went through the code and left some detailed comments. IMHO only the following are real blockers:

  • rename BACKUP_CAP
  • Use quotes for setcap
  • parse the capabilities file in a more robust way, e.g. with this example: while IFS="=" read file cap ; do file="${file% }" cap="${cap# }" ; declare -p file cap ; done <<<'/some file with blanks and " = some_cap,other_cap'

Kind Regards,
Schlomo

@jsmeix
Copy link
Member

jsmeix commented Feb 15, 2016

Regarding #771 (comment) "REQUIRED_PROGS is not working correctly":

Also in my opinion REQUIRED_PROGS is not working correctly, see #755 (comment) and subsequent comments.

But I still do not know under what exact circumstances rear should fail with an error if something in REQUIRED_PROGS is missing so that I cannot decide if REQUIRED_PROGS currently works as intended or not.

Do the test for NETFS_RESTORE_CAPABILITIES with is_true.
Fixed the MISSING_PROGS check in 95_check_missing_programs.sh.
Set the capabilities in a more robust way. Hopefully the output of getcap stays constant.
Moved 41_save_capabilities.sh to rescue/NETFS because capabilities are backup up to the tar archive but are not present in the rescue iso.
@mattihautameki
Copy link
Contributor Author

Hi!
I implemented the suggested improvments to the best of my belief.

  • Changed BACKUP_CAP to NETFS_RESTORE_CAPABILITIES and moved it to the NETFS Section in default.conf.
  • Use quotes for filename and capabilities.
  • I adapted the MISSING_PROGS check in95_check_missing_programs.sh since the Error function is not called even if all binaries from REQUIRED_PROG are missing.

I also moved the block which is collecting the capabilities 41_save_capabilities.sh to the resuce section of NETFS. This is because /var/lib/rear/recovery/capabilities is not in the ISO from the actual mkbackup-run. When rear mkbackup is run a second the file will be included to the ISO because it is already present on the filesystem.
I am not familiar with the whole code of rear so let me know if there is a better way.

Kind Regards,
Markus

@gdha gdha added the enhancement Adaptions and new features label Feb 17, 2016
@gdha gdha added this to the Rear v1.18 milestone Feb 17, 2016
@gdha
Copy link
Member

gdha commented Feb 17, 2016

@mattihautameki looks OK to me. Thank you for the new (missing) feature.

gdha added a commit that referenced this pull request Feb 17, 2016
Added a very simply way to backup up capabilities.
@gdha gdha merged commit e268ea7 into rear:master Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants