[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: sed suggestion: selinux context based on symlink when using -i

From: Jakub Martisko
Subject: Re: sed suggestion: selinux context based on symlink when using -i
Date: Tue, 28 Nov 2017 14:06:40 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

As promised, here is the patch. Comments/suggestions are


On 23.11.2017 14:26, Jakub Martisko wrote:
> Hi and thank you for your reply, I'll try to provide a
> complete patch as soon as I identify the correct entry for
> the NEWS file.
> As for the --follow-symlinks option - unpatched version
> fails at the first grep in the attached test script (and
> succeeds at the second one), while the patched version
> passes both. I hope this is the expected/wanted behaviour.
> Thanks.
> Jakub
> On 22.11.2017 17:26, Jim Meyering wrote:
>> On Wed, Nov 22, 2017 at 3:23 AM, Jakub Martisko <address@hidden> wrote:
>>> On 11.1.2017 09:19, Jakub Martisko wrote:
>>>> First of all, congratulations and thanks for the GNU sed 4.3
>>>> release.
>>>> Now to the main topic. There was a suggestion (see the
>>>> discussion in [1]) to change how the sed handles the selinux
>>>> context when working with symlinked files when using the -i
>>>> option. It was suggested that the context should be based on
>>>> the link instead of the target file itself. Patch that
>>>> changes this behavior was also proposed and is attached to
>>>> this message. Any thoughts about this suggestion?
>>>> Regards,
>>>> Jakub
>>>> P.S. I hope that his is a correct list where to post this.
>>>> [1]
>> Thank you for reporting that and for taking the time to prepare a patch.
>> Are you interested in making it a complete patch?
>> Since it is a bug fix, the patch should add an entry in the NEWS file.
>> Each bug fix also deserves regression tests: tests that fail before
>> and succeed after the patch is applied.
>> Will this patch have to take into account whether the
>> --follow-symlinks command-line option is specified? (haven't looked
>> yet)
>> If so, then there should be in-place tests that exercise this, both
>> with and without that option.
>> Finally, for each bug fix, we try hard to identify the released
>> version in which the bug was introduced (mention that in NEWS), and
>> the actual commit (mention that in the commit log for the patch).
>> Thanks again for the report and patch.
>> No obligation on your part to do any of the above.
>> Just outlining what's required, in case you're inclined.
>> If you don't do it, someone else will.
>From a2f3cd0980735b4ab8513e68f276631baecb2341 Mon Sep 17 00:00:00 2001
From: Jakub Martisko <address@hidden>
Date: Thu, 23 Nov 2017 10:09:08 +0100
Subject: [PATCH] sed: fix selinux context when working inplace with symlinks

* sed/execute.exe sed -i now sets the selinux context of the file based
  on the symlink instead of its target. --follow-symlinks option remains
* testsuite/ test the above change
* credits: bug reported by Jakub Jelen
           fix proposed by Petr Lautrbach
 NEWS                         |  3 +++
 sed/execute.c                |  2 +-
 testsuite/ | 48 ++++++++++++++++++++++++++++++++++++++++++++
 testsuite/           |  1 +
 4 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100755 testsuite/

diff --git a/NEWS b/NEWS
index e9335f0..2035be9 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ GNU sed NEWS                                    -*- outline 
   [Bug introduced in the original implementation of --posix option in
+  sed -i now creates selinux context based on the context of the symlink
+  instead of the symlink target. [Bug present since at least sed-4.2]
+  sed -i --follow-symlinks remains unchanged.
 * Noteworthy changes in release 4.4 (2017-02-03) [stable]
diff --git a/sed/execute.c b/sed/execute.c
index 0d8fa2c..977f024 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -607,7 +607,7 @@ open_next_file(const char *name, struct input *input)
       if (is_selinux_enabled () > 0)
           security_context_t con;
-          if (getfilecon (input->in_file_name, &con) != -1)
+          if (lgetfilecon (input->in_file_name, &con) != -1)
               /* Save and restore the old context for the sake of w and W
                  commands.  */
diff --git a/testsuite/ b/testsuite/
new file mode 100755
index 0000000..2f4a0e6
--- /dev/null
+++ b/testsuite/
@@ -0,0 +1,48 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# GNU General Public License for more details.
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <>.
+. "${srcdir=.}/testsuite/"; path_prepend_ ./sed
+print_ver_ sed
+#Create the first file and symlink pointing at it.
+echo "Hello World" > inplace-selinux-file || framework_failure_
+ln -s ./inplace-selinux-file inplace-selinux-link || framework_failure_
+chcon -h -t user_home_t inplace-selinux-file || framework_failure_
+chcon -h -t user_tmp_t inplace-selinux-link || framework_failure_
+#Create the second file and symlink pointing at it.
+#These will be used with the --follow-symlink option.
+echo "Hello World" > inplace-selinux-file2 || framework_failure_
+ln -s ./inplace-selinux-file2 inplace-selinux-link2 || framework_failure_
+chcon -h -t user_home_t inplace-selinux-file2 || framework_failure_
+chcon -h -t user_tmp_t inplace-selinux-link2 || framework_failure_
+#Modify prepared files inplace via the symlinks
+sed -i -e "s~Hello~Hi~" inplace-selinux-link || fail=1
+sed -i --follow-symlinks -e "s~Hello~Hi~" inplace-selinux-link2 || fail=1
+#Check selinux context - the first file shoul be created with the context 
+#of the symlink...
+ls -Z inplace-selinux-link | grep  user_tmp_t || fail=1
+#...the second file should use the context of the file itself.
+ls -Z inplace-selinux-file2 | grep  user_home_t || fail=1
+Exit $fail
diff --git a/testsuite/ b/testsuite/
index e30f445..d465759 100644
--- a/testsuite/
+++ b/testsuite/
@@ -52,6 +52,7 @@ T =                                   \
   testsuite/            \
   testsuite/         \
   testsuite/  \
+  testsuite/         \
   testsuite/      \
   testsuite/            \
   testsuite/   \

reply via email to

[Prev in Thread] Current Thread [Next in Thread]