[PATCH 1/2] libselinux: restorecon: add fallback for pre 3.6 Linux

[PATCH 1/2] libselinux: restorecon: add fallback for pre 3.6 Linux

[Christian Göttsche]

fstat(2) on file descriptors obtained via O_PATH is supported since
Linux 3.6. Fallback on older systems to lstat(2).

Fixes: 7e979b56 ("libselinux: restorecon: pin file to avoid TOCTOU issues")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/selinux_restorecon.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 9dd6be81..1a185ced 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -89,10 +89,22 @@ struct rest_flags {
 	bool count_errors;
 };
 
+/* Linux version for availability tests. */
+static struct utsname uts;
+
 static void restorecon_init(void)
 {
 	struct selabel_handle *sehandle = NULL;
 
+	if (uname(&uts) < 0) {
+		/*
+		 * utsname(2) should never fail, but assume oldest supported
+		 * LTS release as backup
+		 */
+		strncpy(uts.release, "4.9", sizeof(uts.release));
+		uts.release[sizeof(uts.release) - 1] = '\0';
+	}
+
 	if (!fc_sehandle) {
 		sehandle = selinux_restorecon_default_handle();
 		selinux_restorecon_set_sehandle(sehandle);
@@ -238,7 +250,6 @@ static uint64_t file_system_count(const char *name)
  */
 static uint64_t exclude_non_seclabel_mounts(void)
 {
-	struct utsname uts;
 	FILE *fp;
 	size_t len;
 	int index = 0, found = 0;
@@ -247,7 +258,7 @@ static uint64_t exclude_non_seclabel_mounts(void)
 	char *buf = NULL, *item;
 
 	/* Check to see if the kernel supports seclabel */
-	if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
+	if (strverscmp(uts.release, "2.6.30") < 0)
 		return 0;
 	if (is_selinux_enabled() <= 0)
 		return 0;
@@ -648,9 +659,19 @@ static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool fi
 	if (fd < 0)
 		goto err;
 
+	/*
+	 * fstat(2) on file descriptors obtained via O_PATH are supported
+	 * since Linux 3.6, see man:open(2).
+	 * Test fstat(2) first, support might have been backported.
+	 */
 	rc = fstat(fd, &stat_buf);
-	if (rc < 0)
-		goto err;
+	if (rc < 0) {
+		if (errno == EBADF && strverscmp(uts.release, "3.6") < 0)
+			rc = lstat(pathname, &stat_buf);
+
+		if (rc < 0)
+			goto err;
+	}
 
 	if (rootpath != NULL && lookup_path[0] == '\0')
 		/* this is actually the root dir of the alt root. */
-- 
2.36.1

[PATCH 2/2] libselinux: restorecon: avoid printing NULL pointer

[Christian Göttsche]

The variable `curcon` is NULL in case the file has no current security
context.  Most C standard libraries handle it fine, avoid it nonetheless
for standard conformance.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
"(null)" might not be the best token to display, it was only taken to
not change current behavior
---
 libselinux/src/selinux_restorecon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 1a185ced..1b21a605 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -771,7 +771,9 @@ static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool fi
 			selinux_log(SELINUX_INFO,
 				    "%s %s from %s to %s\n",
 				    updated ? "Relabeled" : "Would relabel",
-				    pathname, curcon, newcon);
+				    pathname,
+				    curcon ? curcon : "(null)",
+				    newcon);
 
 		if (flags->syslog_changes && !flags->nochange) {
 			if (curcon)
-- 
2.36.1

Re: [PATCH 2/2] libselinux: restorecon: avoid printing NULL pointer

[James Carter]

On Tue, May 17, 2022 at 3:20 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The variable `curcon` is NULL in case the file has no current security
> context.  Most C standard libraries handle it fine, avoid it nonetheless
> for standard conformance.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> "(null)" might not be the best token to display, it was only taken to
> not change current behavior
> ---
>  libselinux/src/selinux_restorecon.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 1a185ced..1b21a605 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -771,7 +771,9 @@ static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool fi
>                         selinux_log(SELINUX_INFO,
>                                     "%s %s from %s to %s\n",
>                                     updated ? "Relabeled" : "Would relabel",
> -                                   pathname, curcon, newcon);
> +                                   pathname,
> +                                   curcon ? curcon : "(null)",

Use "<<none>>", this is already used in file context files to indicate
a file should not have a label.

Thanks,
Jim

> +                                   newcon);
>
>                 if (flags->syslog_changes && !flags->nochange) {
>                         if (curcon)
> --
> 2.36.1
>

Re: [PATCH 1/2] libselinux: restorecon: add fallback for pre 3.6 Linux

[James Carter]

On Tue, May 17, 2022 at 12:58 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> fstat(2) on file descriptors obtained via O_PATH is supported since
> Linux 3.6. Fallback on older systems to lstat(2).
>
> Fixes: 7e979b56 ("libselinux: restorecon: pin file to avoid TOCTOU issues")
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libselinux/src/selinux_restorecon.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 9dd6be81..1a185ced 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -89,10 +89,22 @@ struct rest_flags {
>         bool count_errors;
>  };
>
> +/* Linux version for availability tests. */
> +static struct utsname uts;
> +
>  static void restorecon_init(void)
>  {
>         struct selabel_handle *sehandle = NULL;
>
> +       if (uname(&uts) < 0) {
> +               /*
> +                * utsname(2) should never fail, but assume oldest supported

Should be "uname(2) should ..."

> +                * LTS release as backup
> +                */
> +               strncpy(uts.release, "4.9", sizeof(uts.release));

I don't know. Using 4.9 seems arbitrary. I think that I would prefer
just using "3.6" and note that only behavior is only changed for
kernels prior to 3.6 or something like that.

Thanks,
Jim


> +               uts.release[sizeof(uts.release) - 1] = '\0';
> +       }
> +
>         if (!fc_sehandle) {
>                 sehandle = selinux_restorecon_default_handle();
>                 selinux_restorecon_set_sehandle(sehandle);
> @@ -238,7 +250,6 @@ static uint64_t file_system_count(const char *name)
>   */
>  static uint64_t exclude_non_seclabel_mounts(void)
>  {
> -       struct utsname uts;
>         FILE *fp;
>         size_t len;
>         int index = 0, found = 0;
> @@ -247,7 +258,7 @@ static uint64_t exclude_non_seclabel_mounts(void)
>         char *buf = NULL, *item;
>
>         /* Check to see if the kernel supports seclabel */
> -       if (uname(&uts) == 0 && strverscmp(uts.release, "2.6.30") < 0)
> +       if (strverscmp(uts.release, "2.6.30") < 0)
>                 return 0;
>         if (is_selinux_enabled() <= 0)
>                 return 0;
> @@ -648,9 +659,19 @@ static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool fi
>         if (fd < 0)
>                 goto err;
>
> +       /*
> +        * fstat(2) on file descriptors obtained via O_PATH are supported
> +        * since Linux 3.6, see man:open(2).
> +        * Test fstat(2) first, support might have been backported.
> +        */
>         rc = fstat(fd, &stat_buf);
> -       if (rc < 0)
> -               goto err;
> +       if (rc < 0) {
> +               if (errno == EBADF && strverscmp(uts.release, "3.6") < 0)
> +                       rc = lstat(pathname, &stat_buf);
> +
> +               if (rc < 0)
> +                       goto err;
> +       }
>
>         if (rootpath != NULL && lookup_path[0] == '\0')
>                 /* this is actually the root dir of the alt root. */
> --
> 2.36.1
>

Re: [PATCH 2/2] libselinux: restorecon: avoid printing NULL pointer

[Christian Göttsche]

On Wed, 18 May 2022 at 15:54, James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, May 17, 2022 at 3:20 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The variable `curcon` is NULL in case the file has no current security
> > context.  Most C standard libraries handle it fine, avoid it nonetheless
> > for standard conformance.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> > "(null)" might not be the best token to display, it was only taken to
> > not change current behavior
> > ---
> >  libselinux/src/selinux_restorecon.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > index 1a185ced..1b21a605 100644
> > --- a/libselinux/src/selinux_restorecon.c
> > +++ b/libselinux/src/selinux_restorecon.c
> > @@ -771,7 +771,9 @@ static int restorecon_sb(const char *pathname, struct rest_flags *flags, bool fi
> >                         selinux_log(SELINUX_INFO,
> >                                     "%s %s from %s to %s\n",
> >                                     updated ? "Relabeled" : "Would relabel",
> > -                                   pathname, curcon, newcon);
> > +                                   pathname,
> > +                                   curcon ? curcon : "(null)",
>
> Use "<<none>>", this is already used in file context files to indicate
> a file should not have a label.

"<<none>>" used in file contexts definitions does not mean no security
context but never relabel the existing context.
Maybe something like "<no context>" or "<empty context>"?

>
> Thanks,
> Jim
>
> > +                                   newcon);
> >
> >                 if (flags->syslog_changes && !flags->nochange) {
> >                         if (curcon)
> > --
> > 2.36.1
> >

[PATCH v2] libselinux: restorecon: avoid printing NULL pointer

[Christian Göttsche]

The variable `curcon` is NULL in case the file has no current security
context.  Most C standard libraries handle it fine, avoid it nonetheless
for standard conformance.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
   print "<no context>" instead of "(null)"
---
 libselinux/src/selinux_restorecon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 9f5b326c..3c441119 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -744,7 +744,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 			selinux_log(SELINUX_INFO,
 				    "%s %s from %s to %s\n",
 				    updated ? "Relabeled" : "Would relabel",
-				    pathname, curcon, newcon);
+				    pathname,
+				    curcon ? curcon : "<no context>",
+				    newcon);
 
 		if (flags->syslog_changes && !flags->nochange) {
 			if (curcon)
-- 
2.36.1

Re: [PATCH v2] libselinux: restorecon: avoid printing NULL pointer

[Nicolas Iooss]

On Tue, Jun 7, 2022 at 7:00 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The variable `curcon` is NULL in case the file has no current security
> context.  Most C standard libraries handle it fine, avoid it nonetheless
> for standard conformance.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Hello,
What is the status of this patch? As it looks good to me, I can merge
it if nobody has any more comments.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks,
Nicolas

> ---
> v2:
>    print "<no context>" instead of "(null)"
> ---
>  libselinux/src/selinux_restorecon.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 9f5b326c..3c441119 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -744,7 +744,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
>                         selinux_log(SELINUX_INFO,
>                                     "%s %s from %s to %s\n",
>                                     updated ? "Relabeled" : "Would relabel",
> -                                   pathname, curcon, newcon);
> +                                   pathname,
> +                                   curcon ? curcon : "<no context>",
> +                                   newcon);
>
>                 if (flags->syslog_changes && !flags->nochange) {
>                         if (curcon)
> --
> 2.36.1
>

Re: [PATCH v2] libselinux: restorecon: avoid printing NULL pointer

[James Carter]

On Tue, Jun 28, 2022 at 5:06 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Jun 7, 2022 at 7:00 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The variable `curcon` is NULL in case the file has no current security
> > context.  Most C standard libraries handle it fine, avoid it nonetheless
> > for standard conformance.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Hello,
> What is the status of this patch? As it looks good to me, I can merge
> it if nobody has any more comments.
>

This patch is fine. Patch 1 fixes a commit that has been reverted, so
it is not needed.
Thanks,
Jim


> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Thanks,
> Nicolas
>
> > ---
> > v2:
> >    print "<no context>" instead of "(null)"
> > ---
> >  libselinux/src/selinux_restorecon.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > index 9f5b326c..3c441119 100644
> > --- a/libselinux/src/selinux_restorecon.c
> > +++ b/libselinux/src/selinux_restorecon.c
> > @@ -744,7 +744,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
> >                         selinux_log(SELINUX_INFO,
> >                                     "%s %s from %s to %s\n",
> >                                     updated ? "Relabeled" : "Would relabel",
> > -                                   pathname, curcon, newcon);
> > +                                   pathname,
> > +                                   curcon ? curcon : "<no context>",
> > +                                   newcon);
> >
> >                 if (flags->syslog_changes && !flags->nochange) {
> >                         if (curcon)
> > --
> > 2.36.1
> >
>

Re: [PATCH v2] libselinux: restorecon: avoid printing NULL pointer

[Nicolas Iooss]

On Wed, Jun 29, 2022 at 9:09 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Jun 28, 2022 at 5:06 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> >
> > On Tue, Jun 7, 2022 at 7:00 PM Christian Göttsche
> > <cgzones@googlemail.com> wrote:
> > >
> > > The variable `curcon` is NULL in case the file has no current security
> > > context.  Most C standard libraries handle it fine, avoid it nonetheless
> > > for standard conformance.
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >
> > Hello,
> > What is the status of this patch? As it looks good to me, I can merge
> > it if nobody has any more comments.
> >
>
> This patch is fine. Patch 1 fixes a commit that has been reverted, so
> it is not needed.
> Thanks,
> Jim

This patch is now applied.

Thanks,
Nicolas

> > Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> >
> > Thanks,
> > Nicolas
> >
> > > ---
> > > v2:
> > >    print "<no context>" instead of "(null)"
> > > ---
> > >  libselinux/src/selinux_restorecon.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > > index 9f5b326c..3c441119 100644
> > > --- a/libselinux/src/selinux_restorecon.c
> > > +++ b/libselinux/src/selinux_restorecon.c
> > > @@ -744,7 +744,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
> > >                         selinux_log(SELINUX_INFO,
> > >                                     "%s %s from %s to %s\n",
> > >                                     updated ? "Relabeled" : "Would relabel",
> > > -                                   pathname, curcon, newcon);
> > > +                                   pathname,
> > > +                                   curcon ? curcon : "<no context>",
> > > +                                   newcon);
> > >
> > >                 if (flags->syslog_changes && !flags->nochange) {
> > >                         if (curcon)
> > > --
> > > 2.36.1
> > >
> >