All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add a --mode option to chmod the mount point of the maps
@ 2015-09-13 13:56 Cyril B.
  2015-09-14  2:31 ` Ian Kent
  2015-09-14  3:05 ` Ian Kent
  0 siblings, 2 replies; 12+ messages in thread
From: Cyril B. @ 2015-09-13 13:56 UTC (permalink / raw
  To: autofs@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

Hello,

It looks like the mount point of the maps have fixed permissions, 755. I 
need to have different permissions: in my use case, I want /home (which 
is handled by autofs) to be set to 751.

The initial permissions of /home are overwritten when autofs is started, 
so changing those doesn't help.

I can change the permissions of /home after autofs has started, but it's 
really not convenient: there's no easy way to do that automatically with 
either sysvinit or systemd.

For instance, with systemd, I could create a new service that depends on 
autofs and does the chmod, but it will be started once automount has 
been started, and there's guarantee it's already mounted the maps. And 
if it's started after the mounts, there's a small period of time when 
the permissions would be incorrect.

My solution was to add a --mode option to autofs, with the included 
patch. I'm not familiar with autofs's code or even Lex and Yacc, so my 
code is probably more of a proof of concept. It seems to work fine in my 
use case, though.

-- 
Cyril B.

[-- Attachment #2: add_mode.patch --]
[-- Type: text/plain, Size: 5399 bytes --]

diff --git a/daemon/direct.c b/daemon/direct.c
index 5569299..0ddd4be 100644
--- a/daemon/direct.c
+++ b/daemon/direct.c
@@ -433,6 +433,10 @@ int do_mount_autofs_direct(struct autofs_point *ap,
 		goto out_umount;
 	}
 
+	if (ap->mode != -1) {
+		chmod(me->key, ap->mode);
+	}
+
 	ops->open(ap->logopt, &ioctlfd, st.st_dev, me->key);
 	if (ioctlfd < 0) {
 		crit(ap->logopt, "failed to create ioctl fd for %s", me->key);
diff --git a/daemon/indirect.c b/daemon/indirect.c
index a04a624..3fa0659 100644
--- a/daemon/indirect.c
+++ b/daemon/indirect.c
@@ -163,6 +163,10 @@ static int do_mount_autofs_indirect(struct autofs_point *ap, const char *root)
 		goto out_umount;
 	}
 
+	if (ap->mode != -1) {
+		chmod(root, ap->mode);
+	}
+
 	if (ops->open(ap->logopt, &ap->ioctlfd, st.st_dev, root)) {
 		crit(ap->logopt,
 		     "failed to create ioctl fd for autofs path %s", ap->path);
diff --git a/include/automount.h b/include/automount.h
index 447aba1..15cd436 100644
--- a/include/automount.h
+++ b/include/automount.h
@@ -492,6 +492,7 @@ struct kernel_mod_version {
 struct autofs_point {
 	pthread_t thid;
 	char *path;			/* Mount point name */
+	mode_t mode;			/* Mount point mode */
 	char *pref;			/* amd prefix */
 	int pipefd;			/* File descriptor for pipe */
 	int kpipefd;			/* Kernel end descriptor for pipe */
diff --git a/lib/master.c b/lib/master.c
index 6c38b1c..67b25f2 100644
--- a/lib/master.c
+++ b/lib/master.c
@@ -129,6 +129,7 @@ int master_add_autofs_point(struct master_mapent *entry, unsigned logopt,
 		free(ap);
 		return 0;
 	}
+	ap->mode = -1;
 
 	entry->ap = ap;
 
diff --git a/lib/master_parse.y b/lib/master_parse.y
index 9da78fc..71d84d8 100644
--- a/lib/master_parse.y
+++ b/lib/master_parse.y
@@ -63,6 +63,7 @@ static unsigned ghost;
 extern unsigned global_selection_options;
 static unsigned random_selection;
 static unsigned use_weight;
+static mode_t mode;
 static char **tmp_argv;
 static int tmp_argc;
 static char **local_argv;
@@ -101,7 +102,7 @@ static int master_fprintf(FILE *, char *, ...);
 %token COMMENT
 %token MAP
 %token OPT_TIMEOUT OPT_NTIMEOUT OPT_NOBIND OPT_NOGHOST OPT_GHOST OPT_VERBOSE
-%token OPT_DEBUG OPT_RANDOM OPT_USE_WEIGHT OPT_SYMLINK
+%token OPT_DEBUG OPT_RANDOM OPT_USE_WEIGHT OPT_SYMLINK OPT_MODE
 %token COLON COMMA NL DDASH
 %type <strtype> map
 %type <strtype> options
@@ -126,6 +127,7 @@ static int master_fprintf(FILE *, char *, ...);
 %token <strtype> MAPXFN
 %token <strtype> MAPNAME
 %token <longtype> NUMBER
+%token <longtype> OCTALNUMBER
 %token <strtype> OPTION
 
 %start file
@@ -192,6 +194,7 @@ line:
 	| PATH OPT_GHOST { master_notify($1); YYABORT; }
 	| PATH OPT_NOGHOST { master_notify($1); YYABORT; }
 	| PATH OPT_VERBOSE { master_notify($1); YYABORT; }
+	| PATH OPT_MODE { master_notify($1); YYABORT; }
 	| PATH { master_notify($1); YYABORT; }
 	| QUOTE { master_notify($1); YYABORT; }
 	| OPTION { master_notify($1); YYABORT; }
@@ -576,6 +579,7 @@ daemon_option: OPT_TIMEOUT NUMBER { timeout = $2; }
 	| OPT_DEBUG	{ debug = 1; }
 	| OPT_RANDOM	{ random_selection = 1; }
 	| OPT_USE_WEIGHT { use_weight = 1; }
+	| OPT_MODE OCTALNUMBER { mode = $2; }
 	;
 
 mount_option: OPTION
@@ -644,6 +648,7 @@ static void local_init_vars(void)
 	ghost = defaults_get_browse_mode();
 	random_selection = global_selection_options & MOUNT_FLAG_RANDOM_SELECT;
 	use_weight = 0;
+	mode = -1;
 	tmp_argv = NULL;
 	tmp_argc = 0;
 	local_argv = NULL;
@@ -847,6 +852,9 @@ int master_parse_entry(const char *buffer, unsigned int default_timeout, unsigne
 		entry->ap->flags |= MOUNT_FLAG_SYMLINK;
 	if (negative_timeout)
 		entry->ap->negative_timeout = negative_timeout;
+	if (mode != -1) {
+		entry->ap->mode = mode;
+	}
 
 /*
 	source = master_find_map_source(entry, type, format,
diff --git a/lib/master_tok.l b/lib/master_tok.l
index c692e14..ff1c347 100644
--- a/lib/master_tok.l
+++ b/lib/master_tok.l
@@ -84,7 +84,7 @@ unsigned int tlen;
 
 %option nounput
 
-%x PATHSTR MAPSTR DNSTR OPTSTR
+%x PATHSTR MAPSTR DNSTR OPTSTR OCTAL
 
 WS		[[:blank:]]+
 OPTWS		[[:blank:]]*
@@ -95,6 +95,7 @@ OPTIONSTR	([\-]?([[:alpha:]_]([[:alnum:]_\-])*(=(\"?([[:alnum:]_\-\:])+\"?))?)+)
 MACROSTR	(-D{OPTWS}([[:alpha:]_]([[:alnum:]_\-\.])*)=([[:alnum:]_\-\.])+)
 SLASHIFYSTR	(--(no-)?slashify-colons)
 NUMBER		[0-9]+
+OCTALNUMBER	[0-7]+
 
 DNSERVSTR1	([[:alpha:]][[:alnum:]\-.]*(:[0-9]+)?:)
 DNSERVSTR2	(\[([[:xdigit:]]:.)+\](:[0-9]+)?:)
@@ -125,6 +126,8 @@ MTYPE		((file|program|exec|sss|yp|nis|nisplus|ldap|ldaps|hesiod|userdir)(,(sun|h
 OPTTOUT		(-t{OPTWS}|-t{OPTWS}={OPTWS}|--timeout{OPTWS}|--timeout{OPTWS}={OPTWS})
 OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeout{OPTWS}={OPTWS})
 
+MODE		(--mode{OPTWS}|--mode{OPTWS}={OPTWS})
+
 %%
 
 <INITIAL>{
@@ -392,6 +395,11 @@ OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeo
 	-w|--use-weight-only	{ return(OPT_USE_WEIGHT); }
 	-r|--random-multimount-selection { return(OPT_RANDOM); }
 
+	{MODE}/{OCTALNUMBER} {
+		BEGIN(OCTAL);
+		return(OPT_MODE);
+	}
+
 	{OPTWS}","{OPTWS}	{ return(COMMA); }
 
 	{OPTWS} {}
@@ -423,6 +431,16 @@ OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeo
 	<<EOF>> { BEGIN(INITIAL); }
 }
 
+<OCTAL>{
+
+	{OCTALNUMBER} {
+		master_lval.longtype = strtol(master_text, NULL, 8);
+		return(OCTALNUMBER);
+	}
+
+	.	{ BEGIN(OPTSTR); yyless(0); }
+}
+
 %%
 
 #include "automount.h"

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-13 13:56 [PATCH] Add a --mode option to chmod the mount point of the maps Cyril B.
@ 2015-09-14  2:31 ` Ian Kent
  2015-09-14  8:42   ` Cyril B.
  2015-09-14  3:05 ` Ian Kent
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Kent @ 2015-09-14  2:31 UTC (permalink / raw
  To: Cyril B.; +Cc: autofs@vger.kernel.org

On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
> Hello,
> 
> It looks like the mount point of the maps have fixed permissions, 755. I 
> need to have different permissions: in my use case, I want /home (which 
> is handled by autofs) to be set to 751.

Why is this needed?

> 
> The initial permissions of /home are overwritten when autofs is started, 
> so changing those doesn't help.

They aren't overwritten.
The permissions are those of the autofs mount that is mounted
over /home.

> 
> I can change the permissions of /home after autofs has started, but it's 
> really not convenient: there's no easy way to do that automatically with 
> either sysvinit or systemd.

I still don't understand why this is needed.

For indirect mounts no-one can copy things into the autofs mount and the
permissions of mounts on mount points will be the same as those of the
mounted file system when they are done.

Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-13 13:56 [PATCH] Add a --mode option to chmod the mount point of the maps Cyril B.
  2015-09-14  2:31 ` Ian Kent
@ 2015-09-14  3:05 ` Ian Kent
  2015-09-14  3:23   ` Ian Kent
  2015-09-14 11:31   ` Cyril B.
  1 sibling, 2 replies; 12+ messages in thread
From: Ian Kent @ 2015-09-14  3:05 UTC (permalink / raw
  To: Cyril B.; +Cc: autofs@vger.kernel.org

On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
> 
> My solution was to add a --mode option to autofs, with the included 
> patch. I'm not familiar with autofs's code or even Lex and Yacc, so my 
> code is probably more of a proof of concept. It seems to work fine in my 
> use case, though.

If you want to contribute patches then post then in-line as text without
any additional change (eg. ensure the mailer doesn't split lines therby
corrupting the patch).

I can't properly comment on the patch, even if I wanted too, because it
isn't in-line.

Anyway, what about updating the man pages with your new option?
Should text modes be considered, perhaps octal modes are sufficient, and
should be all that's allowed, to keep the change as simple (generally a
good idea) ...

Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  3:05 ` Ian Kent
@ 2015-09-14  3:23   ` Ian Kent
  2015-09-14 11:31   ` Cyril B.
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Kent @ 2015-09-14  3:23 UTC (permalink / raw
  To: Cyril B.; +Cc: autofs@vger.kernel.org

On Mon, 2015-09-14 at 11:05 +0800, Ian Kent wrote:
> On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
> > 
> > My solution was to add a --mode option to autofs, with the included 
> > patch. I'm not familiar with autofs's code or even Lex and Yacc, so my 
> > code is probably more of a proof of concept. It seems to work fine in my 
> > use case, though.
> 
> If you want to contribute patches then post then in-line as text without
> any additional change (eg. ensure the mailer doesn't split lines therby
> corrupting the patch).
> 
> I can't properly comment on the patch, even if I wanted too, because it
> isn't in-line.
> 
> Anyway, what about updating the man pages with your new option?
> Should text modes be considered, perhaps octal modes are sufficient, and
> should be all that's allowed, to keep the change as simple (generally a
> good idea) ...

And btw, I've been trying to ensure that all autofs patches have a
problem description for quite a long time now, so we need that too.

Generally describing what the change does isn't OK, a description of why
it's needed is more important and usually negates the need to describe
what the patch does.

Recently I've started adding "Signed-off-by:" annotation too.

Ian 

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  2:31 ` Ian Kent
@ 2015-09-14  8:42   ` Cyril B.
  2015-09-14  9:20     ` Frank Thommen
  2015-09-14  9:45     ` Ian Kent
  0 siblings, 2 replies; 12+ messages in thread
From: Cyril B. @ 2015-09-14  8:42 UTC (permalink / raw
  To: Ian Kent; +Cc: autofs@vger.kernel.org

Ian Kent wrote:
> On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
>> It looks like the mount point of the maps have fixed permissions, 755. I
>> need to have different permissions: in my use case, I want /home (which
>> is handled by autofs) to be set to 751.
>
> Why is this needed?

Why do I want to set /home to 751? When it was set to 755, I frequently 
had users believing there was a serious vulnerability because they could 
list /home. Stupid, I know, but setting the permissions to 751 was a 
trivial solution for this.

>> The initial permissions of /home are overwritten when autofs is started,
>> so changing those doesn't help.
>
> They aren't overwritten.
> The permissions are those of the autofs mount that is mounted
> over /home.

I'm not sure I'm following you. Here's my auto.master:

/home program:/etc/auto.home

Before launching autofs, permissions are set to 751:

# ls -ald /home
drwxr-x--x 2 root root 4096 Aug  7 11:09 /home

Once I've launched autofs, permissions are reset to 755:

# ls -ald /home
drwxr-xr-x 2 root root 0 Sep 14 10:32 /home

And when I quit autofs, my permissions are back to 751:

# ls -ald /home
drwxr-x--x 2 root root 4096 Aug  7 11:09 /home

What I want is to always have /home permissions set to 751.

Note that I'm not talking about mount points below /home (e.g. 
/home/foobar), those do have the correct permissions.

-- 
Cyril B.
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  8:42   ` Cyril B.
@ 2015-09-14  9:20     ` Frank Thommen
  2015-09-14  9:29       ` Cyril B.
  2015-09-14  9:45     ` Ian Kent
  1 sibling, 1 reply; 12+ messages in thread
From: Frank Thommen @ 2015-09-14  9:20 UTC (permalink / raw
  To: cbay, Ian Kent; +Cc: autofs@vger.kernel.org

On 14.09.15 10:42, Cyril B. wrote:
> Ian Kent wrote:
>> On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
>>> It looks like the mount point of the maps have fixed permissions, 755. I
>>> need to have different permissions: in my use case, I want /home (which
>>> is handled by autofs) to be set to 751.
>>
>> Why is this needed?
>
> Why do I want to set /home to 751? When it was set to 755, I frequently
> had users believing there was a serious vulnerability because they could
> list /home. Stupid, I know, but setting the permissions to 751 was a
> trivial solution for this.
>
>>> The initial permissions of /home are overwritten when autofs is started,
>>> so changing those doesn't help.
>>
>> They aren't overwritten.
>> The permissions are those of the autofs mount that is mounted
>> over /home.
>
> I'm not sure I'm following you. Here's my auto.master:
>
> /home program:/etc/auto.home
>
> Before launching autofs, permissions are set to 751:
>
> # ls -ald /home
> drwxr-x--x 2 root root 4096 Aug  7 11:09 /home
>
> Once I've launched autofs, permissions are reset to 755:
>
> # ls -ald /home
> drwxr-xr-x 2 root root 0 Sep 14 10:32 /home

They are not "re"set.  These are the permissions of the filesystem that 
you are mounting over /home (e.g. myfileserver:/export/homes).  Change 
the permissions of /export/homes on myfileserver.

frank


>
> And when I quit autofs, my permissions are back to 751:
>
> # ls -ald /home
> drwxr-x--x 2 root root 4096 Aug  7 11:09 /home
>
> What I want is to always have /home permissions set to 751.
>
> Note that I'm not talking about mount points below /home (e.g.
> /home/foobar), those do have the correct permissions.
>


-- 
Frank Thommen - Structures IT Management and Support - EMBL Heidelberg
frank.thommen@embl-heidelberg.de - +49 6221 387 8353
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  9:20     ` Frank Thommen
@ 2015-09-14  9:29       ` Cyril B.
  2015-09-14  9:52         ` Ian Kent
  0 siblings, 1 reply; 12+ messages in thread
From: Cyril B. @ 2015-09-14  9:29 UTC (permalink / raw
  To: Frank Thommen; +Cc: Ian Kent, autofs@vger.kernel.org

Frank Thommen wrote:
> They are not "re"set.  These are the permissions of the filesystem that
> you are mounting over /home (e.g. myfileserver:/export/homes).  Change
> the permissions of /export/homes on myfileserver.

In my case, I haven't mounted anything at all yet. My map is an 
executable, and since I haven't triggered an automatic mount (by 
accessing /home/foo), the executable has not even been called once. 
/home is empty.

-- 
Cyril B.
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  8:42   ` Cyril B.
  2015-09-14  9:20     ` Frank Thommen
@ 2015-09-14  9:45     ` Ian Kent
  2015-09-14 10:12       ` Cyril B.
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Kent @ 2015-09-14  9:45 UTC (permalink / raw
  To: cbay; +Cc: autofs@vger.kernel.org

On Mon, 2015-09-14 at 10:42 +0200, Cyril B. wrote:
> Ian Kent wrote:
> > On Sun, 2015-09-13 at 15:56 +0200, Cyril B. wrote:
> >> It looks like the mount point of the maps have fixed permissions, 755. I
> >> need to have different permissions: in my use case, I want /home (which
> >> is handled by autofs) to be set to 751.
> >
> > Why is this needed?
> 
> Why do I want to set /home to 751? When it was set to 755, I frequently 
> had users believing there was a serious vulnerability because they could 
> list /home. Stupid, I know, but setting the permissions to 751 was a 
> trivial solution for this.
> 
> >> The initial permissions of /home are overwritten when autofs is started,
> >> so changing those doesn't help.
> >
> > They aren't overwritten.
> > The permissions are those of the autofs mount that is mounted
> > over /home.
> 
> I'm not sure I'm following you. Here's my auto.master:
> 
> /home program:/etc/auto.home

Which means there will be an autofs pseudo file system mounted on /home
when automount(8) starts.

This is how the kernel intercepts accesses to these and calls back to
the automount(8) daemon to request it perform mounts.

> 
> Before launching autofs, permissions are set to 751:
> 
> # ls -ald /home
> drwxr-x--x 2 root root 4096 Aug  7 11:09 /home
> 
> Once I've launched autofs, permissions are reset to 755:

The permissions are those of the root of the autofs pseudo file system,
the permissions of the underlying mount point aren't changed.

> 
> # ls -ald /home
> drwxr-xr-x 2 root root 0 Sep 14 10:32 /home
> 
> And when I quit autofs, my permissions are back to 751:

Because the autofs pseudo file system is then umounted and the
underlying directory is again visible.

> 
> # ls -ald /home
> drwxr-x--x 2 root root 4096 Aug  7 11:09 /home
> 
> What I want is to always have /home permissions set to 751.

I still don't understand how that's an effective security measure.

Anyone determined enough will be able to find out the directory names
and can then change to them, assuming the permissions of the mounts
themselves allow it.

No-one can do anything much in the autofs pseudo file system except the
daemon itself, and usually the request is that the mount points within
an autofs mount be seen, aka. the browse option, not hidden, ;)

So are you saying you don't have sufficient faith in the permissions set
on the file systems your mounting, that contain the information you want
to protect, that you must have the permissions of an intermediate file
system set to ensure that information about that vulnerability is not
seen?

Can't say I can see how this has any actual benefit at all.

Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  9:29       ` Cyril B.
@ 2015-09-14  9:52         ` Ian Kent
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Kent @ 2015-09-14  9:52 UTC (permalink / raw
  To: cbay; +Cc: Frank Thommen, autofs@vger.kernel.org

On Mon, 2015-09-14 at 11:29 +0200, Cyril B. wrote:
> Frank Thommen wrote:
> > They are not "re"set.  These are the permissions of the filesystem that
> > you are mounting over /home (e.g. myfileserver:/export/homes).  Change
> > the permissions of /export/homes on myfileserver.
> 
> In my case, I haven't mounted anything at all yet. My map is an 
> executable, and since I haven't triggered an automatic mount (by 
> accessing /home/foo), the executable has not even been called once. 
> /home is empty.

And if you create a file inside /home before starting automount(8) it
will (not so) mysteriously disappear when automount is started.

Why don't you check what mounts you have after automount is started.

That may well answer your question and don't forget to check
if /etc/mtab is a symlink to a mount table in the proc file system.

If it isn't then the autofs file system mounts won't be present
in /etc/mtab but they will be seen in the mount table within the proc
pseudo file system at /proc/mounts.

Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  9:45     ` Ian Kent
@ 2015-09-14 10:12       ` Cyril B.
  2015-09-14 10:38         ` Ian Kent
  0 siblings, 1 reply; 12+ messages in thread
From: Cyril B. @ 2015-09-14 10:12 UTC (permalink / raw
  To: Ian Kent; +Cc: autofs@vger.kernel.org

Ian Kent wrote:
> So are you saying you don't have sufficient faith in the permissions set
> on the file systems your mounting, that contain the information you want
> to protect, that you must have the permissions of an intermediate file
> system set to ensure that information about that vulnerability is not
> seen?

I do know that there's no vulnerability at all, and that you can 
trivially list users by other means.

Unfortunately, some of my less tech savvy users believe that there's a 
vulnerability because they can see other accounts' home directories, and 
thus feel that their own files are not safe. Is this stupid? absolutely. 
But changing my /home permissions to 751 makes those users happy and 
saves my time -- and my reputation as a sysadmin :)

I also do realize that the 755 permissions come from the autofs kernel 
filesystem itself. But the kernel doesn't support a 'mode' option for 
autofs (some other file systems do), and even if it did, autofs would 
have to be patched to support it (in a slightly different way than my 
current patch).

I understand that my use case may be a corner case, and I'm perfectly 
fine with keeping my patch in my own tree. I figured that since I had 
written the patch for myself anway, I may as well post it here as it 
could be useful for others :)

Thanks!

-- 
Cyril B.
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14 10:12       ` Cyril B.
@ 2015-09-14 10:38         ` Ian Kent
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Kent @ 2015-09-14 10:38 UTC (permalink / raw
  To: Cyril B.; +Cc: autofs@vger.kernel.org

On Mon, 2015-09-14 at 12:12 +0200, Cyril B. wrote:
> Ian Kent wrote:
> > So are you saying you don't have sufficient faith in the permissions set
> > on the file systems your mounting, that contain the information you want
> > to protect, that you must have the permissions of an intermediate file
> > system set to ensure that information about that vulnerability is not
> > seen?
> 
> I do know that there's no vulnerability at all, and that you can 
> trivially list users by other means.
> 
> Unfortunately, some of my less tech savvy users believe that there's a 
> vulnerability because they can see other accounts' home directories, and 
> thus feel that their own files are not safe. Is this stupid? absolutely. 
> But changing my /home permissions to 751 makes those users happy and 
> saves my time -- and my reputation as a sysadmin :)
> 
> I also do realize that the 755 permissions come from the autofs kernel 
> filesystem itself. But the kernel doesn't support a 'mode' option for 
> autofs (some other file systems do), and even if it did, autofs would 
> have to be patched to support it (in a slightly different way than my 
> current patch).
> 
> I understand that my use case may be a corner case, and I'm perfectly 
> fine with keeping my patch in my own tree. I figured that since I had 
> written the patch for myself anway, I may as well post it here as it 
> could be useful for others :)

And I didn't say I wouldn't accept the change but I will need you to do
the work to include all the things that the patch needs.

I'm not sure if it would be better to add mode as an autofs file system
option to the kernel and yes, the daemon would still need changes. It
might end up more complicated that way.

Ian

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Add a --mode option to chmod the mount point of the maps
  2015-09-14  3:05 ` Ian Kent
  2015-09-14  3:23   ` Ian Kent
@ 2015-09-14 11:31   ` Cyril B.
  1 sibling, 0 replies; 12+ messages in thread
From: Cyril B. @ 2015-09-14 11:31 UTC (permalink / raw
  To: Ian Kent; +Cc: autofs@vger.kernel.org

Ian Kent wrote:
> If you want to contribute patches then post then in-line as text without
> any additional change (eg. ensure the mailer doesn't split lines therby
> corrupting the patch).
>
> I can't properly comment on the patch, even if I wanted too, because it
> isn't in-line.
>
> Anyway, what about updating the man pages with your new option?
> Should text modes be considered, perhaps octal modes are sufficient, and
> should be all that's allowed, to keep the change as simple (generally a
> good idea) ...

My bad, sorry. Here's a second try, including a commit message and an updated man page:

add map option --mode

Add a --mode map option to change the mode for the base location mount
point. If this option is given, autofs will chmod the mount point right
after mounting it (as the kernel autofs filesystem doesn't support a
'mode' option).

Changing the mode of the base location mount point is normally not needed,
but if one wants to do that, it's much better to do it inside autofs rather
than outside to avoid race conditions and making sure the correct permissions
are always set.
---
  daemon/direct.c      |  4 ++++
  daemon/indirect.c    |  4 ++++
  include/automount.h  |  1 +
  lib/master.c         |  1 +
  lib/master_parse.y   | 10 +++++++++-
  lib/master_tok.l     | 20 +++++++++++++++++++-
  man/auto.master.5.in |  5 +++++
  7 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/daemon/direct.c b/daemon/direct.c
index 5569299..55c4aa6 100644
--- a/daemon/direct.c
+++ b/daemon/direct.c
@@ -433,6 +433,10 @@ int do_mount_autofs_direct(struct autofs_point *ap,
  		goto out_umount;
  	}

+       if (ap->mode != -1) {
+               chmod(me->key, ap->mode);
+       }
+
  	ops->open(ap->logopt, &ioctlfd, st.st_dev, me->key);
  	if (ioctlfd < 0) {
  		crit(ap->logopt, "failed to create ioctl fd for %s", me->key);
diff --git a/daemon/indirect.c b/daemon/indirect.c
index a04a624..e071f12 100644
--- a/daemon/indirect.c
+++ b/daemon/indirect.c
@@ -163,6 +163,10 @@ static int do_mount_autofs_indirect(struct autofs_point *ap, const char *root)
  		goto out_umount;
  	}

+       if (ap->mode != -1) {
+               chmod(root, ap->mode);
+       }
+
  	if (ops->open(ap->logopt, &ap->ioctlfd, st.st_dev, root)) {
  		crit(ap->logopt,
  		     "failed to create ioctl fd for autofs path %s", ap->path);
diff --git a/include/automount.h b/include/automount.h
index 447aba1..0b37c32 100644
--- a/include/automount.h
+++ b/include/automount.h
@@ -492,6 +492,7 @@ struct kernel_mod_version {
  struct autofs_point {
  	pthread_t thid;
  	char *path;			/* Mount point name */
+       mode_t mode;                    /* Mount point mode */
  	char *pref;			/* amd prefix */
  	int pipefd;			/* File descriptor for pipe */
  	int kpipefd;			/* Kernel end descriptor for pipe */
diff --git a/lib/master.c b/lib/master.c
index 6c38b1c..8d4c864 100644
--- a/lib/master.c
+++ b/lib/master.c
@@ -129,6 +129,7 @@ int master_add_autofs_point(struct master_mapent *entry, unsigned logopt,
  		free(ap);
  		return 0;
  	}
+       ap->mode = -1;

  	entry->ap = ap;

diff --git a/lib/master_parse.y b/lib/master_parse.y
index 9da78fc..825c565 100644
--- a/lib/master_parse.y
+++ b/lib/master_parse.y
@@ -63,6 +63,7 @@ static unsigned ghost;
  extern unsigned global_selection_options;
  static unsigned random_selection;
  static unsigned use_weight;
+static mode_t mode;
  static char **tmp_argv;
  static int tmp_argc;
  static char **local_argv;
@@ -101,7 +102,7 @@ static int master_fprintf(FILE *, char *, ...);
  %token COMMENT
  %token MAP
  %token OPT_TIMEOUT OPT_NTIMEOUT OPT_NOBIND OPT_NOGHOST OPT_GHOST OPT_VERBOSE
-%token OPT_DEBUG OPT_RANDOM OPT_USE_WEIGHT OPT_SYMLINK
+%token OPT_DEBUG OPT_RANDOM OPT_USE_WEIGHT OPT_SYMLINK OPT_MODE
  %token COLON COMMA NL DDASH
  %type <strtype> map
  %type <strtype> options
@@ -126,6 +127,7 @@ static int master_fprintf(FILE *, char *, ...);
  %token <strtype> MAPXFN
  %token <strtype> MAPNAME
  %token <longtype> NUMBER
+%token <longtype> OCTALNUMBER
  %token <strtype> OPTION

  %start file
@@ -192,6 +194,7 @@ line:
  	| PATH OPT_GHOST { master_notify($1); YYABORT; }
  	| PATH OPT_NOGHOST { master_notify($1); YYABORT; }
  	| PATH OPT_VERBOSE { master_notify($1); YYABORT; }
+       | PATH OPT_MODE { master_notify($1); YYABORT; }
  	| PATH { master_notify($1); YYABORT; }
  	| QUOTE { master_notify($1); YYABORT; }
  	| OPTION { master_notify($1); YYABORT; }
@@ -576,6 +579,7 @@ daemon_option: OPT_TIMEOUT NUMBER { timeout = $2; }
  	| OPT_DEBUG	{ debug = 1; }
  	| OPT_RANDOM	{ random_selection = 1; }
  	| OPT_USE_WEIGHT { use_weight = 1; }
+       | OPT_MODE OCTALNUMBER { mode = $2; }
  	;

  mount_option: OPTION
@@ -644,6 +648,7 @@ static void local_init_vars(void)
  	ghost = defaults_get_browse_mode();
  	random_selection = global_selection_options & MOUNT_FLAG_RANDOM_SELECT;
  	use_weight = 0;
+       mode = -1;
  	tmp_argv = NULL;
  	tmp_argc = 0;
  	local_argv = NULL;
@@ -847,6 +852,9 @@ int master_parse_entry(const char *buffer, unsigned int default_timeout, unsigne
  		entry->ap->flags |= MOUNT_FLAG_SYMLINK;
  	if (negative_timeout)
  		entry->ap->negative_timeout = negative_timeout;
+       if (mode != -1) {
+               entry->ap->mode = mode;
+       }

  /*
  	source = master_find_map_source(entry, type, format,
diff --git a/lib/master_tok.l b/lib/master_tok.l
index c692e14..6433448 100644
--- a/lib/master_tok.l
+++ b/lib/master_tok.l
@@ -84,7 +84,7 @@ unsigned int tlen;

  %option nounput

-%x PATHSTR MAPSTR DNSTR OPTSTR
+%x PATHSTR MAPSTR DNSTR OPTSTR OCTAL

  WS		[[:blank:]]+
  OPTWS		[[:blank:]]*
@@ -95,6 +95,7 @@ OPTIONSTR	([\-]?([[:alpha:]_]([[:alnum:]_\-])*(=(\"?([[:alnum:]_\-\:])+\"?))?)+)
  MACROSTR	(-D{OPTWS}([[:alpha:]_]([[:alnum:]_\-\.])*)=([[:alnum:]_\-\.])+)
  SLASHIFYSTR	(--(no-)?slashify-colons)
  NUMBER		[0-9]+
+OCTALNUMBER    [0-7]+

  DNSERVSTR1	([[:alpha:]][[:alnum:]\-.]*(:[0-9]+)?:)
  DNSERVSTR2	(\[([[:xdigit:]]:.)+\](:[0-9]+)?:)
@@ -125,6 +126,8 @@ MTYPE		((file|program|exec|sss|yp|nis|nisplus|ldap|ldaps|hesiod|userdir)(,(sun|h
  OPTTOUT		(-t{OPTWS}|-t{OPTWS}={OPTWS}|--timeout{OPTWS}|--timeout{OPTWS}={OPTWS})
  OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeout{OPTWS}={OPTWS})

+MODE           (--mode{OPTWS}|--mode{OPTWS}={OPTWS})
+
  %%

  <INITIAL>{
@@ -392,6 +395,11 @@ OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeo
  	-w|--use-weight-only	{ return(OPT_USE_WEIGHT); }
  	-r|--random-multimount-selection { return(OPT_RANDOM); }

+       {MODE}/{OCTALNUMBER} {
+               BEGIN(OCTAL);
+               return(OPT_MODE);
+       }
+
  	{OPTWS}","{OPTWS}	{ return(COMMA); }

  	{OPTWS} {}
@@ -423,6 +431,16 @@ OPTNTOUT	(-n{OPTWS}|-n{OPTWS}={OPTWS}|--negative-timeout{OPTWS}|--negative-timeo
  	<<EOF>> { BEGIN(INITIAL); }
  }

+<OCTAL>{
+
+       {OCTALNUMBER} {
+               master_lval.longtype = strtol(master_text, NULL, 8);
+               return(OCTALNUMBER);
+       }
+
+       .       { BEGIN(OPTSTR); yyless(0); }
+}
+
  %%

  #include "automount.h"
diff --git a/man/auto.master.5.in b/man/auto.master.5.in
index 2e475dc..ba28494 100644
--- a/man/auto.master.5.in
+++ b/man/auto.master.5.in
@@ -211,6 +211,11 @@ or in the configuration.
  Set the timeout for caching failed key lookups. This option can be
  used to override the global default given either on the command line
  or in the configuration.
+.TP
+.I "\-\-mode <octal_mode>"
+Set the directory mode for the base location of the \fBautofs\fP mount point.
+If this option is given, \fBautofs\fP will chmod that directory with this
+mode.
  .SH BUILTIN MAP \-hosts
  If "\-hosts" is given as the map then accessing a key under the mount point
  which corresponds to a hostname will allow access to the exports of that
--
2.1.4


-- 
Cyril B.
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-09-14 11:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-13 13:56 [PATCH] Add a --mode option to chmod the mount point of the maps Cyril B.
2015-09-14  2:31 ` Ian Kent
2015-09-14  8:42   ` Cyril B.
2015-09-14  9:20     ` Frank Thommen
2015-09-14  9:29       ` Cyril B.
2015-09-14  9:52         ` Ian Kent
2015-09-14  9:45     ` Ian Kent
2015-09-14 10:12       ` Cyril B.
2015-09-14 10:38         ` Ian Kent
2015-09-14  3:05 ` Ian Kent
2015-09-14  3:23   ` Ian Kent
2015-09-14 11:31   ` Cyril B.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.