From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51802) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zf3Jo-0003RV-1Z for qemu-devel@nongnu.org; Thu, 24 Sep 2015 06:00:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zf3Jk-0001oD-OU for qemu-devel@nongnu.org; Thu, 24 Sep 2015 05:59:59 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:33006) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zf3Jk-0001o6-Gy for qemu-devel@nongnu.org; Thu, 24 Sep 2015 05:59:56 -0400 Received: by wiclk2 with SMTP id lk2so20622108wic.0 for ; Thu, 24 Sep 2015 02:59:56 -0700 (PDT) Date: Thu, 24 Sep 2015 11:59:52 +0200 From: Eduardo Otubo Message-ID: <20150924095952.GB11859@vader> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="UHN/qo2QbUvPLonB" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2] Add argument filters to the seccomp sandbox List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: namnamc@Safe-mail.net Cc: pmoore@redhat.com, qemu-devel@nongnu.org --UHN/qo2QbUvPLonB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 10, 2015 at 08=3D54=3D28PM -0400, namnamc@Safe-mail.net wrote: > > The current intention of the seccomp filter in QEMU, is that /all/ exis= ting > > QEMU features continue to work unchanged. So even if a flag is used in a > > seemingly uncommon code path, we still need to allow that in a seccomp > > filter. > It already doesn't work very well, e.g. with -chroot, it fails because ch= root() > is not whitelisted, same with -runas because setgid() etc isn't whitelist= ed. > Maybe there should be an extra option for -sandbox, like "-sandbox experi= mental" > which does argument filtering, which of course may break something, and t= he old > behavior would do plain syscall filtering without caring about arguments,= because > that's so much easier to guarantee to work, even if it provides little se= curity. Can you point out which exact use case breaks if you don't whitelist the below mentioned system calls' flags? >=20 > We could also change the default behavior from SCMP_ACT_KILL (which kills= the > entire thing as soon as a single violation occurs) to SCMP_ACT_ERRNO(EPER= M), which > will just return EPERM for a syscall with a violation. The software will = be much > more capable of handling a permission denied error without crashing. Alth= ough of > course that violates the principle of fast-fail. We thought about this in beggining of the development of seccomp on qemu. Some feature like allow all, which would print to stderr all illegal hits and a another argument like -sandbox_add=3D"syscall1,syscall2", but this would be against the concept of the whole security schema. We don't want the user to take full control of it, and if you're a developer, you know what to do. >=20 > > So we need to add DODUMP, DONTDUMP, UNMERGABLE and WILLNEED here. That > > is still stricter than the previous allow-everything rule, so a net > > win. > And MADV_INVALID too I assume? That was one of the others I got with grep. >=20 > > > + > > > + /* shmget */ > > > + rc =3D seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2, > > > + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE), > > > + SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0777)); > >=20 > > I'm not familiar with semantics of these seccomp rules, but is this > > saying that the second arg must be exactly equal to IP_CREAT|0777 ? > > If the app passes IP_CREAT|0600, would that be permitted instead ? > > The latter is what I see gtk2 source code passing for mode. > Argument 2 must be exactly equal to IP_CREAT|0777, yes, otherwise Qemu di= es with > SIGSYS. I did check the Qemu source and saw 0777 was harcoded. Does the 0= 600 mask > in GTK2 ever get called in Qemu? Anyway I added the MADV flags and the 06= 00 mask > in the v2 patch. >=20 > Signed-off-by: Namsun Ch'o > --- > diff --git a/qemu-seccomp.c b/qemu-seccomp.c > index f9de0d3..a353ef9 100644 > --- a/qemu-seccomp.c > +++ b/qemu-seccomp.c > @@ -14,6 +14,8 @@ > */ > #include > #include > +#include > +#include > #include "sysemu/seccomp.h" > =20 > struct QemuSeccompSyscall { > @@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitel= ist[] =3D { > { SCMP_SYS(rt_sigreturn), 245 }, > { SCMP_SYS(sync), 245 }, > { SCMP_SYS(pread64), 245 }, > - { SCMP_SYS(madvise), 245 }, > { SCMP_SYS(set_robust_list), 245 }, > { SCMP_SYS(lseek), 245 }, > { SCMP_SYS(pselect6), 245 }, > @@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_white= list[] =3D { > { SCMP_SYS(arch_prctl), 240 }, > { SCMP_SYS(mkdir), 240 }, > { SCMP_SYS(fchmod), 240 }, > - { SCMP_SYS(shmget), 240 }, > { SCMP_SYS(shmat), 240 }, > { SCMP_SYS(shmdt), 240 }, > { SCMP_SYS(timerfd_create), 240 }, > - { SCMP_SYS(shmctl), 240 }, > { SCMP_SYS(mlockall), 240 }, > { SCMP_SYS(mlock), 240 }, > { SCMP_SYS(munlock), 240 }, > @@ -264,6 +263,60 @@ int seccomp_start(void) > } > } > =20 > + /* madvise */ > + static const int madvise_flags[] =3D { > + MADV_DODUMP, > + MADV_DONTDUMP, > + MADV_INVALID, > + MADV_UNMERGEABLE, > + MADV_WILLNEED, > + MADV_DONTFORK, > + MADV_DONTNEED, > + MADV_HUGEPAGE, > + MADV_MERGEABLE, > + }; > + for (i =3D 0; i < ARRAY_SIZE(madvise_flags); i++) { > + rc =3D seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), = 1, > + SCMP_A2(SCMP_CMP_EQ, madvise_flags[i])); > + if (rc < 0) { > + goto seccomp_return; > + } > + } > + rc =3D seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245); > + if (rc < 0) { > + goto seccomp_return; > + } > + > + /* shmget */ > + rc =3D seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2, > + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE), > + SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0777)); > + if (rc < 0) { > + goto seccomp_return; > + } > + rc =3D seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2, > + SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE), > + SCMP_A2(SCMP_CMP_EQ, IP_CREAT|0600)); Isn't it IPC_CREAT? Or am I missing something? Can you resend a v3 describing the changes you did from v1 to v2 and v3? This helps keep tracking of ideas and discussions. Thanks a lot for the contribution! > + if (rc < 0) { > + goto seccomp_return; > + } > + rc =3D seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240); > + if (rc < 0) { > + goto seccomp_return; > + } > + > + /* shmctl */ > + rc =3D seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2, > + SCMP_A1(SCMP_CMP_EQ, IPC_RMID), > + SCMP_A2(SCMP_CMP_EQ, 0)); > + if (rc < 0) { > + goto seccomp_return; > + } > + rc =3D seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240); > + if (rc < 0) { > + goto seccomp_return; > + } > + > rc =3D seccomp_load(ctx); > =20 > seccomp_return: --=20 Eduardo Otubo ProfitBricks GmbH --UHN/qo2QbUvPLonB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWA8mYAAoJEP0M/1sS+L0veQEH/Ar9y+wgkQKCBR0/AAziREOn iQaR/1O6oH4ugcrk3YIncCnYVLHVS+gOVa1UPhpuH89r9NpOzR2+t9MatUIE0+4a EzueUgJ8nHwslrY8X7Nw3GLVYIBxYselymcDRm8cNyzQUqKp1Q9I2kUI2L/6E5DP WYHvoYRqfzb/U5KQfmc9vLexs3AvUvG+RoDjaZrA8B9tqCMUpcrp3AuDViiUicmH QjIn5pbhml1X+ufF7fDwMkEwJ5FhRYCcmEttiuQshh8kHfWyeOk3stRuQ4ZrgvwC wFazDrxN4iw0RSbvyy8C+RhKJYd1JFs8cR+bGdrOnnmSgzJtGt3+MB4zMyRtI5k= =UAJW -----END PGP SIGNATURE----- --UHN/qo2QbUvPLonB--