| Message ID | 1464565158-18043-7-git-send-email-champetier.etienne@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Etienne, why dont we want to do that ? John On 30/05/2016 01:39, Etienne CHAMPETIER wrote: > Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> > --- > jail/jail.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/jail/jail.c b/jail/jail.c > index e425254..926e42c 100644 > --- a/jail/jail.c > +++ b/jail/jail.c > @@ -386,9 +386,10 @@ int main(int argc, char **argv) > > uloop_init(); > if (opts.namespace) { > - jail_process.pid = clone(exec_jail, > - child_stack + STACK_SIZE, > - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); > + int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD; > + if (opts.hostname) > + flags |= CLONE_NEWUTS; > + jail_process.pid = clone(exec_jail, child_stack + STACK_SIZE, flags, NULL); > } else { > jail_process.pid = fork(); > } >
Hi John, 2016-05-30 9:33 GMT+02:00 John Crispin <john@phrozen.org>: > > > Hi Etienne, > > why dont we want to do that ? If you modify the hostname of the router you might want to propagate it into the jail, it depends Please don't merge this patch, i will improve it a bit: no -h => no CLONE_NEWUTS -h => CLONE_NEWUTS -h <newhostname> => CLONE_NEWUTS + sethostname() CLONE_NEWUTS is not a security feature, sethostname() require CAP_SYS_ADMIN which allow you to escape jail (mknod + mount for exemple) Etienne > > John > > > On 30/05/2016 01:39, Etienne CHAMPETIER wrote: >> Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> >> --- >> jail/jail.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/jail/jail.c b/jail/jail.c >> index e425254..926e42c 100644 >> --- a/jail/jail.c >> +++ b/jail/jail.c >> @@ -386,9 +386,10 @@ int main(int argc, char **argv) >> >> uloop_init(); >> if (opts.namespace) { >> - jail_process.pid = clone(exec_jail, >> - child_stack + STACK_SIZE, >> - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); >> + int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD; >> + if (opts.hostname) >> + flags |= CLONE_NEWUTS; >> + jail_process.pid = clone(exec_jail, child_stack + STACK_SIZE, flags, NULL); >> } else { >> jail_process.pid = fork(); >> } >>
On 30/05/2016 11:59, Etienne Champetier wrote: > Hi John, > > 2016-05-30 9:33 GMT+02:00 John Crispin <john@phrozen.org>: >> >> >> Hi Etienne, >> >> why dont we want to do that ? > > If you modify the hostname of the router you might want to propagate > it into the jail, it depends > > Please don't merge this patch, i will improve it a bit: > no -h => no CLONE_NEWUTS > -h => CLONE_NEWUTS > -h <newhostname> => CLONE_NEWUTS + sethostname() > > CLONE_NEWUTS is not a security feature, > sethostname() require CAP_SYS_ADMIN which allow you to escape jail > (mknod + mount for exemple) ok, i'll merge 1-6 and leave 7/7 out. i wondered abotu this because there are 3 states (the ones you listed) and the code only handles 2. John
diff --git a/jail/jail.c b/jail/jail.c index e425254..926e42c 100644 --- a/jail/jail.c +++ b/jail/jail.c @@ -386,9 +386,10 @@ int main(int argc, char **argv) uloop_init(); if (opts.namespace) { - jail_process.pid = clone(exec_jail, - child_stack + STACK_SIZE, - CLONE_NEWUTS | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD, NULL); + int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | SIGCHLD; + if (opts.hostname) + flags |= CLONE_NEWUTS; + jail_process.pid = clone(exec_jail, child_stack + STACK_SIZE, flags, NULL); } else { jail_process.pid = fork(); }
Signed-off-by: Etienne CHAMPETIER <champetier.etienne@gmail.com> --- jail/jail.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)