On Mon, Aug 17, 2015 at 09:01:24AM +0200, Lukas Slebodnik wrote:
> On (13/08/15 17:46), Jakub Hrozek wrote:
> >Hi,
> >
> >the attached patch hardens the p11_child process.
>
> >From 4023fb832cc5c5122c235b713c0ef401c5d21dd0 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhro...@redhat.com>
> >Date: Wed, 5 Aug 2015 17:25:20 +0200
> >Subject: [PATCH] p11child: set restrictive umask and clear environment
> >
> >https://fedorahosted.org/sssd/ticket/2754
> >
> >Before doing any calls, set a very restrictive umask and clear
> >environment variables to harden p11child execution.
> >---
> > src/p11_child/p11_child_nss.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c
> >index
> >6948c142aa7843cda5ff6d18f5853b10c387c224..44ba6678893408dbfc0c6c7cfd5edcdaa789f518
> > 100644
> >--- a/src/p11_child/p11_child_nss.c
> >+++ b/src/p11_child/p11_child_nss.c
> >@@ -481,6 +481,9 @@ int main(int argc, const char *argv[])
> > /* Set debug level to invalid value so we can decide if -d 0 was used.
> > */
> > debug_level = SSSDBG_INVALID;
> >
> >+ clearenv();
> >+ umask(077);
> >+
> > pc = poptGetContext(argv[0], argc, argv, long_options, 0);
> > while ((opt = poptGetNextOpt(pc)) != -1) {
> > switch(opt) {
> >--
> >2.4.3
> >
>
> LGTM
Thanks, btw this code is unit-tested.
>
> Do you think it would be good idea to the same for proxy_child.
Yes, good idea.
But I would propose to do two patches, because downstream only cares
about p11_child hardening.
> So ticket #2689 would be just partialy fixed but we could
> backport it in a single patch to downstream.
>
> Should we also add 'chdir("/")' to p11child?
We can, it wouldn't hurt, but I'm pretty sure all sssd processes call
chdir to root already (I remember this because it was one of my first
patches to sssd :-))
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
