11:26 a.m.
The attached patch adds support for noexec, nosuid and nodev flags for
tmpfs mounts in pam_namespace.
Please review, thanks.
--
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
[You'll know whether the road is wrong if you carefully listen to your
conscience.]
7:55 a.m.
New subject: Support for noexec, nosuid and nodev flags for tmpfs mounts in pam_namespace
Ping for the review?
On Wed, 2019-10-09 at 18:26 +0200, Tomas Mraz wrote:
The attached patch adds support for noexec, nosuid and nodev flags
for
tmpfs mounts in pam_namespace.
Please review, thanks.
_______________________________________________
Pam-developers mailing list -- pam-developers(a)lists.fedorahosted.org
To unsubscribe send an email to
pam-developers-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/pam-developers@lists.fedorah...
--
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
[You'll know whether the road is wrong if you carefully listen to your
conscience.]
8:43 a.m.
New subject: Support for noexec, nosuid and nodev flags for tmpfs mounts in pam_namespace
2nd ping?
If no objection, I'll commit after one week.
On Mon, 2019-10-14 at 14:55 +0200, Tomas Mraz wrote:
Ping for the review?
On Wed, 2019-10-09 at 18:26 +0200, Tomas Mraz wrote:
> The attached patch adds support for noexec, nosuid and nodev flags
> for
> tmpfs mounts in pam_namespace.
>
> Please review, thanks.
> _______________________________________________
> Pam-developers mailing list --
> pam-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to
> pam-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
> https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedorahosted.org/archives/list/pam-developers@lists.fedorah...
--
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
[You'll know whether the road is wrong if you carefully listen to
your
conscience.]
_______________________________________________
Pam-developers mailing list -- pam-developers(a)lists.fedorahosted.org
To unsubscribe send an email to
pam-developers-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/pam-developers@lists.fedorah...
--
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
[You'll know whether the road is wrong if you carefully listen to your
conscience.]
6:31 p.m.
New subject: Support for noexec, nosuid and nodev flags for tmpfs mounts in pam_namespace
On Wed, Oct 09, 2019 at 06:26:52PM +0200, Tomas Mraz wrote:
The attached patch adds support for noexec, nosuid and nodev flags
for
tmpfs mounts in pam_namespace.
Thanks. I think it works as described, although I have a few questions
about the implementation.
diff --git a/modules/pam_namespace/namespace.conf.5.xml
b/modules/pam_namespace/namespace.conf.5.xml
index c7698cb..a94b49e 100644
--- a/modules/pam_namespace/namespace.conf.5.xml
+++ b/modules/pam_namespace/namespace.conf.5.xml
@@ -122,9 +122,14 @@
<para><emphasis>mntopts</emphasis>=<replaceable>value</replaceable>
- value of this flag is passed to the mount call when the tmpfs mount is
done. It allows for example the specification of the maximum size of the
- tmpfs instance that is created by the mount call. See <citerefentry>
-
<refentrytitle>mount</refentrytitle><manvolnum>8</manvolnum>
- </citerefentry> for details.
+ tmpfs instance that is created by the mount call. In addition to
+ options specified in the <citerefentry>
+
<refentrytitle>tmpfs</refentrytitle><manvolnum>5</manvolnum>
+ </citerefentry> manual the <emphasis>nosuid</emphasis>,
+ <emphasis>noexec</emphasis>, and
<emphasis>nodev</emphasis> flags
+ can be used to respectively disable setuid bit effect, disable running
+ executables, and disable devices to be interpreted on the mounted
+ tmpfs filesystem.
</para>
<para>
diff --git a/modules/pam_namespace/pam_namespace.c
b/modules/pam_namespace/pam_namespace.c
index f541f89..0fd59fb 100644
--- a/modules/pam_namespace/pam_namespace.c
+++ b/modules/pam_namespace/pam_namespace.c
@@ -230,6 +230,78 @@ static int parse_iscript_params(char *params, struct polydir_s
*poly)
return 0;
}
+struct mntflag {
+ const char *name;
+ size_t size;
+ unsigned long flag;
+};
+
+#define LITERAL_AND_SIZE(x) x, sizeof(x)
+
+static const struct mntflag mntflags[] = {
+ { LITERAL_AND_SIZE("noexec"), MS_NOEXEC },
+ { LITERAL_AND_SIZE("nosuid"), MS_NOSUID },
+ { LITERAL_AND_SIZE("nodev"), MS_NODEV }
+ };
The only use case of struct mntflag.size is "if (mntflags[i].size - 1 != len)",
so why it's .size and sizeof instead of .len and sizeof - 1?
+
+static int filter_mntopts(const char *opts, char **filtered,
+ unsigned long *mountflags)
+{
+ size_t origlen = strlen(opts);
+ const char *end;
+ char *dest;
+
+ *filtered = NULL;
+ *mountflags = 0;
+
+ if (origlen == 0)
+ return 0;
+
+ dest = *filtered = calloc(1, origlen);
It has to be origlen + 1 to make room for the terminating null when
nothing is filtered.
+ if (*filtered == NULL)
+ return -1;
+
+ do {
+ size_t len;
+ int i;
+
+ end = strchr(opts, ',');
+ if (end == NULL) {
+ len = strlen(opts);
+ } else {
+ len = end - opts;
If I was writing this, I'd add ++end here, this would change "opts = end +
1"
at the end of loop into "opts = end", which in turn would allow to replace
"end" with "opts" (or even with "opts && *opts") in
the loop condition and
move the definition of "end" inside the loop.
+ }
+
+ for (i = 0; i < (int)(sizeof(mntflags)/sizeof(mntflags[0])); i++) {
+ if (mntflags[i].size - 1 != len)
+ continue;
+ if (strncmp(mntflags[i].name, opts, len) == 0) {
Since strlen(mntflags[i].name) == len && strlen(opts) >= len,
why not use memcmp to highlight this fact?
+ *mountflags |= mntflags[i].flag;
+ opts = end;
+ break;
+ }
+ }
+
+ if (opts != end) {
+ if (dest != *filtered) {
+ *dest = ',';
+ ++dest;
+ }
+ strncpy(dest, opts, len);
Likewise, strlen(opts) >= len means we do not null-terminate dest
(which is not a problem as the whole buffer is calloc'ed),
but why not use memcpy to highlight this fact?
+ dest += len;
+ }
By the way, you could delay memory allocation to this point
by initializing dest to NULL before the loop and changing this code into
if (len && opts != end) {
if (dest) {
*(dest++) = ',';
} else {
dest = *filtered = calloc(1, strlen(opts) + 1);
if (*filtered == NULL)
return -1;
}
memcpy(dest, opts, len);
dest += len;
}
+ opts = end + 1;
+ } while (end != NULL);
+
+ if (dest == *filtered) {
+ /* nothing left */
+ free(dest);
+ *filtered = NULL;
+ }
... and this "if" condition won't be needed, too.
+ return 0;
+}
+
static int parse_method(char *method, struct polydir_s *poly,
struct instance_data *idata)
{
--
ldv
2:35 a.m.
New subject: Support for noexec, nosuid and nodev flags for tmpfs mounts in pam_namespace
On Mon, 2019-11-04 at 03:31 +0300, Dmitry V. Levin wrote:
On Wed, Oct 09, 2019 at 06:26:52PM +0200, Tomas Mraz wrote:
> The attached patch adds support for noexec, nosuid and nodev flags
> for
> tmpfs mounts in pam_namespace.
Thanks. I think it works as described, although I have a few
questions
about the implementation.
OK, here is the patch with most of the suggestions applied except for
the opts/end meaning change as in my opinion the code is more readable
without it.
--
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
[You'll know whether the road is wrong if you carefully listen to your
conscience.]
2:59 a.m.
New subject: Support for noexec, nosuid and nodev flags for tmpfs mounts in pam_namespace
On Mon, Nov 04, 2019 at 09:35:58AM +0100, Tomas Mraz wrote:
On Mon, 2019-11-04 at 03:31 +0300, Dmitry V. Levin wrote:
> On Wed, Oct 09, 2019 at 06:26:52PM +0200, Tomas Mraz wrote:
> > The attached patch adds support for noexec, nosuid and nodev flags
> > for
> > tmpfs mounts in pam_namespace.
>
> Thanks. I think it works as described, although I have a few
> questions
> about the implementation.
OK, here is the patch with most of the suggestions applied except for
the opts/end meaning change as in my opinion the code is more readable
without it.
Looks good now, thanks.
--
ldv
1628
days inactive
1654
days old
pam-developers@lists.fedorahosted.org
5 comments
2 participants
participants (2)
-
Dmitry V. Levin -
Tomas Mraz