On Tue, Jan 10, 2023 at 9:21 AM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > > On Tue, Jan 10, 2023 at 9:14 AM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > On Tue, 10 Jan 2023 at 14:18, Stephen Smalley > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > > > On Mon, Jan 9, 2023 at 12:17 PM Christian Göttsche > > > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > > > > > Add the public interfaces getpidprevcon(3) and getpidprevcon_raw(3), and > > > > the utility getpidprevcon to gather the previous context before the last > > > > exec of a given process. > > > > > > Wondering if we should warn in the manual page for this and other > > > getpid*con() interfaces that they are inherently racy and therefore > > > should never be relied upon for security purposes. > > > > With "and other getpid*con() interfaces" do you mean just getpidcon(3) > > (and getpidcon_raw(3)), which is currently the only interface matching > > your regex and operating on a foreign process, or also getcon(3) and > > getprevcon(3)? > > > > With race conditions you mean the possibility of the target process > > changing its context (via setcon(3)) or querying a replaced process > > (which is mainly possible via spawning a new process after setting > > /proc/sys/kernel/ns_last_pid)? > > Or do you have others in mind? > > Yes, just getpidcon/getpidcon_raw. And correct - the inherent race is > with respect to looking up a target process by pid, > when said process may potentially switch its context via setcon or > execve or exit and be replaced by a process of another context. > This was actually successfully done in Android against a service that > was using getpidcon() to obtain the context of the binder client > before support for passing contexts across binder IPC was added. See https://bugs.chromium.org/p/project-zero/issues/detail?id=1741 > > > > > > p.s.: > > after the discussion has settled I'll send a v2 with the interfaces > > actually published in libselinux.map > > > > > > > > > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > > > > --- > > > > libselinux/include/selinux/selinux.h | 5 ++++ > > > > libselinux/man/man3/getcon.3 | 9 +++++++ > > > > libselinux/man/man3/getpidprevcon.3 | 1 + > > > > libselinux/man/man3/getpidprevcon_raw.3 | 1 + > > > > libselinux/src/procattr.c | 18 ++++++++++++++ > > > > libselinux/utils/.gitignore | 1 + > > > > libselinux/utils/getpidprevcon.c | 33 +++++++++++++++++++++++++ > > > > 7 files changed, 68 insertions(+) > > > > create mode 100644 libselinux/man/man3/getpidprevcon.3 > > > > create mode 100644 libselinux/man/man3/getpidprevcon_raw.3 > > > > create mode 100644 libselinux/utils/getpidprevcon.c > > > > > > > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > > > > index 47af9953..a0948853 100644 > > > > --- a/libselinux/include/selinux/selinux.h > > > > +++ b/libselinux/include/selinux/selinux.h > > > > @@ -54,6 +54,11 @@ extern int getpidcon_raw(pid_t pid, char ** con); > > > > extern int getprevcon(char ** con); > > > > extern int getprevcon_raw(char ** con); > > > > > > > > +/* Get previous context (prior to last exec) of process identified by pid, and > > > > + set *con to refer to it. Caller must free via freecon. */ > > > > +extern int getpidprevcon(pid_t pid, char ** con); > > > > +extern int getpidprevcon_raw(pid_t pid, char ** con); > > > > + > > > > /* Get exec context, and set *con to refer to it. > > > > Sets *con to NULL if no exec context has been set, i.e. using default. > > > > If non-NULL, caller must free via freecon. */ > > > > diff --git a/libselinux/man/man3/getcon.3 b/libselinux/man/man3/getcon.3 > > > > index e7e394f3..38da958b 100644 > > > > --- a/libselinux/man/man3/getcon.3 > > > > +++ b/libselinux/man/man3/getcon.3 > > > > @@ -23,6 +23,10 @@ setcon \- set current security context of a process > > > > .sp > > > > .BI "int getpidcon_raw(pid_t " pid ", char **" context ); > > > > .sp > > > > +.BI "int getpidprevcon(pid_t " pid ", char **" context ); > > > > +.sp > > > > +.BI "int getpidprevcon_raw(pid_t " pid ", char **" context ); > > > > +.sp > > > > .BI "int getpeercon(int " fd ", char **" context ); > > > > .sp > > > > .BI "int getpeercon_raw(int " fd ", char **" context ); > > > > @@ -50,6 +54,11 @@ same as getcon but gets the context before the last exec. > > > > returns the process context for the specified PID, which must be free'd with > > > > .BR freecon (). > > > > > > > > +.TP > > > > +.BR getpidprevcon () > > > > +returns the process context before the last exec for the specified PID, which must be free'd with > > > > +.BR freecon (). > > > > + > > > > .TP > > > > .BR getpeercon () > > > > retrieves the context of the peer socket, which must be free'd with > > > > diff --git a/libselinux/man/man3/getpidprevcon.3 b/libselinux/man/man3/getpidprevcon.3 > > > > new file mode 100644 > > > > index 00000000..1210b5a0 > > > > --- /dev/null > > > > +++ b/libselinux/man/man3/getpidprevcon.3 > > > > @@ -0,0 +1 @@ > > > > +.so man3/getcon.3 > > > > diff --git a/libselinux/man/man3/getpidprevcon_raw.3 b/libselinux/man/man3/getpidprevcon_raw.3 > > > > new file mode 100644 > > > > index 00000000..1210b5a0 > > > > --- /dev/null > > > > +++ b/libselinux/man/man3/getpidprevcon_raw.3 > > > > @@ -0,0 +1 @@ > > > > +.so man3/getcon.3 > > > > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c > > > > index 6f4cfb82..b7a93a2b 100644 > > > > --- a/libselinux/src/procattr.c > > > > +++ b/libselinux/src/procattr.c > > > > @@ -300,3 +300,21 @@ int getpidcon(pid_t pid, char **c) > > > > } > > > > return getprocattrcon(c, pid, "current", NULL); > > > > } > > > > + > > > > +int getpidprevcon_raw(pid_t pid, char **c) > > > > +{ > > > > + if (pid <= 0) { > > > > + errno = EINVAL; > > > > + return -1; > > > > + } > > > > + return getprocattrcon_raw(c, pid, "prev", NULL); > > > > +} > > > > + > > > > +int getpidprevcon(pid_t pid, char **c) > > > > +{ > > > > + if (pid <= 0) { > > > > + errno = EINVAL; > > > > + return -1; > > > > + } > > > > + return getprocattrcon(c, pid, "prev", NULL); > > > > +} > > > > diff --git a/libselinux/utils/.gitignore b/libselinux/utils/.gitignore > > > > index 3ef34374..b19b94a8 100644 > > > > --- a/libselinux/utils/.gitignore > > > > +++ b/libselinux/utils/.gitignore > > > > @@ -9,6 +9,7 @@ getdefaultcon > > > > getenforce > > > > getfilecon > > > > getpidcon > > > > +getpidprevcon > > > > getsebool > > > > getseuser > > > > matchpathcon > > > > diff --git a/libselinux/utils/getpidprevcon.c b/libselinux/utils/getpidprevcon.c > > > > new file mode 100644 > > > > index 00000000..662ad500 > > > > --- /dev/null > > > > +++ b/libselinux/utils/getpidprevcon.c > > > > @@ -0,0 +1,33 @@ > > > > +#include <unistd.h> > > > > +#include <stdio.h> > > > > +#include <stdlib.h> > > > > +#include <string.h> > > > > +#include <errno.h> > > > > +#include <selinux/selinux.h> > > > > + > > > > +int main(int argc, char **argv) > > > > +{ > > > > + pid_t pid; > > > > + char *buf; > > > > + int rc; > > > > + > > > > + if (argc != 2) { > > > > + fprintf(stderr, "usage: %s pid\n", argv[0]); > > > > + exit(1); > > > > + } > > > > + > > > > + if (sscanf(argv[1], "%d", &pid) != 1) { > > > > + fprintf(stderr, "%s: invalid pid %s\n", argv[0], argv[1]); > > > > + exit(2); > > > > + } > > > > + > > > > + rc = getpidprevcon(pid, &buf); > > > > + if (rc < 0) { > > > > + fprintf(stderr, "%s: getpidprevcon() failed: %s\n", argv[0], strerror(errno)); > > > > + exit(3); > > > > + } > > > > + > > > > + printf("%s\n", buf); > > > > + freecon(buf); > > > > + exit(EXIT_SUCCESS); > > > > +} > > > > -- > > > > 2.39.0 > > > >
