From patchwork Wed Mar 29 22:01:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 9652671 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 32533602C8 for ; Wed, 29 Mar 2017 22:05:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22EC4284D6 for ; Wed, 29 Mar 2017 22:05:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 175AD28558; Wed, 29 Mar 2017 22:05:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8C243284D6 for ; Wed, 29 Mar 2017 22:05:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932638AbdC2WFi (ORCPT ); Wed, 29 Mar 2017 18:05:38 -0400 Received: from mail-pg0-f43.google.com ([74.125.83.43]:35763 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932470AbdC2WFg (ORCPT ); Wed, 29 Mar 2017 18:05:36 -0400 Received: by mail-pg0-f43.google.com with SMTP id 81so19590815pgh.2 for ; Wed, 29 Mar 2017 15:05:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=CAcFky5CVD8s533WZZ+RFPPyXPlQrPDKFbHf3K+njXo=; b=CMt0zIZjYXDscndry7XJenEQCYeMCzmYSDd7PN/DaLshsmkxtm61ouDs1f7rCjjgcK qz86mCEKwl7RVEZVNm+NvkaRWGtnUN9AIe81tvYWMm5jhn7kyOcc0y8eTN4PI9neACLh 9vn4fysRCCXh6eyjyF6/FyhY58SmY8EUF6VyoV/w0WbIItMFjzx2B39gIHCmrJqE0iDU j/ZMVeyZ74m/rYnPKEEpKvw12A23OPlhxXLJB7E3KKFzxerO0hNAjrr5kvBjpxeo/y5f 21chgyIs7Cltg0S9H2Mgu7r0r2vWWWWE3iZWx9DVxiX/3hoeQor4wMFYRFq90cke2tPu JUbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=CAcFky5CVD8s533WZZ+RFPPyXPlQrPDKFbHf3K+njXo=; b=jjD0nmNjXlC7W9M+kc8cksm1wO3dvkKPQ/W3TjwirqGXuM4PscB2KIlMH2HV4KTaZx PO4+cU5tmptp9rQZWKfoPiocMuiZJhxIEjGrcAPRo5MsxjEn9QA0BreiTvtRq5R70jXO ON759AwbZY7JQJLlBdFYT7/0d2SitWFn3p0C0CBwBZslN1RfYtjLHPm1QUIuQKEMK4Mp Y08Msfcx9pu0ybT/TfIyfvgc5M9k/HsXHlsxFgv55I04VoHCjeMxatBPSwDpoJDSgRma RWglUmARVq+/aLESesvuWxhegsBM8s47OV8jIwI2EQ87GpHDCe92Ju6UFdDb+H9+rS1z OvQQ== X-Gm-Message-State: AFeK/H3qm/4IwYOputjNwkk/Lha1VvQ3M/5gl41AAZncumaJ9fOtTOzjhJ3cZPnpocTnnXMZ X-Received: by 10.84.194.195 with SMTP id h61mr3231271pld.182.1490825134938; Wed, 29 Mar 2017 15:05:34 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.119.30.131]) by smtp.gmail.com with ESMTPSA id n7sm196838pfn.0.2017.03.29.15.05.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 29 Mar 2017 15:05:33 -0700 (PDT) From: Eric Biggers To: keyrings@vger.kernel.org Cc: David Howells , linux-api@vger.kernel.org, linux-cifs@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-nfs@vger.kernel.org, Gwendal Grignou , Jaegeuk Kim , Richard Weinberger , Theodore Ts'o , stable@vger.kernel.org Subject: [PATCH] KEYS: make keyctl_invalidate() also require Setattr permission Date: Wed, 29 Mar 2017 15:01:01 -0700 Message-Id: <20170329220101.26067-1-ebiggers@google.com> X-Mailer: git-send-email 2.12.2.564.g063fe858b8-goog Sender: linux-fscrypt-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, a process only needs Search permission on a key to invalidate it with keyctl_invalidate(), which has an effect similar to unlinking it from all keyrings. Unfortunately, this significantly compromises the keys permission system because as a result, there is no way to grant read-only access to keys without also granting de-facto "delete" access. Even worse, processes may invalidate entire _keyrings_, given only permission to "search" them. Key invalidation seems to have intended for cases where keyrings are used as caches, and keys can be re-requested at any time by an upcall to /sbin/request-key. However, keyrings are not always used in this way. For example, users of ext4, f2fs, and ubifs encryption may wish to grant many differently-privileged processes access to the same keys, set up in a shared keyring ahead of time. Permitting everyone to invalidate keys in this scenario enables a trivial denial-of-service. And even users of keyrings as "just caches" may wish to restrict invalidation because it may require significant work or user interaction to regenerate keys. This patch fixes the flaw by requiring both Search and Setattr permission to invalidate a key rather than just Search permission. Requiring Setattr permission is appropriate because Setattr permission also allows revoking (via keyctl_revoke()) or expiring (via keyctl_set_timeout()) the key, which also make the key inaccessible regardless of how many keyrings it is in. Continuing to require Search permission ensures we do not decrease the level of permissions required. The problem could also be solved by requiring Write permission, but this would arguably be less relevant because Write is supposed to deal with the key payload, not the key itself. A new "Invalidate" permission would also work but would require keyctl_setperm() users to start including a new permission which never existed before. This is a user-visible API change. But we don't seem to have much choice, and any breakage will be limited to users who override the default key permissions using keyctl_setperm() to remove Setattr permission, then later call keyctl_invalidate(). There are probably not many such users, possibly even none at all. In fact, the only user of keyctl_invalidate() I could find is nfsidmap, which will be unaffected because it relies on the "root is permitted to invalidate certain special keys" exception rather than the actual key permissions. Cc: linux-api@vger.kernel.org Cc: linux-cifs@vger.kernel.org Cc: linux-fscrypt@vger.kernel.org Cc: linux-nfs@vger.kernel.org Cc: Gwendal Grignou Cc: Jaegeuk Kim Cc: Richard Weinberger Cc: Theodore Ts'o Cc: stable@vger.kernel.org # v3.5+ Signed-off-by: Eric Biggers --- Documentation/security/keys.txt | 4 ++-- security/keys/keyctl.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt index 0e03baf271bd..b167f352c043 100644 --- a/Documentation/security/keys.txt +++ b/Documentation/security/keys.txt @@ -820,8 +820,8 @@ The keyctl syscall functions are: immediately, though they are still visible in /proc/keys until deleted (they're marked with an 'i' flag). - A process must have search permission on the key for this function to be - successful. + A process must have Search and Setattr permission on the key for this + function to be successful. (*) Compute a Diffie-Hellman shared secret or public key diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 52c34532c785..c58674710251 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -397,7 +397,7 @@ long keyctl_revoke_key(key_serial_t id) /* * Invalidate a key. * - * The key must be grant the caller Invalidate permission for this to work. + * The key must grant the caller Search and Setattr permission for this to work. * The key and any links to the key will be automatically garbage collected * immediately. * @@ -413,7 +413,7 @@ long keyctl_invalidate_key(key_serial_t id) kenter("%d", id); - key_ref = lookup_user_key(id, 0, KEY_NEED_SEARCH); + key_ref = lookup_user_key(id, 0, KEY_NEED_SEARCH | KEY_NEED_SETATTR); if (IS_ERR(key_ref)) { ret = PTR_ERR(key_ref);