* [RFC PATCH] fixfiles: correctly restore context of mountpoints @ 2020-06-30 14:59 bauen1 2020-07-06 18:25 ` Stephen Smalley 0 siblings, 1 reply; 7+ messages in thread From: bauen1 @ 2020-06-30 14:59 UTC (permalink / raw) To: selinux By bind mounting every filesystem we want to relabel we can access all files without anything hidden due to active mounts. This comes at the cost of user experience, because setfiles only displays the percentage if no path is given or the path is / Signed-off-by: bauen1 <j2468h@gmail.com> --- policycoreutils/scripts/fixfiles | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles index 5d777034..dc5be195 100755 --- a/policycoreutils/scripts/fixfiles +++ b/policycoreutils/scripts/fixfiles @@ -243,7 +243,19 @@ case "$RESTORE_MODE" in if [ -n "${FILESYSTEMSRW}" ]; then LogReadOnly echo "${OPTION}ing `echo ${FILESYSTEMSRW}`" - ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} ${FILESYSTEMSRW} + + # we bind mount so we can fix the labels of files that have already been + # mounted over + for m in `echo $FILESYSTEMSRW`; do + TMP_MOUNT="$(mktemp -d)" + test -z ${TMP_MOUNT+x} && echo "Unable to find temporary directory!" && exit 1 + + mkdir -p "${TMP_MOUNT}${m}" || exit 1 + mount --bind "${m}" "${TMP_MOUNT}${m}" || exit 1 + ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} -r "${TMP_MOUNT}" "${TMP_MOUNT}${m}" + umount "${TMP_MOUNT}${m}" || exit 1 + rm -rf "${TMP_MOUNT}" || echo "Error cleaning up." + done; else echo >&2 "fixfiles: No suitable file systems found" fi -- 2.27.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] fixfiles: correctly restore context of mountpoints 2020-06-30 14:59 [RFC PATCH] fixfiles: correctly restore context of mountpoints bauen1 @ 2020-07-06 18:25 ` Stephen Smalley 2020-07-06 19:16 ` bauen1 2020-08-06 14:48 ` [RFC PATCH v2] " bauen1 0 siblings, 2 replies; 7+ messages in thread From: Stephen Smalley @ 2020-07-06 18:25 UTC (permalink / raw) To: bauen1; +Cc: selinux On Tue, Jun 30, 2020 at 11:01 AM bauen1 <j2468h@googlemail.com> wrote: > > By bind mounting every filesystem we want to relabel we can access all > files without anything hidden due to active mounts. > > This comes at the cost of user experience, because setfiles only > displays the percentage if no path is given or the path is / Perhaps this should be opt-in via a new command-line option rather than the default, given the user-visible difference in behavior and the potential for something to go wrong for existing users. We might also want to look at improving setfiles / selinux_restorecon() to support percentage progress without this limitation. > > Signed-off-by: bauen1 <j2468h@gmail.com> Generally I think a real name is required for Signed-off-by lines in the DCO since otherwise it isn't truly meaningful from a legal perspective. > --- > policycoreutils/scripts/fixfiles | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles > index 5d777034..dc5be195 100755 > --- a/policycoreutils/scripts/fixfiles > +++ b/policycoreutils/scripts/fixfiles > @@ -243,7 +243,19 @@ case "$RESTORE_MODE" in > if [ -n "${FILESYSTEMSRW}" ]; then > LogReadOnly > echo "${OPTION}ing `echo ${FILESYSTEMSRW}`" > - ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} ${FILESYSTEMSRW} > + > + # we bind mount so we can fix the labels of files that have already been > + # mounted over > + for m in `echo $FILESYSTEMSRW`; do > + TMP_MOUNT="$(mktemp -d)" > + test -z ${TMP_MOUNT+x} && echo "Unable to find temporary directory!" && exit 1 > + > + mkdir -p "${TMP_MOUNT}${m}" || exit 1 > + mount --bind "${m}" "${TMP_MOUNT}${m}" || exit 1 > + ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} -r "${TMP_MOUNT}" "${TMP_MOUNT}${m}" > + umount "${TMP_MOUNT}${m}" || exit 1 > + rm -rf "${TMP_MOUNT}" || echo "Error cleaning up." > + done; > else > echo >&2 "fixfiles: No suitable file systems found" > fi > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] fixfiles: correctly restore context of mountpoints 2020-07-06 18:25 ` Stephen Smalley @ 2020-07-06 19:16 ` bauen1 2020-07-06 19:48 ` Stephen Smalley 2020-08-06 14:48 ` [RFC PATCH v2] " bauen1 1 sibling, 1 reply; 7+ messages in thread From: bauen1 @ 2020-07-06 19:16 UTC (permalink / raw) To: Stephen Smalley, bauen1; +Cc: selinux Thank you for reviewing: On 7/6/20 8:25 PM, Stephen Smalley wrote: > On Tue, Jun 30, 2020 at 11:01 AM bauen1 <j2468h@googlemail.com> wrote: >> >> By bind mounting every filesystem we want to relabel we can access all >> files without anything hidden due to active mounts. >> >> This comes at the cost of user experience, because setfiles only >> displays the percentage if no path is given or the path is / > > Perhaps this should be opt-in via a new command-line option rather > than the default, given the user-visible difference in behavior and > the potential for something to go wrong for existing users. We might > also want to look at improving setfiles / selinux_restorecon() to > support percentage progress without this limitation. I would argue that the new behavior is in theory "better" and allows removing a few questionable mounton allow rules from policies. If a user has files in a directory that was mounted over it could lead to surprises, so keeping a backwards compatible behavior is probably preferable. I will implement a new command-line option for it Fixing selinux_restorecon() to display the correct percentage is just a matter of improving it to check if the relabel targets the root of a mounted filesystem instead of the currently hard coded "/" (I think). >> >> Signed-off-by: bauen1 <j2468h@gmail.com> > > Generally I think a real name is required for Signed-off-by lines in > the DCO since otherwise it isn't truly meaningful from a legal > perspective. I've now also read the guide on submitting patches to the linux kernel. What would be the best way to go about adding my real name and email address while also keeping my pseudonym and email in the commit, e.g. would just replacing the Signed-off-by with my real name and email address work ? -- bauen1 https://dn42.bauen1.xyz/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] fixfiles: correctly restore context of mountpoints 2020-07-06 19:16 ` bauen1 @ 2020-07-06 19:48 ` Stephen Smalley 0 siblings, 0 replies; 7+ messages in thread From: Stephen Smalley @ 2020-07-06 19:48 UTC (permalink / raw) To: bauen1; +Cc: selinux On Mon, Jul 6, 2020 at 3:16 PM bauen1 <j2468h@googlemail.com> wrote: > > Thank you for reviewing: > > On 7/6/20 8:25 PM, Stephen Smalley wrote: > > On Tue, Jun 30, 2020 at 11:01 AM bauen1 <j2468h@googlemail.com> wrote: > >> > >> By bind mounting every filesystem we want to relabel we can access all > >> files without anything hidden due to active mounts. > >> > >> This comes at the cost of user experience, because setfiles only > >> displays the percentage if no path is given or the path is / > > > > Perhaps this should be opt-in via a new command-line option rather > > than the default, given the user-visible difference in behavior and > > the potential for something to go wrong for existing users. We might > > also want to look at improving setfiles / selinux_restorecon() to > > support percentage progress without this limitation. > > I would argue that the new behavior is in theory "better" and allows removing a few questionable mounton allow rules from policies. If a user has files in a directory that was mounted over it could lead to surprises, so keeping a backwards compatible behavior is probably preferable. I will implement a new command-line option for it > > Fixing selinux_restorecon() to display the correct percentage is just a matter of improving it to check if the relabel targets the root of a mounted filesystem instead of the currently hard coded "/" (I think). > > >> > >> Signed-off-by: bauen1 <j2468h@gmail.com> > > > > Generally I think a real name is required for Signed-off-by lines in > > the DCO since otherwise it isn't truly meaningful from a legal > > perspective. > > I've now also read the guide on submitting patches to the linux kernel. What would be the best way to go about adding my real name and email address while also keeping my pseudonym and email in the commit, e.g. would just replacing the Signed-off-by with my real name and email address work ? I think the important part is that you use your real (legal) name in the Signed-off-by line. You can use whatever email address you like in the Signed-off-by line (as long as you can in fact receive email sent there), and that need not match the email address in the From header. Of course, IANAL and others may disagree. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH v2] fixfiles: correctly restore context of mountpoints 2020-07-06 18:25 ` Stephen Smalley 2020-07-06 19:16 ` bauen1 @ 2020-08-06 14:48 ` bauen1 2020-08-07 15:05 ` Stephen Smalley 1 sibling, 1 reply; 7+ messages in thread From: bauen1 @ 2020-08-06 14:48 UTC (permalink / raw) To: selinux By bind mounting every filesystem we want to relabel we can access all files without anything hidden due to active mounts. This comes at the cost of user experience, because setfiles only displays the percentage if no path is given or the path is / Signed-off-by: Jonathan Hettwer <j2468h@gmail.com> --- policycoreutils/scripts/fixfiles | 29 +++++++++++++++++++++++++---- policycoreutils/scripts/fixfiles.8 | 8 ++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/policycoreutils/scripts/fixfiles b/policycoreutils/scripts/fixfiles index 5d777034..30dadb4f 100755 --- a/policycoreutils/scripts/fixfiles +++ b/policycoreutils/scripts/fixfiles @@ -112,6 +112,7 @@ FORCEFLAG="" RPMFILES="" PREFC="" RESTORE_MODE="" +BIND_MOUNT_FILESYSTEMS="" SETFILES=/sbin/setfiles RESTORECON=/sbin/restorecon FILESYSTEMSRW=`get_rw_labeled_mounts` @@ -243,7 +244,23 @@ case "$RESTORE_MODE" in if [ -n "${FILESYSTEMSRW}" ]; then LogReadOnly echo "${OPTION}ing `echo ${FILESYSTEMSRW}`" - ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} ${FILESYSTEMSRW} + + if [ -z "$BIND_MOUNT_FILESYSTEMS" ]; then + ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} ${FILESYSTEMSRW} + else + # we bind mount so we can fix the labels of files that have already been + # mounted over + for m in `echo $FILESYSTEMSRW`; do + TMP_MOUNT="$(mktemp -d)" + test -z ${TMP_MOUNT+x} && echo "Unable to find temporary directory!" && exit 1 + + mkdir -p "${TMP_MOUNT}${m}" || exit 1 + mount --bind "${m}" "${TMP_MOUNT}${m}" || exit 1 + ${SETFILES} ${VERBOSE} ${EXCLUDEDIRS} ${FORCEFLAG} $* -q ${FC} -r "${TMP_MOUNT}" "${TMP_MOUNT}${m}" + umount "${TMP_MOUNT}${m}" || exit 1 + rm -rf "${TMP_MOUNT}" || echo "Error cleaning up." + done; + fi else echo >&2 "fixfiles: No suitable file systems found" fi @@ -313,6 +330,7 @@ case "$1" in > /.autorelabel || exit $? [ -z "$FORCEFLAG" ] || echo -n "$FORCEFLAG " >> /.autorelabel [ -z "$BOOTTIME" ] || echo -N $BOOTTIME >> /.autorelabel + [ -z "$BIND_MOUNT_FILESYSTEMS" ] || echo "-M" >> /.autorelabel # Force full relabel if SELinux is not enabled selinuxenabled || echo -F > /.autorelabel echo "System will relabel on next boot" @@ -324,7 +342,7 @@ esac } usage() { echo $""" -Usage: $0 [-v] [-F] [-f] relabel +Usage: $0 [-v] [-F] [-M] [-f] relabel or Usage: $0 [-v] [-F] [-B | -N time ] { check | restore | verify } or @@ -334,7 +352,7 @@ Usage: $0 [-v] [-F] -R rpmpackage[,rpmpackage...] { check | restore | verify } or Usage: $0 [-v] [-F] -C PREVIOUS_FILECONTEXT { check | restore | verify } or -Usage: $0 [-F] [-B] onboot +Usage: $0 [-F] [-M] [-B] onboot """ } @@ -353,7 +371,7 @@ set_restore_mode() { } # See how we were called. -while getopts "N:BC:FfR:l:v" i; do +while getopts "N:BC:FfR:l:vM" i; do case "$i" in B) BOOTTIME=`/bin/who -b | awk '{print $3}'` @@ -379,6 +397,9 @@ while getopts "N:BC:FfR:l:v" i; do echo "Redirecting output to $OPTARG" exec >>"$OPTARG" 2>&1 ;; + M) + BIND_MOUNT_FILESYSTEMS="-M" + ;; F) FORCEFLAG="-F" ;; diff --git a/policycoreutils/scripts/fixfiles.8 b/policycoreutils/scripts/fixfiles.8 index 9f447f03..12342530 100644 --- a/policycoreutils/scripts/fixfiles.8 +++ b/policycoreutils/scripts/fixfiles.8 @@ -6,7 +6,7 @@ fixfiles \- fix file SELinux security contexts. .na .B fixfiles -.I [\-v] [\-F] [\-f] relabel +.I [\-v] [\-F] [-M] [\-f] relabel .B fixfiles .I [\-v] [\-F] { check | restore | verify } dir/file ... @@ -21,7 +21,7 @@ fixfiles \- fix file SELinux security contexts. .I [\-v] [\-F] \-C PREVIOUS_FILECONTEXT { check | restore | verify } .B fixfiles -.I [-F] [-B] onboot +.I [-F] [-M] [-B] onboot .ad @@ -68,6 +68,10 @@ Run a diff on the PREVIOUS_FILECONTEXT file to the currently installed one, and Only act on files created after the specified date. Date must be specified in "YYYY\-MM\-DD HH:MM" format. Date field will be passed to find \-\-newermt command. +.TP +.B \-M +Bind mount filesystems before relabeling them, this allows fixing the context of files or directories that have been mounted over. + .TP .B -v Modify verbosity from progress to verbose. (Run restorecon with \-v instead of \-p) -- 2.28.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] fixfiles: correctly restore context of mountpoints 2020-08-06 14:48 ` [RFC PATCH v2] " bauen1 @ 2020-08-07 15:05 ` Stephen Smalley 2020-08-17 15:57 ` Stephen Smalley 0 siblings, 1 reply; 7+ messages in thread From: Stephen Smalley @ 2020-08-07 15:05 UTC (permalink / raw) To: bauen1, selinux On 8/6/20 10:48 AM, bauen1 wrote: > By bind mounting every filesystem we want to relabel we can access all > files without anything hidden due to active mounts. > > This comes at the cost of user experience, because setfiles only > displays the percentage if no path is given or the path is / > > Signed-off-by: Jonathan Hettwer <j2468h@gmail.com> This looks good to me and running fixfiles -vM relabel appeared to work as expected, and a subsequent fixfiles -v relabel didn't show any changes so it seems to yield a consistent result. Would welcome comments from others though since fixfiles has always been somewhat magical / obscure to me and I am not much of a shell programmer. Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2] fixfiles: correctly restore context of mountpoints 2020-08-07 15:05 ` Stephen Smalley @ 2020-08-17 15:57 ` Stephen Smalley 0 siblings, 0 replies; 7+ messages in thread From: Stephen Smalley @ 2020-08-17 15:57 UTC (permalink / raw) To: bauen1, selinux On 8/7/20 11:05 AM, Stephen Smalley wrote: > On 8/6/20 10:48 AM, bauen1 wrote: > >> By bind mounting every filesystem we want to relabel we can access all >> files without anything hidden due to active mounts. >> >> This comes at the cost of user experience, because setfiles only >> displays the percentage if no path is given or the path is / >> >> Signed-off-by: Jonathan Hettwer <j2468h@gmail.com> > > This looks good to me and running fixfiles -vM relabel appeared to > work as expected, and a subsequent fixfiles -v relabel didn't show any > changes so it seems to yield a consistent result. Would welcome > comments from others though since fixfiles has always been somewhat > magical / obscure to me and I am not much of a shell programmer. > > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> Applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-17 18:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-30 14:59 [RFC PATCH] fixfiles: correctly restore context of mountpoints bauen1 2020-07-06 18:25 ` Stephen Smalley 2020-07-06 19:16 ` bauen1 2020-07-06 19:48 ` Stephen Smalley 2020-08-06 14:48 ` [RFC PATCH v2] " bauen1 2020-08-07 15:05 ` Stephen Smalley 2020-08-17 15:57 ` Stephen Smalley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).