From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5449C433DB for ; Thu, 14 Jan 2021 22:44:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 99C8223435 for ; Thu, 14 Jan 2021 22:44:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730809AbhANWoM (ORCPT ); Thu, 14 Jan 2021 17:44:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730450AbhANWoL (ORCPT ); Thu, 14 Jan 2021 17:44:11 -0500 Received: from mail-lj1-x234.google.com (mail-lj1-x234.google.com [IPv6:2a00:1450:4864:20::234]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4F77C061757 for ; Thu, 14 Jan 2021 14:43:30 -0800 (PST) Received: by mail-lj1-x234.google.com with SMTP id f17so8280629ljg.12 for ; Thu, 14 Jan 2021 14:43:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=l0aLIrK9Ls3yChluxnDYpvoAFnbmuAeFSQRaGDSDK6s=; b=Xp2UG1bDqZV6MWUIg3PXRWlOxm3AESTgyUaOByzy05KbBMIlWQTWhAjiAXqIJLco8n CKATzDajP+/i7+U9T8QSEVzVCGM9KCooEF2Qog6oGaP7m4UbxqlPABfgxbuvEWJTi9CN eqzQu31AW2vsl1w6jVkXxJ/M5h/fqc7PNe9pqO61lRW1/nWovu8unaTR5XmmIZkfZgwP tr+Axa7VGzhhKFDPvqTzv2L+EqZ4C5BEDY31jsINoIDSNsVV2C0gKRsxVsVvX38Zs0wH p15PatmNXOKYvYVm9VjZ1UxZrw3YprkBpqifeMBtnXs/ATFse8XhCqZ+GPH/cm/nlJcE s4sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=l0aLIrK9Ls3yChluxnDYpvoAFnbmuAeFSQRaGDSDK6s=; b=IFQco6f8ZgxYxNtAA1B2z/07VnCPIOUuQqnIbZsyaTHuBBhOfSx2xqUHXwHQoWH0HM TsNSSLWXwhERzBPs8iqVV8MvTY6ahjc/fIkXMIFmWU0dVDYG3rflo0lKQjKtgdv7s4wf PLNx+kbFkmEFLEu4sv+J10HgabZ/8nI6/2rnWc2/OrtI9TXMS9m/UD0dD+CDwdkhE7Qe oE2BeFgG/uZ6/JnVgD2suskIJUJUrec3xVTTxxntHyjIRZIFWX/6vCAxVvnkwWLOEYWu 11iTFWXrrdgCmRap3VCOeqj9Ko/H1q5lqNcAZQjQVL1kErjynfy2/lVPCw0J2J83LND0 eWFQ== X-Gm-Message-State: AOAM530XM+CswCkhqWKv36+yQ+n5pn0cDlPYOBIOEgb+nems5B2jVU41 u2kFjgysCl+k7zILNg6Yy5Am267moLegRxM0f9e0Ew== X-Google-Smtp-Source: ABdhPJwmu2cBfmtpdruzglhRIadtD3T+wRN+tSM4C2y7Zg3rHsx1RPPVdIX6GIoHvdQgt3ctrnlKrS5mVOsEqS6Krko= X-Received: by 2002:a2e:50c:: with SMTP id 12mr4154104ljf.226.1610664208863; Thu, 14 Jan 2021 14:43:28 -0800 (PST) MIME-Version: 1.0 References: <20201209192839.1396820-1-mic@digikod.net> <20201209192839.1396820-8-mic@digikod.net> In-Reply-To: From: Jann Horn Date: Thu, 14 Jan 2021 23:43:02 +0100 Message-ID: Subject: Re: [PATCH v26 07/12] landlock: Support filesystem access-control To: =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= Cc: James Morris , "Serge E . Hallyn" , Al Viro , Andy Lutomirski , Anton Ivanov , Arnd Bergmann , Casey Schaufler , Jeff Dike , Jonathan Corbet , Kees Cook , Michael Kerrisk , Richard Weinberger , Shuah Khan , Vincent Dagonneau , Kernel Hardening , Linux API , linux-arch , "open list:DOCUMENTATION" , linux-fsdevel , kernel list , "open list:KERNEL SELFTEST FRAMEWORK" , linux-security-module , "the arch/x86 maintainers" , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 14, 2021 at 7:54 PM Micka=C3=ABl Sala=C3=BCn = wrote: > On 14/01/2021 04:22, Jann Horn wrote: > > On Wed, Dec 9, 2020 at 8:28 PM Micka=C3=ABl Sala=C3=BCn wrote: > >> Thanks to the Landlock objects and ruleset, it is possible to identify > >> inodes according to a process's domain. To enable an unprivileged > >> process to express a file hierarchy, it first needs to open a director= y > >> (or a file) and pass this file descriptor to the kernel through > >> landlock_add_rule(2). When checking if a file access request is > >> allowed, we walk from the requested dentry to the real root, following > >> the different mount layers. The access to each "tagged" inodes are > >> collected according to their rule layer level, and ANDed to create > >> access to the requested file hierarchy. This makes possible to identi= fy > >> a lot of files without tagging every inodes nor modifying the > >> filesystem, while still following the view and understanding the user > >> has from the filesystem. > >> > >> Add a new ARCH_EPHEMERAL_INODES for UML because it currently does not > >> keep the same struct inodes for the same inodes whereas these inodes a= re > >> in use. > >> > >> This commit adds a minimal set of supported filesystem access-control > >> which doesn't enable to restrict all file-related actions. This is th= e > >> result of multiple discussions to minimize the code of Landlock to eas= e > >> review. Thanks to the Landlock design, extending this access-control > >> without breaking user space will not be a problem. Moreover, seccomp > >> filters can be used to restrict the use of syscall families which may > >> not be currently handled by Landlock. > > [...] > >> +static bool check_access_path_continue( > >> + const struct landlock_ruleset *const domain, > >> + const struct path *const path, const u32 access_reques= t, > >> + u64 *const layer_mask) > >> +{ > > [...] > >> + /* > >> + * An access is granted if, for each policy layer, at least on= e rule > >> + * encountered on the pathwalk grants the access, regardless o= f their > >> + * position in the layer stack. We must then check not-yet-se= en layers > >> + * for each inode, from the last one added to the first one. > >> + */ > >> + for (i =3D 0; i < rule->num_layers; i++) { > >> + const struct landlock_layer *const layer =3D &rule->la= yers[i]; > >> + const u64 layer_level =3D BIT_ULL(layer->level - 1); > >> + > >> + if (!(layer_level & *layer_mask)) > >> + continue; > >> + if ((layer->access & access_request) !=3D access_reque= st) > >> + return false; > >> + *layer_mask &=3D ~layer_level; > > > > Hmm... shouldn't the last 5 lines be replaced by the following? > > > > if ((layer->access & access_request) =3D=3D access_request) > > *layer_mask &=3D ~layer_level; > > > > And then, since this function would always return true, you could > > change its return type to "void". > > > > > > As far as I can tell, the current version will still, if a ruleset > > looks like this: > > > > /usr read+write > > /usr/lib/ read > > > > reject write access to /usr/lib, right? > > If these two rules are from different layers, then yes it would work as > intended. However, if these rules are from the same layer the path walk > will not stop at /usr/lib but go down to /usr, which grants write > access. I don't see why the code would do what you're saying it does. And an experiment seems to confirm what I said; I checked out landlock-v26, and the behavior I get is: user@vm:~/landlock$ dd if=3D/dev/null of=3D/tmp/aaa 0+0 records in 0+0 records out 0 bytes copied, 0.00106365 s, 0.0 kB/s user@vm:~/landlock$ LL_FS_RO=3D'/lib' LL_FS_RW=3D'/' ./sandboxer dd if=3D/dev/null of=3D/tmp/aaa 0+0 records in 0+0 records out 0 bytes copied, 0.000491814 s, 0.0 kB/s user@vm:~/landlock$ LL_FS_RO=3D'/tmp' LL_FS_RW=3D'/' ./sandboxer dd if=3D/dev/null of=3D/tmp/aaa dd: failed to open '/tmp/aaa': Permission denied user@vm:~/landlock$ Granting read access to /tmp prevents writing to it, even though write access was granted to /.