* Re: [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE
From: Christian Brauner @ 2020-07-19 18:17 UTC (permalink / raw)
To: Adrian Reber, Nicolas Viennot
Cc: Eric Biederman, Pavel Emelyanov, Oleg Nesterov, Dmitry Safonov,
Andrei Vagin, Michał Cłapiński, Kamil Yurtsever,
Dirk Petersen, Christine Flood, Casey Schaufler, Mike Rapoport,
Radostin Stoyanov, Cyrill Gorcunov, Serge Hallyn, Stephen Smalley,
Sargun Dhillon, Arnd Bergmann, linux-security-module,
linux-kernel, selinux, Eric Paris, Jann Horn, linux-fsdevel
In-Reply-To: <20200719100418.2112740-1-areber@redhat.com>
On Sun, Jul 19, 2020 at 12:04:10PM +0200, Adrian Reber wrote:
> This is v6 of the 'Introduce CAP_CHECKPOINT_RESTORE' patchset. The
> changes to v5 are:
>
> * split patch dealing with /proc/self/exe into two patches:
> * first patch to enable changing it with CAP_CHECKPOINT_RESTORE
> and detailed history in the commit message
> * second patch changes -EINVAL to -EPERM
> * use kselftest_harness.h infrastructure for test
> * replace if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE))
> with if (!checkpoint_restore_ns_capable(&init_user_ns))
>
> Adrian Reber (5):
> capabilities: Introduce CAP_CHECKPOINT_RESTORE
> pid: use checkpoint_restore_ns_capable() for set_tid
> pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid
> proc: allow access in init userns for map_files with
> CAP_CHECKPOINT_RESTORE
> selftests: add clone3() CAP_CHECKPOINT_RESTORE test
>
> Nicolas Viennot (2):
> prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe
> prctl: exe link permission error changed from -EINVAL to -EPERM
>
> fs/proc/base.c | 8 +-
> include/linux/capability.h | 6 +
> include/uapi/linux/capability.h | 9 +-
> kernel/pid.c | 2 +-
> kernel/pid_namespace.c | 2 +-
> kernel/sys.c | 13 +-
> security/selinux/include/classmap.h | 5 +-
> tools/testing/selftests/clone3/.gitignore | 1 +
> tools/testing/selftests/clone3/Makefile | 4 +-
> .../clone3/clone3_cap_checkpoint_restore.c | 177 ++++++++++++++++++
> 10 files changed, 212 insertions(+), 15 deletions(-)
> create mode 100644 tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
>
> base-commit: d31958b30ea3b7b6e522d6bf449427748ad45822
Adrian, Nicolas thank you!
I grabbed the series to run the various core test-suites we've added
over the last year and pushed it to
https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=cap_checkpoint_restore
for now to let kbuild/ltp chew on it for a bit.
Thanks!
Christian
^ permalink raw reply
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
From: Lakshmi Ramasubramanian @ 2020-07-20 2:04 UTC (permalink / raw)
To: kernel test robot, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
In-Reply-To: <202007181027.UwJXCNXk%lkp@intel.com>
On 7/17/20 8:14 PM, kernel test robot wrote:
> Hi Lakshmi,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on integrity/next-integrity]
> [cannot apply to pcmoore-selinux/next security/next-testing linus/master v5.8-rc5 next-20200717]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
Thank you for catching this.
I did not see these failures with the compiler and make options I'd used.
Am able to reproduce the errors with the instructions you'd provided.
Will post the updated patches shortly.
thanks,
-lakshmi
^ permalink raw reply
* RE: [PATCH 07/13] fs/kernel_read_file: Switch buffer size arg to size_t
From: David Laight @ 2020-07-20 8:34 UTC (permalink / raw)
To: 'Kees Cook', Scott Branden
Cc: Mimi Zohar, Matthew Wilcox, James Morris, Luis Chamberlain,
Greg Kroah-Hartman, Rafael J. Wysocki, Alexander Viro, Jessica Yu,
Dmitry Kasatkin, Serge E. Hallyn, Casey Schaufler,
Eric W. Biederman, Peter Zijlstra, Matthew Garrett, David Howells,
Mauro Carvalho Chehab, Randy Dunlap, Joel Fernandes (Google),
KP Singh, Dave Olsthoorn, Hans de Goede, Peter Jones,
Andrew Morton, Stephen Boyd, Paul Moore, Stephen Smalley,
linux-security-module@vger.kernel.org,
linux-integrity@vger.kernel.org, selinux@vger.kernel.org,
linux-fsdevel@vger.kernel.org, kexec@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200717174309.1164575-8-keescook@chromium.org>
From: Kees Cook
> Sent: 17 July 2020 18:43
> In preparation for further refactoring of kernel_read_file*(), rename
> the "max_size" argument to the more accurate "buf_size", and correct
> its type to size_t. Add kerndoc to explain the specifics of how the
> arguments will be used. Note that with buf_size now size_t, it can no
> longer be negative (and was never called with a negative value). Adjust
> callers to use it as a "maximum size" when *buf is NULL.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> fs/kernel_read_file.c | 34 +++++++++++++++++++++++---------
> include/linux/kernel_read_file.h | 8 ++++----
> security/integrity/digsig.c | 2 +-
> security/integrity/ima/ima_fs.c | 2 +-
> 4 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index dc28a8def597..e21a76001fff 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -5,15 +5,31 @@
> #include <linux/security.h>
> #include <linux/vmalloc.h>
>
> +/**
> + * kernel_read_file() - read file contents into a kernel buffer
> + *
> + * @file file to read from
> + * @buf pointer to a "void *" buffer for reading into (if
> + * *@buf is NULL, a buffer will be allocated, and
> + * @buf_size will be ignored)
> + * @buf_size size of buf, if already allocated. If @buf not
> + * allocated, this is the largest size to allocate.
> + * @id the kernel_read_file_id identifying the type of
> + * file contents being read (for LSMs to examine)
> + *
> + * Returns number of bytes read (no single read will be bigger
> + * than INT_MAX), or negative on error.
> + *
> + */
That seems to be self-inconsistent.
If '*buf' is NULL is both says that buf_size is ignored and
is treated as a limit.
To make life easier, zero should probably be treated as no-limit.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH 00/15] [libcap] Manual pages: various fixes
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
Hello Andrew,
There's a range of changes in this series. Some are trivial
fixes, and a few are more substantial. The first 11 patches are,
I think, uncontentious.
I would be happy if you check the details in patches patches 12 and 13:
Manual pages: cap_get_file.3: NOTES: note the effect of the Ambient set
Manual pages: cap_get_proc.3: Update description of capsetp()
and I've placed the last two patches at the and because there's
a (hopefully small) chance that you disagree with them.
Michael Kerrisk (man-pages) (15):
Manual pages: various pages: Use "\-" for real minus signs
Manual pages: cap_init.3: Formatting fix
Manual pages: capsh.1: Various minor wording and formatting fixes
Manual pages: cap_copy_ext.3: Typo fix
Manual pages; cap_get_file.3: Fix some clumsily worded text
Manual pages: getcap.8: Add missing word
Manual pages: getcap.8: Fix a clumsily worded sentence
Manual pages: getpcaps.8: Format options as a hanging list
Manual pages: getpcaps.8: Remove a stray .br macro
Manual pages: getpcaps.8: SEE ALSO: fix section number for capsh
Manual pages: setcap.8: Typo fix
Manual pages: cap_get_file.3: NOTES: note the effect of the Ambient set
Manual pages: cap_get_proc.3: Update description of capsetp()
Manual pages: cap_get_proc.3, capsh.1: Use "UID" and "GID" consistently
Manual pages: capsh.1: Change .TP indent to the default
doc/cap_copy_ext.3 | 2 +-
doc/cap_get_file.3 | 19 +++++++----
doc/cap_get_proc.3 | 50 +++++++++++++++++-----------
doc/cap_init.3 | 2 +-
doc/capsh.1 | 81 ++++++++++++++++++++++++++++------------------
doc/getcap.8 | 4 +--
doc/getpcaps.8 | 17 +++++-----
doc/libpsx.3 | 6 ++--
doc/setcap.8 | 2 +-
9 files changed, 111 insertions(+), 72 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH 01/15] Manual pages: various pages: Use "\-" for real minus signs
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_proc.3 | 8 ++++----
doc/capsh.1 | 14 +++++++-------
doc/getpcaps.8 | 6 +++---
doc/libpsx.3 | 6 +++---
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index fda00e0..fce8f59 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -242,11 +242,11 @@ is packaged with a separate POSIX semantics system call library:
If your program uses POSIX threads, to achieve meaningful POSIX
semantics capability manipulation, you should link your program with:
.sp
-.B ld ... -lcap -lpsx -lpthread --wrap=pthread_create
+.B ld ... \-lcap \-lpsx \-lpthread \-\-wrap=pthread_create
.sp
or,
.sp
-.B gcc ... -lcap -lpsx -lpthread -Wl,-wrap,pthread_create
+.B gcc ... \-lcap \-lpsx \-lpthread \-Wl,\-wrap,pthread_create
.sp
When linked this way, due to linker magic, libcap uses
.BR psx_syscall "(3) and " psx_syscall6 (3)
@@ -362,10 +362,10 @@ Note, the above sequence can be performed by the
.B capsh
tool as follows:
.sp
-.B sudo /sbin/capsh --user=nobody --mode=NOPRIV --print
+.B sudo /sbin/capsh \-\-user=nobody \-\-mode=NOPRIV \-\-print
.sp
where
-.B --print
+.B \-\-print
displays the resulting privilege state.
.SH "SEE ALSO"
.BR libcap (3),
diff --git a/doc/capsh.1 b/doc/capsh.1
index 0b987f0..242727c 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -107,7 +107,7 @@ preparations for setting the uid without dropping capabilities in the
process. Following this command the prevailing effective capabilities
will be lowered.
.TP
-.BI \-\-is-uid= <id>
+.BI \-\-is\-uid= <id>
Exit with status 1 unless the current
.IR uid " equals " <id> .
.TP
@@ -120,7 +120,7 @@ using the
.BR setgid (2)
system call.
.TP
-.BI \-\-is-gid= <id>
+.BI \-\-is\-gid= <id>
Exit with status 1 unless the current
.IR gid " equals " <id> .
.TP
@@ -129,7 +129,7 @@ Set the supplementary groups to the numerical list provided. The
groups are set with the
.BR setgroups (2)
system call. See
-.B --user
+.B \-\-user
for a more convenient way of doing this.
.TP
.BI \-\-keep= <0|1>
@@ -152,7 +152,7 @@ the current process. In all cases,
is deactivated when an
.BR exec ()
is performed. See
-.B --secbits
+.B \-\-secbits
for ways to disable this feature.
.TP
.BI \-\-secbits= N
@@ -225,18 +225,18 @@ will cause capsh to promptly exit with a status of 1 when run on
kernel 2.6.27. However, when run on kernel 2.6.38 it will silently
succeed.
.TP
-.BI \-\-has-p= xxx
+.BI \-\-has\-p= xxx
Exit with status 1 unless the
.I permitted
vector has capability
.B xxx
raised.
.TP
-.B \-\-has-ambient
+.B \-\-has\-ambient
Performs a check to see if the running kernel supports ambient
capabilities. If not, the capsh command exits with status 1.
.TP
-.BI \-\-has-a= xxx
+.BI \-\-has\-a= xxx
Exit with status 1 unless the
.I ambient
vector has capability
diff --git a/doc/getpcaps.8 b/doc/getpcaps.8
index 53d342e..7b73e86 100644
--- a/doc/getpcaps.8
+++ b/doc/getpcaps.8
@@ -24,13 +24,13 @@ format.
.PP
Optional arguments:
.PP
-.BR --help " or " --usage
+.BR \-\-help " or " \-\-usage
Displays usage information and exits.
.PP
-.BR --ugly " or " --legacy
+.BR \-\-ugly " or " \-\-legacy
Displays output in a somewhat ugly legacy format.
.PP
-.B --verbose
+.B \-\-verbose
Displays usage in a legacy-like format but not quite so ugly in modern
default terminal fonts.
.SH SEE ALSO
diff --git a/doc/libpsx.3 b/doc/libpsx.3
index 615fceb..a907d8b 100644
--- a/doc/libpsx.3
+++ b/doc/libpsx.3
@@ -11,9 +11,9 @@ psx_syscall3, psx_syscall6 \- POSIX semantics for system calls
.sp
Link with one of these:
.sp
-.I ld ... -lpsx -lpthread --wrap=pthread_create
+.I ld ... \-lpsx \-lpthread \-\-wrap=pthread_create
.sp
-.I gcc ... -lpsx -lpthread -Wl,-wrap,pthread_create
+.I gcc ... \-lpsx \-lpthread \-Wl,\-wrap,pthread_create
.SH DESCRIPTION
The
.B libpsx
@@ -58,7 +58,7 @@ and
functions.
.SH RETURN VALUE
The return value for system call functions is generally the value
-returned by the kernel, or -1 in the case of an error. In such cases
+returned by the kernel, or \-1 in the case of an error. In such cases
.BR errno (3)
is set to the detailed error value. The
.BR psx_syscall3 " and " psx_syscall6
--
2.26.2
^ permalink raw reply related
* [PATCH 02/15] Manual pages: cap_init.3: Formatting fix
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Use nonbreaking space inside 'char *'. In addition to prevent a line break
between these two tokens, the space is not widened when performing line
fill. (The filling makes it look weird.)
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_init.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/cap_init.3 b/doc/cap_init.3
index 96cfea6..362db66 100644
--- a/doc/cap_init.3
+++ b/doc/cap_init.3
@@ -41,7 +41,7 @@ The
argument may identify either a
.I cap_t
entity, or a
-.I char *
+.I "char\ *"
entity allocated by the
.BR cap_to_text ()
function.
--
2.26.2
^ permalink raw reply related
* [PATCH 03/15] Manual pages: capsh.1: Various minor wording and formatting fixes
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/capsh.1 | 47 ++++++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/doc/capsh.1 b/doc/capsh.1
index 242727c..f19a3ea 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -10,7 +10,8 @@ this tool. This tool provides a handy wrapper for certain types
of capability testing and environment creation. It also provides some
debugging features useful for summarizing capability state.
.SH OPTIONS
-The tool takes a number of optional arguments, acting on them in the
+.B capsh
+takes a number of optional arguments, acting on them in the
order they are provided. They are as follows:
.TP 22
.B \-\-help
@@ -30,7 +31,7 @@ for specific commands.
.B ==
Execute
.B capsh
-again with remaining arguments. Useful for testing
+again with the remaining arguments. Useful for testing
.BR exec ()
behavior.
.TP
@@ -44,11 +45,12 @@ is a text-representation of capability state as per
.TP
.BI \-\-drop= cap-list
Remove the listed capabilities from the prevailing bounding set. The
-capabilities are a comma separated list of capabilities as recognized
+capabilities are a comma-separated list of capabilities as recognized
by the
.BR cap_from_name (3)
-function. Use of this feature requires that the capsh program is
-operating with
+function. Use of this feature requires that
+.B capsh
+is operating with
.B CAP_SETPCAP
in its effective set.
.TP
@@ -57,7 +59,9 @@ Set the inheritable set of capabilities for the current process to
equal those provided in the comma separated list. For this action to
succeed, the prevailing process should already have each of these
capabilities in the union of the current inheritable and permitted
-capability sets, or the capsh program is operating with
+capability sets, or
+.B capsh
+should be operating with
.B CAP_SETPCAP
in its effective set.
.TP
@@ -73,7 +77,7 @@ and set them all using
and
.BR cap_setgroups (3).
Following this command, the effective capabilities will be cleared,
-but the permitted set will not be so the running program is still
+but the permitted set will not be, so the running program is still
privileged.
.TP
.B \-\-modes
@@ -87,7 +91,9 @@ security mode. This is a set of securebits and prevailing capability
arrangement recommended for its pre-determined security stance.
.TP
.BR \-\-inmode= <mode>
-Confirm that the prevailing mode is so named, or exit with a status 1.
+Confirm that the prevailing mode is that specified in
+.IR <mode> ,
+or exit with a status 1.
.TP
.BI \-\-uid= id
Force all
@@ -156,9 +162,12 @@ is performed. See
for ways to disable this feature.
.TP
.BI \-\-secbits= N
-Set the security-bits for the program, this is via
-.BR prctl "(2), " PR_SET_SECUREBITS
-API, and the list of supported bits and their meaning can be found in
+Set the security-bits for the program.
+This is done using the
+.BR prctl (2)
+.B PR_SET_SECUREBITS
+operation.
+The list of supported bits and their meaning can be found in
the
.B <sys/secbits.h>
header file. The program will list these bits via the
@@ -221,7 +230,9 @@ $ \fBcapsh \-\-decode=3\fP
As the kernel evolves, more capabilities are added. This option can be used
to verify the existence of a capability on the system. For example,
.BI \-\-supports= cap_syslog
-will cause capsh to promptly exit with a status of 1 when run on
+will cause
+.B capsh
+to promptly exit with a status of 1 when run on
kernel 2.6.27. However, when run on kernel 2.6.38 it will silently
succeed.
.TP
@@ -234,7 +245,9 @@ raised.
.TP
.B \-\-has\-ambient
Performs a check to see if the running kernel supports ambient
-capabilities. If not, the capsh command exits with status 1.
+capabilities. If not,
+.B capsh
+exits with status 1.
.TP
.BI \-\-has\-a= xxx
Exit with status 1 unless the
@@ -252,8 +265,12 @@ Removes the specified ambient capability from the running process.
.B \-\-noamb
Drops all ambient capabilities from the running process.
.SH "EXIT STATUS"
-Following successful execution the tool exits with status 0. Following
-an error, the tool immediately exits with status 1.
+Following successful execution,
+.B capsh
+exits with status 0. Following
+an error,
+.B capsh
+immediately exits with status 1.
.SH AUTHOR
Written by Andrew G. Morgan <morgan@kernel.org>.
.SH "REPORTING BUGS"
--
2.26.2
^ permalink raw reply related
* [PATCH 04/15] Manual pages: cap_copy_ext.3: Typo fix
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_copy_ext.3 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/cap_copy_ext.3 b/doc/cap_copy_ext.3
index 18c2fe6..acbb487 100644
--- a/doc/cap_copy_ext.3
+++ b/doc/cap_copy_ext.3
@@ -34,7 +34,7 @@ function in order to hold the capability data record created from
.BR cap_copy_ext ()
copies a capability state in working storage, identified by
.IR cap_p ,
-from system managed space to user-managed space (pointed to by
+from system-managed space to user-managed space (pointed to by
.IR ext_p )
and returns the length of the resulting data record. The size parameter
represents the maximum size, in bytes, of the resulting data record. The
--
2.26.2
^ permalink raw reply related
* [PATCH 05/15] Manual pages; cap_get_file.3: Fix some clumsily worded text
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Make the text a bit easier to read, and also fix the terms used.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_file.3 | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/doc/cap_get_file.3 b/doc/cap_get_file.3
index c028148..ceacbaf 100644
--- a/doc/cap_get_file.3
+++ b/doc/cap_get_file.3
@@ -57,12 +57,12 @@ A NULL value for
.IR cap_p
is used to indicate that capabilities for the file should be deleted.
For these functions to succeed, the calling process must have the
-effective capability,
-.BR CAP_SETFCAP ,
-enabled and either the effective user ID of the process must match the
+.BR CAP_SETFCAP
+capability in its effective set
+and either the effective user ID of the process must match the
file owner or the calling process must have the
.B CAP_FOWNER
-flag in its effective capability set. The effects of writing the
+capability in its effective capability set. The effects of writing the
capability state to any file type other than a regular file are
undefined.
.PP
--
2.26.2
^ permalink raw reply related
* [PATCH 06/15] Manual pages: getcap.8: Add missing word
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getcap.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/getcap.8 b/doc/getcap.8
index d867203..367d010 100644
--- a/doc/getcap.8
+++ b/doc/getcap.8
@@ -6,7 +6,7 @@ getcap \- examine file capabilities
\fBgetcap\fP [\-v] [\-n] [\-r] [\-h] \fIfilename\fP [ ... ]
.SH DESCRIPTION
.B getcap
-displays the name and capabilities of each specified
+displays the name and capabilities of each specified file.
.SH OPTIONS
.TP 4
.B \-h
--
2.26.2
^ permalink raw reply related
* [PATCH 07/15] Manual pages: getcap.8: Fix a clumsily worded sentence
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getcap.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/getcap.8 b/doc/getcap.8
index 367d010..2ad8092 100644
--- a/doc/getcap.8
+++ b/doc/getcap.8
@@ -20,7 +20,7 @@ a file's capabilities.
enables recursive search.
.TP 4
.B \-v
-enables to display all searched entries, even if it has no file-capabilities.
+display all searched entries, even if the have no file-capabilities.
.TP 4
.IR filename
One file per line.
--
2.26.2
^ permalink raw reply related
* [PATCH 08/15] Manual pages: getpcaps.8: Format options as a hanging list
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Make the options list more readable.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getpcaps.8 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/getpcaps.8 b/doc/getpcaps.8
index 7b73e86..fb3bc65 100644
--- a/doc/getpcaps.8
+++ b/doc/getpcaps.8
@@ -23,13 +23,13 @@ the
format.
.PP
Optional arguments:
-.PP
+.TP
.BR \-\-help " or " \-\-usage
Displays usage information and exits.
-.PP
+.TP
.BR \-\-ugly " or " \-\-legacy
Displays output in a somewhat ugly legacy format.
-.PP
+.TP
.B \-\-verbose
Displays usage in a legacy-like format but not quite so ugly in modern
default terminal fonts.
--
2.26.2
^ permalink raw reply related
* [PATCH 09/15] Manual pages: getpcaps.8: Remove a stray .br macro
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getpcaps.8 | 1 -
1 file changed, 1 deletion(-)
diff --git a/doc/getpcaps.8 b/doc/getpcaps.8
index fb3bc65..dadd365 100644
--- a/doc/getpcaps.8
+++ b/doc/getpcaps.8
@@ -36,7 +36,6 @@ default terminal fonts.
.SH SEE ALSO
.BR capabilities (7),
.BR capsh "(8), " setcap "(8) and " getcap (8).
-.br
.SH AUTHOR
This manual page was originally written by Robert Bihlmeyer
<robbe@debian.org>, for the Debian GNU/Linux system (but may be used
--
2.26.2
^ permalink raw reply related
* [PATCH 10/15] Manual pages: getpcaps.8: SEE ALSO: fix section number for capsh
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
capsh is in Section 1, not Section 8. Also, reformat the SEE ALSO list
in a more conventional way.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/getpcaps.8 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/doc/getpcaps.8 b/doc/getpcaps.8
index dadd365..d519357 100644
--- a/doc/getpcaps.8
+++ b/doc/getpcaps.8
@@ -34,8 +34,10 @@ Displays output in a somewhat ugly legacy format.
Displays usage in a legacy-like format but not quite so ugly in modern
default terminal fonts.
.SH SEE ALSO
+.BR capsh (1),
.BR capabilities (7),
-.BR capsh "(8), " setcap "(8) and " getcap (8).
+.BR getcap (8),
+.BR setcap (8)
.SH AUTHOR
This manual page was originally written by Robert Bihlmeyer
<robbe@debian.org>, for the Debian GNU/Linux system (but may be used
--
2.26.2
^ permalink raw reply related
* [PATCH 11/15] Manual pages: setcap.8: Typo fix
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/setcap.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/setcap.8 b/doc/setcap.8
index ae044aa..582c781 100644
--- a/doc/setcap.8
+++ b/doc/setcap.8
@@ -39,7 +39,7 @@ is used to remove a capability set from a file. Note, setting an empty
capability set is
.B not the same
as removing it. An empty set can be used to guarantee a file is not
-executed with privilege inspite of the fact that the prevailing
+executed with privilege in spite of the fact that the prevailing
ambient+inheritable sets would otherwise bestow capabilities on
executed binaries.
.PP
--
2.26.2
^ permalink raw reply related
* [PATCH 12/15] Manual pages: cap_get_file.3: NOTES: note the effect of the Ambient set
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
The addition of Ambient capabilities in Linux 4.3 rendered the text on
the effect of the Effective bit during execve(2) out-of-date. Fix that.
Also add a couple of paragraph breaks to improve readability.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_file.3 | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/doc/cap_get_file.3 b/doc/cap_get_file.3
index ceacbaf..dc7b571 100644
--- a/doc/cap_get_file.3
+++ b/doc/cap_get_file.3
@@ -103,13 +103,18 @@ or
These functions are specified by withdrawn POSIX.1e draft specification.
.SH NOTES
Support for file capabilities is provided on Linux since version 2.6.24.
-
+.PP
On Linux, the file Effective set is a single bit.
If it is enabled, then all Permitted capabilities are enabled
in the Effective set of the calling process when the file is executed;
-otherwise, no capabilities are enabled in the process's Effective set
+otherwise, the process's Ambient capabilities
+(or, before the Linux 4.3 addition of Ambient capabilities, no capabilities)
+are enabled in the process's Effective set
following an
-.BR execve (2).
+.BR execve (2)
+(see
+.BR capabilities (7)).
+.PP
Because the file Effective set is a single bit,
if any capability is enabled in the Effective set of the
.I cap_t
--
2.26.2
^ permalink raw reply related
* [PATCH 13/15] Manual pages: cap_get_proc.3: Update description of capsetp()
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
The details currently provided for capsetp() were current before 2008,
but ceased to be accurate with the 2008 addition of VFS file
capabilities in 2008. Update the text accordingly.
At the same time, add a subheading, a few paragraph breaks, and a few
other wording tweaks to make the description of capgetp() and capsetp()
more readable.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_proc.3 | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index fce8f59..40475fd 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -251,7 +251,7 @@ or,
When linked this way, due to linker magic, libcap uses
.BR psx_syscall "(3) and " psx_syscall6 (3)
to perform state setting system calls.
-.PP
+.SS capgetp() and capsetp()
The library also supports the deprecated functions:
.PP
.BI "int capgetp(pid_t " pid ", cap_t " cap_d );
@@ -264,14 +264,20 @@ capabilities in a pre-allocated
.IR cap_d .
See
.BR cap_init ()
-for information on allocating an empty capability set. This function,
-.BR capgetp (),
-is deprecated, you should use
+for information on allocating an empty capability set. This function
+is deprecated; you should use
.BR cap_get_pid ().
.PP
.BR capsetp ()
-attempts to set the capabilities of some other process(es),
-.IR pid .
+attempts to set the capabilities of the calling porcess or of
+some other process(es),
+.IR pid .
+Note that setting capabilities of another process is only possible on older
+kernels that do not provide VFS support for setting file capabilities.
+See
+.BR capset (2)
+for information on which kernels provide such support.
+.PP
If
.I pid
is positive it refers to a specific process; if it is zero, it refers
@@ -280,29 +286,37 @@ calling process and process '1' (typically
.BR init (8));
other negative values refer to the
.I \-pid
-process group. In order to use this function, the kernel must support
+process group.
+.PP
+In order to use this function, the kernel must support
it and the calling process must have
.B CAP_SETPCAP
raised in its Effective capability set. The capabilities set in the
target process(es) are those contained in
.IR cap_d .
+.PP
Kernels that support filesystem capabilities redefine the semantics of
.B CAP_SETPCAP
-and on such systems this function will always fail for any target not
+and on such systems,
+.BR capsetp ()
+will always fail for any target not
equal to the calling process.
.BR capsetp ()
returns zero for success, and \-1 on failure.
-
-Where supported by the kernel, the function
+.PP
+On kernels where it is (was) supported,
.BR capsetp ()
should be used with care. It existed, primarily, to overcome an early
lack of support for capabilities in the filesystems supported by
-Linux. Note that, by default, the only processes that have
+Linux. Note that on older kernels where
+.BR capsetp ()
+could be used to set the capabilities of another process,
+the only processes that had
.B CAP_SETPCAP
-available to them are processes started as a kernel thread.
+available to them by default were processes started as kernel threads.
(Typically this includes
.BR init (8),
-kflushd and kswapd.) You will need to recompile the kernel to modify
+kflushd and kswapd.) A kernel recompilation was needed to modify
this default.
.SH EXAMPLE
The code segment below raises the
--
2.26.2
^ permalink raw reply related
* [PATCH 14/15] Manual pages: cap_get_proc.3, capsh.1: Use "UID" and "GID" consistently
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Replace terms such as "uid" and "use-id" with the more conventional
abbreviation UID. Similarly for GID.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/cap_get_proc.3 | 2 +-
doc/capsh.1 | 18 ++++++++++--------
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/doc/cap_get_proc.3 b/doc/cap_get_proc.3
index 40475fd..74e5e8c 100644
--- a/doc/cap_get_proc.3
+++ b/doc/cap_get_proc.3
@@ -349,7 +349,7 @@ effective capabilities for the caller:
.fi
Alternatively, to completely drop privilege in a program launched
-setuid-root but wanting to run as a specific user-id etc. in such a
+setuid-root but wanting to run as a specific user ID etc. in such a
way that neither it, nor any of its children can acquire privilege
again:
.nf
diff --git a/doc/capsh.1 b/doc/capsh.1
index f19a3ea..d124889 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -67,7 +67,7 @@ in its effective set.
.TP
.BI \-\-user= username
Assume the identity of the named user. That is, look up the user's
-.IR uid " and " gid
+UID and GID
with
.BR getpwuid (3)
and their group memberships with
@@ -97,7 +97,7 @@ or exit with a status 1.
.TP
.BI \-\-uid= id
Force all
-.B uid
+UID
values to equal
.I id
using the
@@ -108,18 +108,19 @@ effective set.
.BR \-\-cap\-uid= <uid>
use the
.BR cap_setuid (3)
-function to set the uid of the current process. This performs all
-preparations for setting the uid without dropping capabilities in the
+function to set the UID of the current process. This performs all
+preparations for setting the UID without dropping capabilities in the
process. Following this command the prevailing effective capabilities
will be lowered.
.TP
.BI \-\-is\-uid= <id>
Exit with status 1 unless the current
-.IR uid " equals " <id> .
+UID equals
+.IR <id> .
.TP
.BI \-\-gid= <id>
Force all
-.B gid
+GID
values to equal
.I id
using the
@@ -128,7 +129,8 @@ system call.
.TP
.BI \-\-is\-gid= <id>
Exit with status 1 unless the current
-.IR gid " equals " <id> .
+GIQ equals
+.IR <id> .
.TP
.BI \-\-groups= <gid-list>
Set the supplementary groups to the numerical list provided. The
@@ -142,7 +144,7 @@ for a more convenient way of doing this.
In a non-pure capability mode, the kernel provides liberal privilege
to the super-user. However, it is normally the case that when the
super-user changes
-.I uid
+UID
to some lesser user, then capabilities are dropped. For these
situations, the kernel can permit the process to retain its
capabilities after a
--
2.26.2
^ permalink raw reply related
* [PATCH 15/15] Manual pages: capsh.1: Change .TP indent to the default
From: Michael Kerrisk (man-pages) @ 2020-07-20 9:13 UTC (permalink / raw)
To: mtk.manpages, Andrew Morgan; +Cc: linux-security-module
In-Reply-To: <20200720091328.290336-1-mtk.manpages@gmail.com>
Currently, the long list of options in this page is formatted as a
hanging list with a very deep indent (22), which causes the rendered
text to be rather narrow. That's uncomfortable when viewing on
something other than an 80 column display, and also causes some
ugliness in line breaks and line filling. Change to the more
traditional default indentation for .TP.
Signed-off-by: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
---
doc/capsh.1 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/capsh.1 b/doc/capsh.1
index d124889..b02793b 100644
--- a/doc/capsh.1
+++ b/doc/capsh.1
@@ -13,7 +13,7 @@ debugging features useful for summarizing capability state.
.B capsh
takes a number of optional arguments, acting on them in the
order they are provided. They are as follows:
-.TP 22
+.TP
.B \-\-help
Display the list of commands supported by
.BR capsh .
--
2.26.2
^ permalink raw reply related
* Re: [PATCH bpf-next v4 1/4] bpf: Generalize bpf_sk_storage
From: kernel test robot @ 2020-07-20 11:18 UTC (permalink / raw)
To: KP Singh, linux-kernel, bpf, linux-security-module
Cc: kbuild-all, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Paul Turner, Jann Horn, Florent Revest
In-Reply-To: <20200709101239.3829793-2-kpsingh@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]
Hi KP,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/KP-Singh/Generalizing-bpf_local_storage/20200709-181906
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: s390-randconfig-s032-20200719 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-49-g707c5017-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> kernel/bpf/bpf_local_storage.c:237:16: sparse: sparse: non size-preserving integer to pointer cast
vim +237 kernel/bpf/bpf_local_storage.c
221
222 /* Publish local_storage to the address. This is used because we are already
223 * in a region where we cannot grab a lock on the object owning the storage (
224 * (e.g sk->sk_lock). Hence, atomic ops is used.
225 *
226 * From now on, the addr pointer is protected
227 * by the local_storage->lock. Hence, upon freeing,
228 * the local_storage->lock must be held before unlinking the storage from the
229 * owner.
230 */
231 int bpf_local_storage_publish(struct bpf_local_storage_elem *first_selem,
232 struct bpf_local_storage **addr,
233 struct bpf_local_storage *curr)
234 {
235 struct bpf_local_storage *prev;
236
> 237 prev = cmpxchg(addr, NULL, curr);
238 if (unlikely(prev)) {
239 /* Note that even first_selem was linked to smap's
240 * bucket->list, first_selem can be freed immediately
241 * (instead of kfree_rcu) because
242 * bpf_local_storage_map_free() does a
243 * synchronize_rcu() before walking the bucket->list.
244 * Hence, no one is accessing selem from the
245 * bucket->list under rcu_read_lock().
246 */
247 bpf_selem_unlink_map(first_selem);
248 return -EAGAIN;
249 }
250
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27346 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE
From: Christian Brauner @ 2020-07-20 11:54 UTC (permalink / raw)
To: Adrian Reber, Nicolas Viennot
Cc: Eric Biederman, Pavel Emelyanov, Oleg Nesterov, Dmitry Safonov,
Andrei Vagin, Michał Cłapiński, Kamil Yurtsever,
Dirk Petersen, Christine Flood, Casey Schaufler, Mike Rapoport,
Radostin Stoyanov, Cyrill Gorcunov, Serge Hallyn, Stephen Smalley,
Sargun Dhillon, Arnd Bergmann, linux-security-module,
linux-kernel, selinux, Eric Paris, Jann Horn, linux-fsdevel
In-Reply-To: <20200719181729.6f37lilhvov5a74f@wittgenstein>
On Sun, Jul 19, 2020 at 08:17:30PM +0200, Christian Brauner wrote:
> On Sun, Jul 19, 2020 at 12:04:10PM +0200, Adrian Reber wrote:
> > This is v6 of the 'Introduce CAP_CHECKPOINT_RESTORE' patchset. The
> > changes to v5 are:
> >
> > * split patch dealing with /proc/self/exe into two patches:
> > * first patch to enable changing it with CAP_CHECKPOINT_RESTORE
> > and detailed history in the commit message
> > * second patch changes -EINVAL to -EPERM
> > * use kselftest_harness.h infrastructure for test
> > * replace if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE))
> > with if (!checkpoint_restore_ns_capable(&init_user_ns))
> >
> > Adrian Reber (5):
> > capabilities: Introduce CAP_CHECKPOINT_RESTORE
> > pid: use checkpoint_restore_ns_capable() for set_tid
> > pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid
> > proc: allow access in init userns for map_files with
> > CAP_CHECKPOINT_RESTORE
> > selftests: add clone3() CAP_CHECKPOINT_RESTORE test
> >
> > Nicolas Viennot (2):
> > prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe
> > prctl: exe link permission error changed from -EINVAL to -EPERM
> >
> > fs/proc/base.c | 8 +-
> > include/linux/capability.h | 6 +
> > include/uapi/linux/capability.h | 9 +-
> > kernel/pid.c | 2 +-
> > kernel/pid_namespace.c | 2 +-
> > kernel/sys.c | 13 +-
> > security/selinux/include/classmap.h | 5 +-
> > tools/testing/selftests/clone3/.gitignore | 1 +
> > tools/testing/selftests/clone3/Makefile | 4 +-
> > .../clone3/clone3_cap_checkpoint_restore.c | 177 ++++++++++++++++++
> > 10 files changed, 212 insertions(+), 15 deletions(-)
> > create mode 100644 tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> >
> > base-commit: d31958b30ea3b7b6e522d6bf449427748ad45822
>
> Adrian, Nicolas thank you!
> I grabbed the series to run the various core test-suites we've added
> over the last year and pushed it to
> https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=cap_checkpoint_restore
> for now to let kbuild/ltp chew on it for a bit.
Ok, I ran the test-suite this morning and there's nothing to worry about
it all passes _but_ the selftests had a bug using SKIP() instead of
XFAIL() and they mixed ksft_print_msg() and TH_LOG(). I know that I
think I mentioned to you that you can't use TH_LOG() outside of TEST*().
Turns out I was wrong. You can do it if you pass in a specific global
variable. Here's the diff I applied on top of the selftests you sent.
After these changes the output looks like this:
[==========] Running 1 tests from 1 test cases.
[ RUN ] global.clone3_cap_checkpoint_restore
# clone3() syscall supported
clone3_cap_checkpoint_restore.c:155:clone3_cap_checkpoint_restore:Child has PID 12303
clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
clone3_cap_checkpoint_restore.c:55:clone3_cap_checkpoint_restore:Operation not permitted - Failed to create new process
clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:-1
clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
clone3_cap_checkpoint_restore.c:70:clone3_cap_checkpoint_restore:I am the parent (12302). My child's pid is 12303
clone3_cap_checkpoint_restore.c:63:clone3_cap_checkpoint_restore:I am the child, my PID is 12303 (expected 12303)
clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:0
[ OK ] global.clone3_cap_checkpoint_restore
[==========] 1 / 1 tests passed.
[ PASSED ]
Ok with this below being applied on top of it?
diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
index c0d83511cd28..9562425aa0a9 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -38,7 +38,8 @@ static void child_exit(int ret)
_exit(ret);
}
-static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
+static int call_clone3_set_tid(struct __test_metadata *_metadata,
+ pid_t *set_tid, size_t set_tid_size)
{
int status;
pid_t pid = -1;
@@ -51,7 +52,7 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
pid = sys_clone3(&args, sizeof(struct clone_args));
if (pid < 0) {
- ksft_print_msg("%s - Failed to create new process\n", strerror(errno));
+ TH_LOG("%s - Failed to create new process", strerror(errno));
return -errno;
}
@@ -59,18 +60,17 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
int ret;
char tmp = 0;
- ksft_print_msg
- ("I am the child, my PID is %d (expected %d)\n", getpid(), set_tid[0]);
+ TH_LOG("I am the child, my PID is %d (expected %d)", getpid(), set_tid[0]);
if (set_tid[0] != getpid())
child_exit(EXIT_FAILURE);
child_exit(EXIT_SUCCESS);
}
- ksft_print_msg("I am the parent (%d). My child's pid is %d\n", getpid(), pid);
+ TH_LOG("I am the parent (%d). My child's pid is %d", getpid(), pid);
if (waitpid(pid, &status, 0) < 0) {
- ksft_print_msg("Child returned %s\n", strerror(errno));
+ TH_LOG("Child returned %s", strerror(errno));
return -errno;
}
@@ -80,13 +80,14 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
return WEXITSTATUS(status);
}
-static int test_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
+static int test_clone3_set_tid(struct __test_metadata *_metadata,
+ pid_t *set_tid, size_t set_tid_size)
{
int ret;
- ksft_print_msg("[%d] Trying clone3() with CLONE_SET_TID to %d\n", getpid(), set_tid[0]);
- ret = call_clone3_set_tid(set_tid, set_tid_size);
- ksft_print_msg("[%d] clone3() with CLONE_SET_TID %d says:%d\n", getpid(), set_tid[0], ret);
+ TH_LOG("[%d] Trying clone3() with CLONE_SET_TID to %d", getpid(), set_tid[0]);
+ ret = call_clone3_set_tid(_metadata, set_tid, set_tid_size);
+ TH_LOG("[%d] clone3() with CLONE_SET_TID %d says:%d", getpid(), set_tid[0], ret);
return ret;
}
@@ -144,7 +145,7 @@ TEST(clone3_cap_checkpoint_restore)
test_clone3_supported();
EXPECT_EQ(getuid(), 0)
- SKIP(return, "Skipping all tests as non-root\n");
+ XFAIL(return, "Skipping all tests as non-root\n");
memset(&set_tid, 0, sizeof(set_tid));
@@ -162,16 +163,20 @@ TEST(clone3_cap_checkpoint_restore)
ASSERT_EQ(set_capability(), 0)
TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
- prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
- setgid(1000);
- setuid(1000);
+
+ ASSERT_EQ(prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0), 0);
+
+ EXPECT_EQ(setgid(65534), 0)
+ TH_LOG("Failed to setgid(65534)");
+ ASSERT_EQ(setuid(65534), 0);
+
set_tid[0] = pid;
/* This would fail without CAP_CHECKPOINT_RESTORE */
- ASSERT_EQ(test_clone3_set_tid(set_tid, 1), -EPERM);
+ ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), -EPERM);
ASSERT_EQ(set_capability(), 0)
TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
/* This should work as we have CAP_CHECKPOINT_RESTORE as non-root */
- ASSERT_EQ(test_clone3_set_tid(set_tid, 1), 0);
+ ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), 0);
}
TEST_HARNESS_MAIN
^ permalink raw reply related
* Re: [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE
From: Adrian Reber @ 2020-07-20 12:46 UTC (permalink / raw)
To: Christian Brauner
Cc: Nicolas Viennot, Eric Biederman, Pavel Emelyanov, Oleg Nesterov,
Dmitry Safonov, Andrei Vagin, Michał Cłapiński,
Kamil Yurtsever, Dirk Petersen, Christine Flood, Casey Schaufler,
Mike Rapoport, Radostin Stoyanov, Cyrill Gorcunov, Serge Hallyn,
Stephen Smalley, Sargun Dhillon, Arnd Bergmann,
linux-security-module, linux-kernel, selinux, Eric Paris,
Jann Horn, linux-fsdevel
In-Reply-To: <20200720115452.ne2vqtdneuungb3j@wittgenstein>
On Mon, Jul 20, 2020 at 01:54:52PM +0200, Christian Brauner wrote:
> On Sun, Jul 19, 2020 at 08:17:30PM +0200, Christian Brauner wrote:
> > On Sun, Jul 19, 2020 at 12:04:10PM +0200, Adrian Reber wrote:
> > > This is v6 of the 'Introduce CAP_CHECKPOINT_RESTORE' patchset. The
> > > changes to v5 are:
> > >
> > > * split patch dealing with /proc/self/exe into two patches:
> > > * first patch to enable changing it with CAP_CHECKPOINT_RESTORE
> > > and detailed history in the commit message
> > > * second patch changes -EINVAL to -EPERM
> > > * use kselftest_harness.h infrastructure for test
> > > * replace if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE))
> > > with if (!checkpoint_restore_ns_capable(&init_user_ns))
> > >
> > > Adrian Reber (5):
> > > capabilities: Introduce CAP_CHECKPOINT_RESTORE
> > > pid: use checkpoint_restore_ns_capable() for set_tid
> > > pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid
> > > proc: allow access in init userns for map_files with
> > > CAP_CHECKPOINT_RESTORE
> > > selftests: add clone3() CAP_CHECKPOINT_RESTORE test
> > >
> > > Nicolas Viennot (2):
> > > prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe
> > > prctl: exe link permission error changed from -EINVAL to -EPERM
> > >
> > > fs/proc/base.c | 8 +-
> > > include/linux/capability.h | 6 +
> > > include/uapi/linux/capability.h | 9 +-
> > > kernel/pid.c | 2 +-
> > > kernel/pid_namespace.c | 2 +-
> > > kernel/sys.c | 13 +-
> > > security/selinux/include/classmap.h | 5 +-
> > > tools/testing/selftests/clone3/.gitignore | 1 +
> > > tools/testing/selftests/clone3/Makefile | 4 +-
> > > .../clone3/clone3_cap_checkpoint_restore.c | 177 ++++++++++++++++++
> > > 10 files changed, 212 insertions(+), 15 deletions(-)
> > > create mode 100644 tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> > >
> > > base-commit: d31958b30ea3b7b6e522d6bf449427748ad45822
> >
> > Adrian, Nicolas thank you!
> > I grabbed the series to run the various core test-suites we've added
> > over the last year and pushed it to
> > https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=cap_checkpoint_restore
> > for now to let kbuild/ltp chew on it for a bit.
>
> Ok, I ran the test-suite this morning and there's nothing to worry about
> it all passes _but_ the selftests had a bug using SKIP() instead of
> XFAIL() and they mixed ksft_print_msg() and TH_LOG(). I know that I
> think I mentioned to you that you can't use TH_LOG() outside of TEST*().
> Turns out I was wrong. You can do it if you pass in a specific global
> variable. Here's the diff I applied on top of the selftests you sent.
> After these changes the output looks like this:
>
> [==========] Running 1 tests from 1 test cases.
> [ RUN ] global.clone3_cap_checkpoint_restore
> # clone3() syscall supported
> clone3_cap_checkpoint_restore.c:155:clone3_cap_checkpoint_restore:Child has PID 12303
> clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
> clone3_cap_checkpoint_restore.c:55:clone3_cap_checkpoint_restore:Operation not permitted - Failed to create new process
> clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:-1
> clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
> clone3_cap_checkpoint_restore.c:70:clone3_cap_checkpoint_restore:I am the parent (12302). My child's pid is 12303
> clone3_cap_checkpoint_restore.c:63:clone3_cap_checkpoint_restore:I am the child, my PID is 12303 (expected 12303)
> clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:0
> [ OK ] global.clone3_cap_checkpoint_restore
> [==========] 1 / 1 tests passed.
> [ PASSED ]
>
> Ok with this below being applied on top of it?
>
> diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> index c0d83511cd28..9562425aa0a9 100644
> --- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> +++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> @@ -38,7 +38,8 @@ static void child_exit(int ret)
> _exit(ret);
> }
>
> -static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> +static int call_clone3_set_tid(struct __test_metadata *_metadata,
> + pid_t *set_tid, size_t set_tid_size)
> {
> int status;
> pid_t pid = -1;
> @@ -51,7 +52,7 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
>
> pid = sys_clone3(&args, sizeof(struct clone_args));
> if (pid < 0) {
> - ksft_print_msg("%s - Failed to create new process\n", strerror(errno));
> + TH_LOG("%s - Failed to create new process", strerror(errno));
> return -errno;
> }
>
> @@ -59,18 +60,17 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> int ret;
> char tmp = 0;
>
> - ksft_print_msg
> - ("I am the child, my PID is %d (expected %d)\n", getpid(), set_tid[0]);
> + TH_LOG("I am the child, my PID is %d (expected %d)", getpid(), set_tid[0]);
>
> if (set_tid[0] != getpid())
> child_exit(EXIT_FAILURE);
> child_exit(EXIT_SUCCESS);
> }
>
> - ksft_print_msg("I am the parent (%d). My child's pid is %d\n", getpid(), pid);
> + TH_LOG("I am the parent (%d). My child's pid is %d", getpid(), pid);
>
> if (waitpid(pid, &status, 0) < 0) {
> - ksft_print_msg("Child returned %s\n", strerror(errno));
> + TH_LOG("Child returned %s", strerror(errno));
> return -errno;
> }
>
> @@ -80,13 +80,14 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> return WEXITSTATUS(status);
> }
>
> -static int test_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> +static int test_clone3_set_tid(struct __test_metadata *_metadata,
> + pid_t *set_tid, size_t set_tid_size)
> {
> int ret;
>
> - ksft_print_msg("[%d] Trying clone3() with CLONE_SET_TID to %d\n", getpid(), set_tid[0]);
> - ret = call_clone3_set_tid(set_tid, set_tid_size);
> - ksft_print_msg("[%d] clone3() with CLONE_SET_TID %d says:%d\n", getpid(), set_tid[0], ret);
> + TH_LOG("[%d] Trying clone3() with CLONE_SET_TID to %d", getpid(), set_tid[0]);
> + ret = call_clone3_set_tid(_metadata, set_tid, set_tid_size);
> + TH_LOG("[%d] clone3() with CLONE_SET_TID %d says:%d", getpid(), set_tid[0], ret);
> return ret;
> }
>
> @@ -144,7 +145,7 @@ TEST(clone3_cap_checkpoint_restore)
> test_clone3_supported();
>
> EXPECT_EQ(getuid(), 0)
> - SKIP(return, "Skipping all tests as non-root\n");
> + XFAIL(return, "Skipping all tests as non-root\n");
>
> memset(&set_tid, 0, sizeof(set_tid));
>
> @@ -162,16 +163,20 @@ TEST(clone3_cap_checkpoint_restore)
>
> ASSERT_EQ(set_capability(), 0)
> TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
> - prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
> - setgid(1000);
> - setuid(1000);
> +
> + ASSERT_EQ(prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0), 0);
> +
> + EXPECT_EQ(setgid(65534), 0)
> + TH_LOG("Failed to setgid(65534)");
> + ASSERT_EQ(setuid(65534), 0);
> +
> set_tid[0] = pid;
> /* This would fail without CAP_CHECKPOINT_RESTORE */
> - ASSERT_EQ(test_clone3_set_tid(set_tid, 1), -EPERM);
> + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), -EPERM);
> ASSERT_EQ(set_capability(), 0)
> TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
> /* This should work as we have CAP_CHECKPOINT_RESTORE as non-root */
> - ASSERT_EQ(test_clone3_set_tid(set_tid, 1), 0);
> + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), 0);
> }
>
Thanks for the changes. That looks much better.
Can you fix the test directly or do you need a new reworked patch from us?
Adrian
^ permalink raw reply
* Re: [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE
From: Christian Brauner @ 2020-07-20 12:58 UTC (permalink / raw)
To: Adrian Reber
Cc: Nicolas Viennot, Eric Biederman, Pavel Emelyanov, Oleg Nesterov,
Dmitry Safonov, Andrei Vagin, Michał Cłapiński,
Kamil Yurtsever, Dirk Petersen, Christine Flood, Casey Schaufler,
Mike Rapoport, Radostin Stoyanov, Cyrill Gorcunov, Serge Hallyn,
Stephen Smalley, Sargun Dhillon, Arnd Bergmann,
linux-security-module, linux-kernel, selinux, Eric Paris,
Jann Horn, linux-fsdevel
In-Reply-To: <20200720124637.GA1788746@dcbz.redhat.com>
On Mon, Jul 20, 2020 at 02:46:37PM +0200, Adrian Reber wrote:
> On Mon, Jul 20, 2020 at 01:54:52PM +0200, Christian Brauner wrote:
> > On Sun, Jul 19, 2020 at 08:17:30PM +0200, Christian Brauner wrote:
> > > On Sun, Jul 19, 2020 at 12:04:10PM +0200, Adrian Reber wrote:
> > > > This is v6 of the 'Introduce CAP_CHECKPOINT_RESTORE' patchset. The
> > > > changes to v5 are:
> > > >
> > > > * split patch dealing with /proc/self/exe into two patches:
> > > > * first patch to enable changing it with CAP_CHECKPOINT_RESTORE
> > > > and detailed history in the commit message
> > > > * second patch changes -EINVAL to -EPERM
> > > > * use kselftest_harness.h infrastructure for test
> > > > * replace if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE))
> > > > with if (!checkpoint_restore_ns_capable(&init_user_ns))
> > > >
> > > > Adrian Reber (5):
> > > > capabilities: Introduce CAP_CHECKPOINT_RESTORE
> > > > pid: use checkpoint_restore_ns_capable() for set_tid
> > > > pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid
> > > > proc: allow access in init userns for map_files with
> > > > CAP_CHECKPOINT_RESTORE
> > > > selftests: add clone3() CAP_CHECKPOINT_RESTORE test
> > > >
> > > > Nicolas Viennot (2):
> > > > prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe
> > > > prctl: exe link permission error changed from -EINVAL to -EPERM
> > > >
> > > > fs/proc/base.c | 8 +-
> > > > include/linux/capability.h | 6 +
> > > > include/uapi/linux/capability.h | 9 +-
> > > > kernel/pid.c | 2 +-
> > > > kernel/pid_namespace.c | 2 +-
> > > > kernel/sys.c | 13 +-
> > > > security/selinux/include/classmap.h | 5 +-
> > > > tools/testing/selftests/clone3/.gitignore | 1 +
> > > > tools/testing/selftests/clone3/Makefile | 4 +-
> > > > .../clone3/clone3_cap_checkpoint_restore.c | 177 ++++++++++++++++++
> > > > 10 files changed, 212 insertions(+), 15 deletions(-)
> > > > create mode 100644 tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> > > >
> > > > base-commit: d31958b30ea3b7b6e522d6bf449427748ad45822
> > >
> > > Adrian, Nicolas thank you!
> > > I grabbed the series to run the various core test-suites we've added
> > > over the last year and pushed it to
> > > https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=cap_checkpoint_restore
> > > for now to let kbuild/ltp chew on it for a bit.
> >
> > Ok, I ran the test-suite this morning and there's nothing to worry about
> > it all passes _but_ the selftests had a bug using SKIP() instead of
> > XFAIL() and they mixed ksft_print_msg() and TH_LOG(). I know that I
> > think I mentioned to you that you can't use TH_LOG() outside of TEST*().
> > Turns out I was wrong. You can do it if you pass in a specific global
> > variable. Here's the diff I applied on top of the selftests you sent.
> > After these changes the output looks like this:
> >
> > [==========] Running 1 tests from 1 test cases.
> > [ RUN ] global.clone3_cap_checkpoint_restore
> > # clone3() syscall supported
> > clone3_cap_checkpoint_restore.c:155:clone3_cap_checkpoint_restore:Child has PID 12303
> > clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
> > clone3_cap_checkpoint_restore.c:55:clone3_cap_checkpoint_restore:Operation not permitted - Failed to create new process
> > clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:-1
> > clone3_cap_checkpoint_restore.c:88:clone3_cap_checkpoint_restore:[12302] Trying clone3() with CLONE_SET_TID to 12303
> > clone3_cap_checkpoint_restore.c:70:clone3_cap_checkpoint_restore:I am the parent (12302). My child's pid is 12303
> > clone3_cap_checkpoint_restore.c:63:clone3_cap_checkpoint_restore:I am the child, my PID is 12303 (expected 12303)
> > clone3_cap_checkpoint_restore.c:90:clone3_cap_checkpoint_restore:[12302] clone3() with CLONE_SET_TID 12303 says:0
> > [ OK ] global.clone3_cap_checkpoint_restore
> > [==========] 1 / 1 tests passed.
> > [ PASSED ]
> >
> > Ok with this below being applied on top of it?
> >
> > diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> > index c0d83511cd28..9562425aa0a9 100644
> > --- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> > +++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
> > @@ -38,7 +38,8 @@ static void child_exit(int ret)
> > _exit(ret);
> > }
> >
> > -static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> > +static int call_clone3_set_tid(struct __test_metadata *_metadata,
> > + pid_t *set_tid, size_t set_tid_size)
> > {
> > int status;
> > pid_t pid = -1;
> > @@ -51,7 +52,7 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> >
> > pid = sys_clone3(&args, sizeof(struct clone_args));
> > if (pid < 0) {
> > - ksft_print_msg("%s - Failed to create new process\n", strerror(errno));
> > + TH_LOG("%s - Failed to create new process", strerror(errno));
> > return -errno;
> > }
> >
> > @@ -59,18 +60,17 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> > int ret;
> > char tmp = 0;
> >
> > - ksft_print_msg
> > - ("I am the child, my PID is %d (expected %d)\n", getpid(), set_tid[0]);
> > + TH_LOG("I am the child, my PID is %d (expected %d)", getpid(), set_tid[0]);
> >
> > if (set_tid[0] != getpid())
> > child_exit(EXIT_FAILURE);
> > child_exit(EXIT_SUCCESS);
> > }
> >
> > - ksft_print_msg("I am the parent (%d). My child's pid is %d\n", getpid(), pid);
> > + TH_LOG("I am the parent (%d). My child's pid is %d", getpid(), pid);
> >
> > if (waitpid(pid, &status, 0) < 0) {
> > - ksft_print_msg("Child returned %s\n", strerror(errno));
> > + TH_LOG("Child returned %s", strerror(errno));
> > return -errno;
> > }
> >
> > @@ -80,13 +80,14 @@ static int call_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> > return WEXITSTATUS(status);
> > }
> >
> > -static int test_clone3_set_tid(pid_t *set_tid, size_t set_tid_size)
> > +static int test_clone3_set_tid(struct __test_metadata *_metadata,
> > + pid_t *set_tid, size_t set_tid_size)
> > {
> > int ret;
> >
> > - ksft_print_msg("[%d] Trying clone3() with CLONE_SET_TID to %d\n", getpid(), set_tid[0]);
> > - ret = call_clone3_set_tid(set_tid, set_tid_size);
> > - ksft_print_msg("[%d] clone3() with CLONE_SET_TID %d says:%d\n", getpid(), set_tid[0], ret);
> > + TH_LOG("[%d] Trying clone3() with CLONE_SET_TID to %d", getpid(), set_tid[0]);
> > + ret = call_clone3_set_tid(_metadata, set_tid, set_tid_size);
> > + TH_LOG("[%d] clone3() with CLONE_SET_TID %d says:%d", getpid(), set_tid[0], ret);
> > return ret;
> > }
> >
> > @@ -144,7 +145,7 @@ TEST(clone3_cap_checkpoint_restore)
> > test_clone3_supported();
> >
> > EXPECT_EQ(getuid(), 0)
> > - SKIP(return, "Skipping all tests as non-root\n");
> > + XFAIL(return, "Skipping all tests as non-root\n");
> >
> > memset(&set_tid, 0, sizeof(set_tid));
> >
> > @@ -162,16 +163,20 @@ TEST(clone3_cap_checkpoint_restore)
> >
> > ASSERT_EQ(set_capability(), 0)
> > TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
> > - prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
> > - setgid(1000);
> > - setuid(1000);
> > +
> > + ASSERT_EQ(prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0), 0);
> > +
> > + EXPECT_EQ(setgid(65534), 0)
> > + TH_LOG("Failed to setgid(65534)");
> > + ASSERT_EQ(setuid(65534), 0);
> > +
> > set_tid[0] = pid;
> > /* This would fail without CAP_CHECKPOINT_RESTORE */
> > - ASSERT_EQ(test_clone3_set_tid(set_tid, 1), -EPERM);
> > + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), -EPERM);
> > ASSERT_EQ(set_capability(), 0)
> > TH_LOG("Could not set CAP_CHECKPOINT_RESTORE");
> > /* This should work as we have CAP_CHECKPOINT_RESTORE as non-root */
> > - ASSERT_EQ(test_clone3_set_tid(set_tid, 1), 0);
> > + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), 0);
> > }
> >
>
> Thanks for the changes. That looks much better.
>
> Can you fix the test directly or do you need a new reworked patch from us?
No, I squashed this into your commit, added a comment about my changes,
signed it off and am going to push it out for some more testing.
Thanks!
Christian
^ permalink raw reply
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
From: Stephen Smalley @ 2020-07-20 14:31 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
In-Reply-To: <20200717222819.26198-5-nramas@linux.microsoft.com>
On Fri, Jul 17, 2020 at 6:28 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> SELinux configuration and policy are some of the critical data for this
> security module that needs to be measured. To enable this measurement
> SELinux needs to implement the interface function,
> security_measure_data(), that the LSM can call.
>
> Define the security_measure_data() function in SELinux to measure SELinux
> configuration and policy. Call this function to measure SELinux data
> when there is a change in the security module's state.
>
> Sample measurement of SELinux state and hash of the policy:
>
> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
> 10 f4a7...9408 ima-buf sha256:4941...68fc selinux-policy-hash 8d1d...1834
>
> To verify the measurement check the following:
>
> Execute the following command to extract the measured data
> from the IMA log for SELinux configuration (selinux-state).
>
> cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-state" | cut -d' ' -f 6 | xxd -r -p
>
> The output should be the list of key-value pairs. For example,
> enabled=1;enforcing=0;checkreqprot=1;network_peer_controls=1;open_perms=1;extended_socket_class=1;always_check_network=0;cgroup_seclabel=1;nnp_nosuid_transition=1;genfs_seclabel_symlinks=0;
>
> To verify the measured data with the current SELinux state:
>
> => enabled should be set to 1 if /sys/fs/selinux folder exists,
> 0 otherwise
>
> For other entries, compare the integer value in the files
> => /sys/fs/selinux/enforce
> => /sys/fs/selinux/checkreqprot
> And, each of the policy capabilities files under
> => /sys/fs/selinux/policy_capabilities
>
> The data for selinux-policy-hash is the SHA256 hash of SELinux policy.
>
> To verify the measured data with the current SELinux policy run
> the following commands and verify the output hash values match.
>
> sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
>
> cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-policy-hash" | cut -d' ' -f 6
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> diff --git a/security/selinux/measure.c b/security/selinux/measure.c
> new file mode 100644
> index 000000000000..659011637ae7
> --- /dev/null
> +++ b/security/selinux/measure.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Measure SELinux state using IMA subsystem.
> + */
> +#include <linux/ima.h>
> +#include "security.h"
> +
> +/* Pre-allocated buffer used for measuring state */
> +static char *selinux_state_string;
> +static size_t selinux_state_string_len;
> +static char *str_format = "%s=%d;";
> +static int selinux_state_count;
> +
> +void __init selinux_init_measurement(void)
> +{
> + int i;
> +
> + /*
> + * enabled
> + * enforcing
> + * checkreqport
checkreqprot (spelling)
What about initialized? Or do you consider that to be implicitly
true/1 else we wouldn't be taking a measurement? Only caveat there is
that it provides one more means of disabling measurements (at the same
time as disabling enforcement) by setting it to false/0 via kernel
write flaw.
> + * All policy capability flags
> + */
> + selinux_state_count = 3 + __POLICYDB_CAPABILITY_MAX;
> +
> + selinux_state_string_len = snprintf(NULL, 0, str_format,
> + "enabled", 0);
> + selinux_state_string_len += snprintf(NULL, 0, str_format,
> + "enforcing", 0);
> + selinux_state_string_len += snprintf(NULL, 0, str_format,
> + "checkreqprot", 0);
> + for (i = 3; i < selinux_state_count; i++) {
> + selinux_state_string_len +=
> + snprintf(NULL, 0, str_format,
> + selinux_policycap_names[i-3], 0);
> + }
What's the benefit of this pattern versus just making the loop go from
0 to __POLICYDB_CAPABILITY_MAX and using selinux_policycap_names[i]?
> +void selinux_measure_state(struct selinux_state *selinux_state)
> +{
> + void *policy = NULL;
> + void *policy_hash = NULL;
> + size_t curr, buflen;
> + int i, policy_hash_len, rc = 0;
> +
> + if (!selinux_initialized(selinux_state)) {
> + pr_warn("%s: SELinux not yet initialized.\n", __func__);
> + return;
> + }
We could measure the global state variables before full SELinux
initialization (i.e. policy load).
Only the policy hash depends on having loaded the policy.
> +
> + if (!selinux_state_string) {
> + pr_warn("%s: Buffer for state not allocated.\n", __func__);
> + return;
> + }
> +
> + curr = snprintf(selinux_state_string, selinux_state_string_len,
> + str_format, "enabled",
> + !selinux_disabled(selinux_state));
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format, "enforcing",
> + enforcing_enabled(selinux_state));
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format, "checkreqprot",
> + selinux_checkreqprot(selinux_state));
> +
> + for (i = 3; i < selinux_state_count; i++) {
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format,
> + selinux_policycap_names[i - 3],
> + selinux_state->policycap[i - 3]);
> + }
Same question here as for the previous loop; seems cleaner to go from
0 to __POLICYDB_CAPABILITY_MAX and use [i].
What public git tree / branch would you recommend trying to use your
patches against? Didn't seem to apply to any of the obvious ones.
^ permalink raw reply
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
From: Lakshmi Ramasubramanian @ 2020-07-20 15:17 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
In-Reply-To: <CAEjxPJ7xQtZToF4d2w_o8SXFKG9kPZaWTWTFqyC-7GwBWnQa0A@mail.gmail.com>
On 7/20/20 7:31 AM, Stephen Smalley wrote:
>> +void __init selinux_init_measurement(void)
>> +{
>> + int i;
>> +
>> + /*
>> + * enabled
>> + * enforcing
>> + * checkreqport
>
> checkreqprot (spelling)
:( - will fix that.
>
> What about initialized? Or do you consider that to be implicitly
> true/1 else we wouldn't be taking a measurement? Only caveat there is
> that it provides one more means of disabling measurements (at the same
> time as disabling enforcement) by setting it to false/0 via kernel
> write flaw.
Yes - I was thinking measuring SELinux state would be meaningful only
when initialized is set to true/1.
I can include "initialized" as well in the measurement.
>
>> + * All policy capability flags
>> + */
>> + selinux_state_count = 3 + __POLICYDB_CAPABILITY_MAX;
>> +
>> + selinux_state_string_len = snprintf(NULL, 0, str_format,
>> + "enabled", 0);
>> + selinux_state_string_len += snprintf(NULL, 0, str_format,
>> + "enforcing", 0);
>> + selinux_state_string_len += snprintf(NULL, 0, str_format,
>> + "checkreqprot", 0);
>> + for (i = 3; i < selinux_state_count; i++) {
>> + selinux_state_string_len +=
>> + snprintf(NULL, 0, str_format,
>> + selinux_policycap_names[i-3], 0);
>> + }
>
> What's the benefit of this pattern versus just making the loop go from
> 0 to __POLICYDB_CAPABILITY_MAX and using selinux_policycap_names[i]?
No real benefit - I was just trying to use selinux_state_count.
I'll change the loop to go from 0 to POLICY_CAP_MAX
>
>> +void selinux_measure_state(struct selinux_state *selinux_state)
>> +{
>> + void *policy = NULL;
>> + void *policy_hash = NULL;
>> + size_t curr, buflen;
>> + int i, policy_hash_len, rc = 0;
>> +
>> + if (!selinux_initialized(selinux_state)) {
>> + pr_warn("%s: SELinux not yet initialized.\n", __func__);
>> + return;
>> + }
>
> We could measure the global state variables before full SELinux
> initialization (i.e. policy load).
> Only the policy hash depends on having loaded the policy.
Thanks for the information. I'll measure the state variables always and
measure policy only if "initialized" is true/1.
>
>> +
>> + if (!selinux_state_string) {
>> + pr_warn("%s: Buffer for state not allocated.\n", __func__);
>> + return;
>> + }
>> +
>> + curr = snprintf(selinux_state_string, selinux_state_string_len,
>> + str_format, "enabled",
>> + !selinux_disabled(selinux_state));
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format, "enforcing",
>> + enforcing_enabled(selinux_state));
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format, "checkreqprot",
>> + selinux_checkreqprot(selinux_state));
>> +
>> + for (i = 3; i < selinux_state_count; i++) {
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format,
>> + selinux_policycap_names[i - 3],
>> + selinux_state->policycap[i - 3]);
>> + }
>
> Same question here as for the previous loop; seems cleaner to go from
> 0 to __POLICYDB_CAPABILITY_MAX and use [i].
Will change it.
>
> What public git tree / branch would you recommend trying to use your
> patches against? Didn't seem to apply to any of the obvious ones.
>
Please try it on Mimi's next-integrity branch
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-integrity
You can try it on Linus's mainline as well if you apply the following
patch first (have mentioned that in the Cover letter as well)
https://patchwork.kernel.org/patch/11612989/
Thanks for trying out the changes. Please let me know the defects you find.
Just to let you know - I am making the following change (will update in
the next patch):
=> Save the last policy hash and state string in selinux_state struct.
=> Measure policy and hash only if it has changed since the last
measurement.
=> Also, suffix the IMA event name used with time stamp. For example,
10 e32e...5ac3 ima-buf sha256:86e8...4594
selinux-state-1595257807:874963248
656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
10 f4a7...9408 ima-buf sha256:4941...68fc
selinux-policy-hash-1595257807:874963248
8d1d...1834
The above will ensure the following sequence will be measured:
#1 State A - Measured
#2 Change from State A to State B - Measured
#3 Change from State B back to State A - Since the measured data is
same as in #1, the change will be measured only if the event name is
different between #1 and #3
thanks,
-lakshmi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox