* [PATCH] mkswap: Add warnings for insecure device permissions/owners
@ 2016-01-19 18:37 Wayne R. Roth
2016-01-19 19:44 ` Mike Frysinger
2016-01-20 10:30 ` Karel Zak
0 siblings, 2 replies; 18+ messages in thread
From: Wayne R. Roth @ 2016-01-19 18:37 UTC (permalink / raw)
To: util-linux; +Cc: Wayne R. Roth
Logic copied from sys-utils/swapon.c
Signed-off-by: Wayne R. Roth <wayneroth42@gmail.com>
---
disk-utils/mkswap.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index c559e60..2729596 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -344,7 +344,7 @@ static void write_header_to_device(struct mkswap_control *ctl)
int main(int argc, char **argv)
{
struct mkswap_control ctl = { .fd = -1 };
- int c;
+ int c, permMask;
uint64_t sz;
int version = SWAP_VERSION;
char *block_count = NULL, *strsz = NULL;
@@ -464,6 +464,15 @@ int main(int argc, char **argv)
ctl.devname);
open_device(&ctl);
+ permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
+ if ((ctl.devstat.st_mode & permMask) != 0)
+ warnx(_("%s: insecure permissions %04o, %04o suggested."),
+ ctl.devname, ctl.devstat.st_mode & 07777,
+ ~permMask & 0666);
+ if (S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
+ warnx(_("%s: insecure file owner %d, 0 (root) suggested."),
+ ctl.devname, ctl.devstat.st_uid);
+
if (ctl.check)
check_blocks(&ctl);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-19 18:37 [PATCH] mkswap: Add warnings for insecure device permissions/owners Wayne R. Roth
@ 2016-01-19 19:44 ` Mike Frysinger
2016-01-20 4:17 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c Wayne R. Roth
2016-01-20 9:39 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Sami Kerola
2016-01-20 10:30 ` Karel Zak
1 sibling, 2 replies; 18+ messages in thread
From: Mike Frysinger @ 2016-01-19 19:44 UTC (permalink / raw)
To: Wayne R. Roth; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 940 bytes --]
On 19 Jan 2016 10:37, Wayne R. Roth wrote:
> + permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
> + if ((ctl.devstat.st_mode & permMask) != 0)
> + warnx(_("%s: insecure permissions %04o, %04o suggested."),
> + ctl.devname, ctl.devstat.st_mode & 07777,
> + ~permMask & 0666);
> + if (S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
> + warnx(_("%s: insecure file owner %d, 0 (root) suggested."),
> + ctl.devname, ctl.devstat.st_uid);
i haven't read/tested the code, so my assumptions might be off, but this
seems to complain even when creating files as non-root. mkswap should
not do that. a perfectly reasonable use case is to create images as a
non-root user for use with something like qemu.
maybe you want to add a getuid() check in there, or scuttle it altogether.
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c
2016-01-19 19:44 ` Mike Frysinger
@ 2016-01-20 4:17 ` Wayne R. Roth
2016-01-20 4:58 ` Mike Frysinger
2016-01-20 9:39 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Sami Kerola
1 sibling, 1 reply; 18+ messages in thread
From: Wayne R. Roth @ 2016-01-20 4:17 UTC (permalink / raw)
To: util-linux; +Cc: Wayne R. Roth
From: "Wayne R. Roth" <wayneroth42@gmail.com>
Signed-off-by: Wayne R. Roth <wayneroth42@gmail.com>
---
disk-utils/mkswap.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index c559e60..0a8dc05 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -344,7 +344,7 @@ static void write_header_to_device(struct mkswap_control *ctl)
int main(int argc, char **argv)
{
struct mkswap_control ctl = { .fd = -1 };
- int c;
+ int c, permMask;
uint64_t sz;
int version = SWAP_VERSION;
char *block_count = NULL, *strsz = NULL;
@@ -464,6 +464,15 @@ int main(int argc, char **argv)
ctl.devname);
open_device(&ctl);
+ permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
+ if ((ctl.devstat.st_mode & permMask) != 0)
+ warnx(_("%s: insecure permissions %04o, %04o suggested."),
+ ctl.devname, ctl.devstat.st_mode & 07777,
+ ~permMask & 0666);
+ if (getuid()==0 && S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
+ warnx(_("%s: insecure file owner %d, 0 (root) suggested."),
+ ctl.devname, ctl.devstat.st_uid);
+
if (ctl.check)
check_blocks(&ctl);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c
2016-01-20 4:17 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c Wayne R. Roth
@ 2016-01-20 4:58 ` Mike Frysinger
2016-01-20 6:09 ` [PATCH] mkswap: add " Wayne R. Roth
0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2016-01-20 4:58 UTC (permalink / raw)
To: Wayne R. Roth; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 294 bytes --]
On 19 Jan 2016 20:17, Wayne R. Roth wrote:
> + permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
pretty sure you want to indent w/tabs, not spaces
> + if (getuid()==0 && S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
there should be spaces around the ==
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] mkswap: add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c
2016-01-20 4:58 ` Mike Frysinger
@ 2016-01-20 6:09 ` Wayne R. Roth
2016-01-26 10:35 ` Karel Zak
0 siblings, 1 reply; 18+ messages in thread
From: Wayne R. Roth @ 2016-01-20 6:09 UTC (permalink / raw)
To: util-linux; +Cc: Wayne R. Roth
From: "Wayne R. Roth" <wayneroth42@gmail.com>
Signed-off-by: Wayne R. Roth <wayneroth42@gmail.com>
---
disk-utils/mkswap.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/disk-utils/mkswap.c b/disk-utils/mkswap.c
index c559e60..22999da 100644
--- a/disk-utils/mkswap.c
+++ b/disk-utils/mkswap.c
@@ -344,7 +344,7 @@ static void write_header_to_device(struct mkswap_control *ctl)
int main(int argc, char **argv)
{
struct mkswap_control ctl = { .fd = -1 };
- int c;
+ int c, permMask;
uint64_t sz;
int version = SWAP_VERSION;
char *block_count = NULL, *strsz = NULL;
@@ -464,6 +464,15 @@ int main(int argc, char **argv)
ctl.devname);
open_device(&ctl);
+ permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
+ if ((ctl.devstat.st_mode & permMask) != 0)
+ warnx(_("%s: insecure permissions %04o, %04o suggested."),
+ ctl.devname, ctl.devstat.st_mode & 07777,
+ ~permMask & 0666);
+ if (getuid() == 0 && S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
+ warnx(_("%s: insecure file owner %d, 0 (root) suggested."),
+ ctl.devname, ctl.devstat.st_uid);
+
if (ctl.check)
check_blocks(&ctl);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-19 19:44 ` Mike Frysinger
2016-01-20 4:17 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c Wayne R. Roth
@ 2016-01-20 9:39 ` Sami Kerola
1 sibling, 0 replies; 18+ messages in thread
From: Sami Kerola @ 2016-01-20 9:39 UTC (permalink / raw)
To: Wayne R. Roth, util-linux
On 19 January 2016 at 19:44, Mike Frysinger <vapier@gentoo.org> wrote:
> On 19 Jan 2016 10:37, Wayne R. Roth wrote:
>> + permMask = S_ISBLK(ctl.devstat.st_mode) ? 07007 : 07077;
>> + if ((ctl.devstat.st_mode & permMask) != 0)
>> + warnx(_("%s: insecure permissions %04o, %04o suggested."),
>> + ctl.devname, ctl.devstat.st_mode & 07777,
>> + ~permMask & 0666);
>> + if (S_ISREG(ctl.devstat.st_mode) && ctl.devstat.st_uid != 0)
>> + warnx(_("%s: insecure file owner %d, 0 (root) suggested."),
>> + ctl.devname, ctl.devstat.st_uid);
>
> i haven't read/tested the code, so my assumptions might be off, but this
> seems to complain even when creating files as non-root. mkswap should
> not do that. a perfectly reasonable use case is to create images as a
> non-root user for use with something like qemu.
>
> maybe you want to add a getuid() check in there, or scuttle it altogether.
I do not think mkswap, or mkfs of any sort, is an operation only root is
allowed or expected to run. Taking device(s) in use is different matter,
and that is already covered in swapon(8) and mount(8).
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-19 18:37 [PATCH] mkswap: Add warnings for insecure device permissions/owners Wayne R. Roth
2016-01-19 19:44 ` Mike Frysinger
@ 2016-01-20 10:30 ` Karel Zak
2016-01-21 22:19 ` Sarah Newman
1 sibling, 1 reply; 18+ messages in thread
From: Karel Zak @ 2016-01-20 10:30 UTC (permalink / raw)
To: Wayne R. Roth; +Cc: util-linux
On Tue, Jan 19, 2016 at 10:37:06AM -0800, Wayne R. Roth wrote:
> Logic copied from sys-utils/swapon.c
Why? I think swapon is the right place for this check.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-20 10:30 ` Karel Zak
@ 2016-01-21 22:19 ` Sarah Newman
2016-01-22 16:01 ` Tilman Schmidt
0 siblings, 1 reply; 18+ messages in thread
From: Sarah Newman @ 2016-01-21 22:19 UTC (permalink / raw)
To: Karel Zak; +Cc: Wayne R. Roth, util-linux
On 01/20/2016 02:30 AM, Karel Zak wrote:
> On Tue, Jan 19, 2016 at 10:37:06AM -0800, Wayne R. Roth wrote:
>> Logic copied from sys-utils/swapon.c
>
> Why? I think swapon is the right place for this check.
>
> Karel
>
Hi Karel,
Warnings are probably best put in *both* mkswap and swapon for the following two reasons:
1. The person(s) reviewing the output for swapon may not be the same person(s) reviewing the output for mkswap. For example, this might happen with a
company with a separate development and quality assurance department.
2. To my best knowledge the release of mkswap and swapon do not have to match. An example of when this might happen is the build process for an
embedded device or virtual machine. I am pretty sure busybox does not warn on world readable swap right now.
This patch does not break any existing behavior. The worst case possibility from accepting this patch is it will annoy some people, and best case it
will save millions of devices from being shipped with insecure permissions.
Thanks, Sarah
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-21 22:19 ` Sarah Newman
@ 2016-01-22 16:01 ` Tilman Schmidt
2016-01-22 19:14 ` Sarah Newman
0 siblings, 1 reply; 18+ messages in thread
From: Tilman Schmidt @ 2016-01-22 16:01 UTC (permalink / raw)
To: Sarah Newman; +Cc: Karel Zak, Wayne R. Roth, util-linux
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
Am 21.01.2016 um 23:19 schrieb Sarah Newman:
> This patch does not break any existing behavior. The worst case possibility from accepting this patch is it will annoy some people, and best case it
> will save millions of devices from being shipped with insecure permissions.
The worst case is it will train millions of administrators to ignore
warning messages.
--
Tilman Schmidt E-Mail: tilman@imap.cc
Bonn, Germany
Nous, on a des fleurs et des bougies pour nous protéger.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-22 16:01 ` Tilman Schmidt
@ 2016-01-22 19:14 ` Sarah Newman
2016-01-22 22:03 ` Sami Kerola
0 siblings, 1 reply; 18+ messages in thread
From: Sarah Newman @ 2016-01-22 19:14 UTC (permalink / raw)
To: Tilman Schmidt, Karel Zak; +Cc: Wayne R. Roth, util-linux
On 01/22/2016 08:01 AM, Tilman Schmidt wrote:
> Am 21.01.2016 um 23:19 schrieb Sarah Newman:
>> This patch does not break any existing behavior. The worst case possibility from accepting this patch is it will annoy some people, and best case it
>> will save millions of devices from being shipped with insecure permissions.
>
> The worst case is it will train millions of administrators to ignore
> warning messages.
>
If the warnings in swapon are legitimate, they are just as legitimate in mkswap if the file owner check is only done when mkswap is run as root.
Regarding the legitimacy of the swapon warnings: do you honestly believe most of the people who will get these warnings will have intended to have
world readable swap or swap owned as a non-root owner?
When I search for "linux swap file" on google this is the second hit for me, the first being an arch linux wiki page:
https://www.linux.com/news/software/applications/8208-all-about-linux-swap-space "centos swap file" top two hits
https://www.centos.org/docs/5/html/5.2/Deployment_Guide/s2-swap-creating-file.html
https://www.centos.org/docs/5/html/Deployment_Guide-en-US/s1-swap-adding.html
I followed the instructions for CentOS on a CentOS 5 machine and it resulted in world readable swap. Those instructions came from Red Hat. If
documentation from Red Hat gets it wrong, it's presumably a very common error. I made this mistake myself and I knew better.
--Sarah
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-22 19:14 ` Sarah Newman
@ 2016-01-22 22:03 ` Sami Kerola
2016-01-23 16:22 ` Karel Zak
0 siblings, 1 reply; 18+ messages in thread
From: Sami Kerola @ 2016-01-22 22:03 UTC (permalink / raw)
To: Sarah Newman; +Cc: Tilman Schmidt, Karel Zak, Wayne R. Roth, util-linux
On 22 January 2016 at 19:14, Sarah Newman <srn@prgmr.com> wrote:
> On 01/22/2016 08:01 AM, Tilman Schmidt wrote:
>> Am 21.01.2016 um 23:19 schrieb Sarah Newman:
>>> This patch does not break any existing behavior. The worst case possibility from accepting this patch is it will annoy some people, and best case it
>>> will save millions of devices from being shipped with insecure permissions.
>>
>> The worst case is it will train millions of administrators to ignore
>> warning messages.
>>
>
> If the warnings in swapon are legitimate, they are just as legitimate in mkswap if the file owner check is only done when mkswap is run as root.
>
> Regarding the legitimacy of the swapon warnings: do you honestly believe most of the people who will get these warnings will have intended to have
> world readable swap or swap owned as a non-root owner?
>
> When I search for "linux swap file" on google this is the second hit for me, the first being an arch linux wiki page:
> https://www.linux.com/news/software/applications/8208-all-about-linux-swap-space "centos swap file" top two hits
> https://www.centos.org/docs/5/html/5.2/Deployment_Guide/s2-swap-creating-file.html
> https://www.centos.org/docs/5/html/Deployment_Guide-en-US/s1-swap-adding.html
>
> I followed the instructions for CentOS on a CentOS 5 machine and it resulted in world readable swap. Those instructions came from Red Hat. If
> documentation from Red Hat gets it wrong, it's presumably a very common error. I made this mistake myself and I knew better.
Fair points, but should same logic be applied to file holes when
running mkswap? If the test _must_ be the same surely the mkswap &
swapon should share the function(s) performing them, as drifting would
be bad. Should these messages be optional, and if so which way around
default ought to be? This will lead to --quite or --verbose options.
Alternatively one could make swapon to get rid of all permission bits
and set ownership to UID 0 by default when ever it activates a
swapfile. How about that.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-22 22:03 ` Sami Kerola
@ 2016-01-23 16:22 ` Karel Zak
2016-01-24 11:09 ` Sami Kerola
2016-01-25 21:39 ` Sarah Newman
0 siblings, 2 replies; 18+ messages in thread
From: Karel Zak @ 2016-01-23 16:22 UTC (permalink / raw)
To: kerolasa; +Cc: Sarah Newman, Tilman Schmidt, Wayne R. Roth, util-linux
On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
> Alternatively one could make swapon to get rid of all permission bits
> and set ownership to UID 0 by default when ever it activates a
> swapfile. How about that.
Not sure if want to change any permissions on the fly, it would be
better to reject files (by swapon) with insecure permissions and
require something like --force for crazy users who wants to ignore
this problem.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-23 16:22 ` Karel Zak
@ 2016-01-24 11:09 ` Sami Kerola
2016-01-25 19:55 ` Sami Kerola
2016-01-26 10:42 ` Karel Zak
2016-01-25 21:39 ` Sarah Newman
1 sibling, 2 replies; 18+ messages in thread
From: Sami Kerola @ 2016-01-24 11:09 UTC (permalink / raw)
To: Karel Zak; +Cc: Sarah Newman, Tilman Schmidt, Wayne R. Roth, util-linux
On 23 January 2016 at 16:22, Karel Zak <kzak@redhat.com> wrote:
> On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
>> Alternatively one could make swapon to get rid of all permission bits
>> and set ownership to UID 0 by default when ever it activates a
>> swapfile. How about that.
>
> Not sure if want to change any permissions on the fly, it would be
> better to reject files (by swapon) with insecure permissions and
> require something like --force for crazy users who wants to ignore
> this problem.
Why not completely optional?
$ swapon --path-permissions [ignore|complain|stop|fix]
Current default is 'complain', and it feels about right.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-24 11:09 ` Sami Kerola
@ 2016-01-25 19:55 ` Sami Kerola
2016-01-26 10:42 ` Karel Zak
1 sibling, 0 replies; 18+ messages in thread
From: Sami Kerola @ 2016-01-25 19:55 UTC (permalink / raw)
To: Karel Zak; +Cc: Sarah Newman, Tilman Schmidt, Wayne R. Roth, util-linux
On 24 January 2016 at 11:09, Sami Kerola <kerolasa@iki.fi> wrote:
> On 23 January 2016 at 16:22, Karel Zak <kzak@redhat.com> wrote:
>> On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
>>> Alternatively one could make swapon to get rid of all permission bits
>>> and set ownership to UID 0 by default when ever it activates a
>>> swapfile. How about that.
>>
>> Not sure if want to change any permissions on the fly, it would be
>> better to reject files (by swapon) with insecure permissions and
>> require something like --force for crazy users who wants to ignore
>> this problem.
>
> Why not completely optional?
>
> $ swapon --path-permissions [ignore|complain|stop|fix]
>
> Current default is 'complain', and it feels about right.
Something like this.
https://github.com/kerolasa/lelux-utiliteetit/commit/d79b3cdbe4e61b7e10d595dec4d4e299c8300c9e
Notice that the change above is wrote on top of another change, that
added swapon control structure.
https://github.com/kerolasa/lelux-utiliteetit/commit/4d510f769de438092558f51430f3b195f328ef0e
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-24 11:09 ` Sami Kerola
2016-01-25 19:55 ` Sami Kerola
@ 2016-01-26 10:42 ` Karel Zak
2016-01-26 16:28 ` Sarah Newman
1 sibling, 1 reply; 18+ messages in thread
From: Karel Zak @ 2016-01-26 10:42 UTC (permalink / raw)
To: kerolasa; +Cc: Sarah Newman, Tilman Schmidt, Wayne R. Roth, util-linux
On Sun, Jan 24, 2016 at 11:09:47AM +0000, Sami Kerola wrote:
> On 23 January 2016 at 16:22, Karel Zak <kzak@redhat.com> wrote:
> > On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
> >> Alternatively one could make swapon to get rid of all permission bits
> >> and set ownership to UID 0 by default when ever it activates a
> >> swapfile. How about that.
> >
> > Not sure if want to change any permissions on the fly, it would be
> > better to reject files (by swapon) with insecure permissions and
> > require something like --force for crazy users who wants to ignore
> > this problem.
>
> Why not completely optional?
>
> $ swapon --path-permissions [ignore|complain|stop|fix]
I don't think we want to merge another functionality to swapon. The
warnings are enough. For the rest we have ch{own,mod}.
Let's Keep It Simple and Stupid. We all love kisses, right? :-)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-26 10:42 ` Karel Zak
@ 2016-01-26 16:28 ` Sarah Newman
0 siblings, 0 replies; 18+ messages in thread
From: Sarah Newman @ 2016-01-26 16:28 UTC (permalink / raw)
To: Karel Zak; +Cc: kerolasa, Tilman Schmidt, Wayne R. Roth, util-linux
On 01/26/2016 02:42 AM, Karel Zak wrote:
> On Sun, Jan 24, 2016 at 11:09:47AM +0000, Sami Kerola wrote:
>> On 23 January 2016 at 16:22, Karel Zak <kzak@redhat.com> wrote:
>>> On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
>>>> Alternatively one could make swapon to get rid of all permission bits
>>>> and set ownership to UID 0 by default when ever it activates a
>>>> swapfile. How about that.
>>>
>>> Not sure if want to change any permissions on the fly, it would be
>>> better to reject files (by swapon) with insecure permissions and
>>> require something like --force for crazy users who wants to ignore
>>> this problem.
>>
>> Why not completely optional?
>>
>> $ swapon --path-permissions [ignore|complain|stop|fix]
>
> I don't think we want to merge another functionality to swapon. The
> warnings are enough. For the rest we have ch{own,mod}.
>
> Let's Keep It Simple and Stupid. We all love kisses, right? :-)
Hi Karel,
Your original suggestion for swapon to require '--force' for insecure permissions seems like the most sane thing to do - it protects the user without
adding a lot of knobs. Presumably there would need to a "force" option for fstab too.
But implementing that without advance notice could lead to broken systems. Maybe it would make sense to add the --force option now and change the
warning to indicate that in future versions of swapon, insecure permissions used without --force will be rejected. Then in a couple of years actually
implement that change.
If you agree this is sane behavior appropriate for upstream, I'll get you a patch for swapon.
--Sarah
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] mkswap: Add warnings for insecure device permissions/owners
2016-01-23 16:22 ` Karel Zak
2016-01-24 11:09 ` Sami Kerola
@ 2016-01-25 21:39 ` Sarah Newman
1 sibling, 0 replies; 18+ messages in thread
From: Sarah Newman @ 2016-01-25 21:39 UTC (permalink / raw)
To: Karel Zak; +Cc: Wayne R. Roth, util-linux
On 01/23/2016 08:22 AM, Karel Zak wrote:
> On Fri, Jan 22, 2016 at 10:03:47PM +0000, Sami Kerola wrote:
>> Alternatively one could make swapon to get rid of all permission bits
>> and set ownership to UID 0 by default when ever it activates a
>> swapfile. How about that.
>
> Not sure if want to change any permissions on the fly, it would be
> better to reject files (by swapon) with insecure permissions and
> require something like --force for crazy users who wants to ignore
> this problem.
Rejecting insecure permissions in swapon will make warnings in mkswap even more important. Are there any further changes required to Wayne's latest
patch to mkswap.c https://marc.info/?l=util-linux-ng&m=145327019508709&w=2 before it is merged?
Thanks, Sarah
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-01-26 16:28 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-19 18:37 [PATCH] mkswap: Add warnings for insecure device permissions/owners Wayne R. Roth
2016-01-19 19:44 ` Mike Frysinger
2016-01-20 4:17 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Logic modified from sys-utils/swapon.c Wayne R. Roth
2016-01-20 4:58 ` Mike Frysinger
2016-01-20 6:09 ` [PATCH] mkswap: add " Wayne R. Roth
2016-01-26 10:35 ` Karel Zak
2016-01-20 9:39 ` [PATCH] mkswap: Add warnings for insecure device permissions/owners Sami Kerola
2016-01-20 10:30 ` Karel Zak
2016-01-21 22:19 ` Sarah Newman
2016-01-22 16:01 ` Tilman Schmidt
2016-01-22 19:14 ` Sarah Newman
2016-01-22 22:03 ` Sami Kerola
2016-01-23 16:22 ` Karel Zak
2016-01-24 11:09 ` Sami Kerola
2016-01-25 19:55 ` Sami Kerola
2016-01-26 10:42 ` Karel Zak
2016-01-26 16:28 ` Sarah Newman
2016-01-25 21:39 ` Sarah Newman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox