Re: Addition to kauth(9) framework
If all listerners unshare kauth_cred_t *unconditionally*, we lost data set by kauth_cred_setdata. As I said later there is a workaround (kauth_cred_getrefcnt or kauth_cred_copy) but I don't like it. why don't you like it? I cannot imagine applications for KAUTH_CRED_CHROOT other than adding some information to kauth_cred_t, e.g. root directory, chroot serial number or something equivalent for some purposes. So, a code for unsharing kauth_cred_t should *always* be called by *all* listerers/modules before modification. In my opinion this adds unnecessary overcomplication for no benefits (unsharing credentials in chroot(2) unconditionally cannot cause performance degradation). This is why I think it's better and easier to unshare it in one place, that is in chroot(2). -- Best regards, Aleksey Cheusov.
Re: Addition to kauth(9) framework
[good explanation deleted] Yeah, that part I did get. But: The question is *where* new kauth_cred_t instance should be created and assigned to the process: 1) Inside chroot/fchroot(2) (this is in my patch) 2) Modules that adds credential private data. Is the kauth_t passed to the securchroot secmodule (are all other listeners) by value or by reference (at least conceptually). It has to be by reference, isn't it? It is passed by reference. sys/types.h: typedef struct kauth_cred *kauth_cred_t You said choosing (2) over (1) would lead to problems in case we have multiple listeners and I fail to understand how, If all listerners unshare kauth_cred_t *unconditionally*, we lost data set by kauth_cred_setdata. As I said later there is a workaround (kauth_cred_getrefcnt or kauth_cred_copy) but I don't like it. in that case, choosing (1) over (2) does not lead to (different) problems. I don't see any problem with (1) -- Best regards, Aleksey Cheusov.
Re: Addition to kauth(9) framework
On Tue, Aug 30, 2011 at 10:24:08AM +0300, Aleksey Cheusov wrote: If all listerners unshare kauth_cred_t *unconditionally*, we lost data set by kauth_cred_setdata. As I said later there is a workaround (kauth_cred_getrefcnt or kauth_cred_copy) but I don't like it. Ok, I got it now - thanks for baring with me. Martin
Re: Addition to kauth(9) framework
On Mon, Aug 29, 2011 at 07:08:06PM +, David Holland wrote: So hold on. kauth_cred_t (ignoring some silly typedef issues) is the process credentials structure, with the uids and gids in it. Right? In the normal world, each process should have its own, so all sharing is purely an optimization and should be copy-on-write IIRC shared-writable credentials happen for one of the places where processes get used for threads. I also recall something odd in one of the emulations. David -- David Laight: da...@l8s.co.uk
Re: Addition to kauth(9) framework
[good explanation deleted] Yeah, that part I did get. But: The question is *where* new kauth_cred_t instance should be created and assigned to the process: 1) Inside chroot/fchroot(2) (this is in my patch) 2) Modules that adds credential private data. Is the kauth_t passed to the securchroot secmodule (are all other listeners) by value or by reference (at least conceptually). It has to be by reference, isn't it? It is passed by reference. sys/types.h: typedef struct kauth_cred *kauth_cred_t You said choosing (2) over (1) would lead to problems in case we have multiple listeners and I fail to understand how, If all listerners unshare kauth_cred_t *unconditionally*, we lost data set by kauth_cred_setdata. As I said later there is a workaround (kauth_cred_getrefcnt or kauth_cred_copy) but I don't like it. why don't you like it? YAMAMOTO Takashi in that case, choosing (1) over (2) does not lead to (different) problems. I don't see any problem with (1) -- Best regards, Aleksey Cheusov.
Re: Addition to kauth(9) framework
In article 20110829003259.913f014a...@mail.netbsd.org, YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote: hi, I'd like to apply the attached patch. It implements two things: - chroot(2)-ed process is given new kauth_cred_t with reference count equal to 1. can you find a way to avoid this? YAMAMOTO Takashi He tried and I think that this is the minimal hook he needs. christos
re: Addition to kauth(9) framework
In article 20110829003259.913f014a...@mail.netbsd.org, YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote: hi, I'd like to apply the attached patch. It implements two things: - chroot(2)-ed process is given new kauth_cred_t with reference count equal to 1. can you find a way to avoid this? YAMAMOTO Takashi He tried and I think that this is the minimal hook he needs. do you mean that we need to unshare the credential unconditionally, regardless his module is used or not? why? maybe it's just me, but i actually have absolutely no problem with chroot unsharing kauth_cred_t by default. it just seems to have more generic safety aspects. .mrg.
Re: Addition to kauth(9) framework
On Mon 29 Aug 2011 at 13:32:50 +0200, Martin Husemann wrote: On Mon, Aug 29, 2011 at 12:13:38PM +0200, Aleksey Cheusov wrote: we will lost our data. Data set by first listerner will be overriden by the second listerner. This is not just waste of time. Yes, but it is a design bug in the modules or in kauth and unrelated to the (un-)sharing, isn't it? My expectation would be that if the first module unshares, the newly unshared data is passed to the second module, who can unshare it again. (I have not looked at code to check if that is what happens, or even if it is easy to make it so) -Olaf. -- ___ Olaf 'Rhialto' Seibert -- There's no point being grown-up if you \X/ rhialto/at/xs4all.nl-- can't be childish sometimes. -The 4th Doctor
re: Addition to kauth(9) framework
On Aug 29, 7:54pm, m...@eterna.com.au (matthew green) wrote: -- Subject: re: Addition to kauth(9) framework | | In article 20110829003259.913f014a...@mail.netbsd.org, | YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote: | hi, | | I'd like to apply the attached patch. | It implements two things: | | - chroot(2)-ed process is given new kauth_cred_t with reference count | equal to 1. | | can you find a way to avoid this? | | YAMAMOTO Takashi | | He tried and I think that this is the minimal hook he needs. | | do you mean that we need to unshare the credential unconditionally, | regardless his module is used or not? why? | | maybe it's just me, but i actually have absolutely no problem | with chroot unsharing kauth_cred_t by default. it just seems | to have more generic safety aspects. I share the same sentiment; I don't see the change as a big deal. christos
Re: Addition to kauth(9) framework
On Mon, Aug 29, 2011 at 09:19:11AM -0400, Christos Zoulas wrote: On Aug 29, 7:54pm, m...@eterna.com.au (matthew green) wrote: -- Subject: re: Addition to kauth(9) framework | | In article 20110829003259.913f014a...@mail.netbsd.org, | YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote: | hi, | | I'd like to apply the attached patch. | It implements two things: | | - chroot(2)-ed process is given new kauth_cred_t with reference count | equal to 1. | | can you find a way to avoid this? | | YAMAMOTO Takashi | | He tried and I think that this is the minimal hook he needs. | | do you mean that we need to unshare the credential unconditionally, | regardless his module is used or not? why? | | maybe it's just me, but i actually have absolutely no problem | with chroot unsharing kauth_cred_t by default. it just seems | to have more generic safety aspects. I share the same sentiment; I don't see the change as a big deal. Likewise - the whole idea behind chroot is the isolatino of operations, and I can only see the unsharing of kauth_cred_t by default as helping this. Maybe I'm missing something here? Thanks, Alistair
Re: Addition to kauth(9) framework
On Mon, Aug 29, 2011 at 02:36:04PM +0200, Aleksey Cheusov wrote: If sender (chroot(2)) cares about unsharing kauth_cred_t structure, all listeners will set their data without any problem provided that kauth_key_t keys they use are different. Key uniqueness is garanteed by kauth_register_key. I'm sorry, I'm very likely still missing some important detail: this sounds to me as if we have to choose here between the sender distributing individual unshared credentials to every receiver (I thought kauth would handle the messaging?), which means every receiver gets its own copy, but those lack modifications done by previous receivers - or if the receiver does the unsharing, its modifications will get lost if we have multiple receivers. Both options sound wrong to me, what did I misunderstand? Thanks, Martin
Re: Addition to kauth(9) framework
On Mon, Aug 29, 2011 at 07:54:38PM +1000, matthew green wrote: do you mean that we need to unshare the credential unconditionally, regardless his module is used or not? why? maybe it's just me, but i actually have absolutely no problem with chroot unsharing kauth_cred_t by default. it just seems to have more generic safety aspects. So hold on. kauth_cred_t (ignoring some silly typedef issues) is the process credentials structure, with the uids and gids in it. Right? In the normal world, each process should have its own, so all sharing is purely an optimization and should be copy-on-write. Therefore, anything that changes the contents of the structure should first establish a private copy of it. Because the type is private to kern_auth.c, this is fairly easy to establish and audit. So far so good. Now, the question is: in the Elad world, are there cases where this thing is supposed to be shared for modification? If so, what are they, nothing as far as i know. YAMAMOTO Takashi what's the intent and the intended semantics, and is there an implementation that makes it all behave reasonably without being a code nightmare? Like with sharing pages in VM, handling objects that are both shared-update and copy-on-write at once is not entirely trivial. -- David A. Holland dholl...@netbsd.org
Re: Addition to kauth(9) framework
hi, I'd like to apply the attached patch. It implements two things: - chroot(2)-ed process is given new kauth_cred_t with reference count equal to 1. can you find a way to avoid this? YAMAMOTO Takashi - New id KAUTH_CRED_CHROOT is added to kauth(9) credentials scope which is used when chroot(2) or fchroot(2) is called. This two things allows to implement things like securechroot(9) secmodel described here http://mail-index.netbsd.org/tech-kern/2011/07/09/msg010903.html After commiting this patch I'll move the rest of securechroot(9) to pkgsrc until it is ready to be integrated into the kernel. Objections? -- Best regards, Aleksey Cheusov.
Re: Addition to kauth(9) framework
Sorry. Attachment is here.
Index: share/man/man9/kauth.9
===
RCS file: /cvsroot/src/share/man/man9/kauth.9,v
retrieving revision 1.91
diff -u -r1.91 kauth.9
--- share/man/man9/kauth.9 28 Apr 2011 12:22:35 - 1.91
+++ share/man/man9/kauth.9 23 Jul 2011 11:06:41 -
@@ -1087,6 +1087,19 @@
are both
.Ft struct proc *
of the parent and child processes, respectively.
+.It Dv KAUTH_CRED_CHROOT
+The credentials are being initialized during
+.Xr chroot 2
+or
+.Xr fchroot 2
+syscalls.
+.Pp
+.Ar cred
+are the credentials of the proc context doing the chroot, and
+.Ar arg0
+is a
+.Ft struct cwdinfo *
+of the process.
.It Dv KAUTH_CRED_FREE
The credentials in
.Ar cred
Index: sys/kern/kern_auth.c
===
RCS file: /cvsroot/src/sys/kern/kern_auth.c,v
retrieving revision 1.65
diff -u -r1.65 kern_auth.c
--- sys/kern/kern_auth.c 31 Dec 2009 02:20:36 - 1.65
+++ sys/kern/kern_auth.c 23 Jul 2011 11:06:52 -
@@ -286,6 +286,12 @@
child);
}
+void
+kauth_proc_chroot(kauth_cred_t cred, struct cwdinfo *cwdi)
+{
+ kauth_cred_hook(cred, KAUTH_CRED_CHROOT, cwdi, NULL);
+}
+
uid_t
kauth_cred_getuid(kauth_cred_t cred)
{
Index: sys/kern/vfs_syscalls.c
===
RCS file: /cvsroot/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.431
diff -u -r1.431 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 3 Jul 2011 15:25:09 - 1.431
+++ sys/kern/vfs_syscalls.c 23 Jul 2011 11:06:53 -
@@ -1035,6 +1035,10 @@
void
change_root(struct cwdinfo *cwdi, struct vnode *vp, struct lwp *l)
{
+ struct proc *p = l-l_proc;
+ kauth_cred_t ncred;
+
+ ncred = kauth_cred_alloc();
rw_enter(cwdi-cwdi_lock, RW_WRITER);
if (cwdi-cwdi_rdir != NULL)
@@ -1056,6 +1060,15 @@
cwdi-cwdi_cdir = vp;
}
rw_exit(cwdi-cwdi_lock);
+
+ /* Get a write lock on the process credential. */
+ proc_crmod_enter();
+
+ kauth_cred_clone(p-p_cred, ncred);
+ kauth_proc_chroot(ncred, p-p_cwdi);
+
+ /* Broadcast our credentials to the process and other LWPs. */
+ proc_crmod_leave(ncred, p-p_cred, true);
}
/*
Index: sys/sys/kauth.h
===
RCS file: /cvsroot/src/sys/sys/kauth.h,v
retrieving revision 1.64
diff -u -r1.64 kauth.h
--- sys/sys/kauth.h 24 Dec 2009 19:02:07 - 1.64
+++ sys/sys/kauth.h 23 Jul 2011 11:06:54 -
@@ -41,6 +41,7 @@
struct proc;
struct tty;
struct vnode;
+struct cwdinfo;
/* Types. */
typedef struct kauth_scope *kauth_scope_t;
@@ -282,7 +283,8 @@
KAUTH_CRED_INIT=1,
KAUTH_CRED_FORK,
KAUTH_CRED_COPY,
- KAUTH_CRED_FREE
+ KAUTH_CRED_FREE,
+ KAUTH_CRED_CHROOT
};
/*
@@ -418,6 +420,7 @@
kauth_cred_t kauth_cred_get(void);
void kauth_proc_fork(struct proc *, struct proc *);
+void kauth_proc_chroot(kauth_cred_t cred, struct cwdinfo *cwdi);
void secmodel_register(void);
void secmodel_deregister(void);
I'd like to apply the attached patch.
It implements two things:
- chroot(2)-ed process is given new kauth_cred_t with reference count
equal to 1.
- New id KAUTH_CRED_CHROOT is added to kauth(9) credentials scope
which is used when chroot(2) or fchroot(2) is called.
This two things allows to implement things like securechroot(9) secmodel
described here
http://mail-index.netbsd.org/tech-kern/2011/07/09/msg010903.html
After commiting this patch I'll move the rest of securechroot(9)
to pkgsrc until it is ready to be integrated into the kernel.
Objections?
--
Best regards, Aleksey Cheusov.
