From patchwork Tue Aug 14 02:05:38 2018
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Alison Schofield <alison.schofield@intel.com>
X-Patchwork-Id: 10564987
Return-Path: <linux-security-module-owner@kernel.org>
Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org
 [172.30.200.125])
	by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BB9FC15A6
	for <patchwork-linux-security-module@patchwork.kernel.org>;
 Tue, 14 Aug 2018 02:04:06 +0000 (UTC)
Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1])
	by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A8A19295A1
	for <patchwork-linux-security-module@patchwork.kernel.org>;
 Tue, 14 Aug 2018 02:04:06 +0000 (UTC)
Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486)
	id 9C8A0295A9; Tue, 14 Aug 2018 02:04:06 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI
	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 24436295A1
	for <patchwork-linux-security-module@patchwork.kernel.org>;
 Tue, 14 Aug 2018 02:04:06 +0000 (UTC)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1729459AbeHNEs7 (ORCPT
        <rfc822;patchwork-linux-security-module@patchwork.kernel.org>);
        Tue, 14 Aug 2018 00:48:59 -0400
Received: from mga02.intel.com ([134.134.136.20]:10362 "EHLO mga02.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1725829AbeHNEs7 (ORCPT
        <rfc822;linux-security-module@vger.kernel.org>);
        Tue, 14 Aug 2018 00:48:59 -0400
X-Amp-Result: UNSCANNABLE
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
  by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 13 Aug 2018 19:04:03 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.53,236,1531810800";
   d="scan'208";a="224405328"
Received: from alison-desk.jf.intel.com ([10.54.74.53])
  by orsmga004.jf.intel.com with ESMTP; 13 Aug 2018 19:04:03 -0700
Date: Mon, 13 Aug 2018 19:05:38 -0700
From: Alison Schofield <alison.schofield@intel.com>
To: dhowells@redhat.com
Cc: jmorris@namei.org, serge@hallyn.com, keyrings@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        Dave Hansen <dave.hansen@intel.com>,
        Kirill Shutemov <kirill.shutemov@intel.com>,
        Kai Huang <kai.huang@intel.com>,
        Jun Nakajima <jun.nakajima@intel.com>
Subject: [RFC] KEYS: inject an MKTME specific safety check in the keyctl
 revoke path
Message-ID: <20180814020538.GA18424@alison-desk.jf.intel.com>
MIME-Version: 1.0
Content-Disposition: inline
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: owner-linux-security-module@vger.kernel.org
Precedence: bulk
List-ID: <linux-security-module.vger.kernel.org>
X-Virus-Scanned: ClamAV using ClamSMTP

This RFC is asking for feedback on a problem I'm running into using
the Kernel Key Service for MKTME (MultiKey Total Memory Encryption).

I previously posted an RFC with the proposal to create a new key type
"mktme" to support MKTME (Multi-Key Total Memory Encryption).
https://www.spinics.net/lists/keyrings/msg03702.html

The MKTME key service maps userspace keys to hardware keyids. Those
keys are used in a new system call that encrypts memory. The keys
need to be tightly controlled. One example is that userspace keys
should not be revoked while the hardware keyid slot is still in use.

The KEY_FLAG_KEEP bit offers good control. The mktme service uses that
bit to prevent userspace keys from disappearing without the service
being notified.

Problem is that we need a safe and synchronous way to revoke keys. The
way .revoke methods function now, the key service type is called late
in the revoke process. The mktme key service has no means to reject the
request. So, even if the mktme service sanity checks the request in its
.revoke method, it's too late to do anything about it.

This proposal inserts an MKTME specific check earlier into the existing
keyctl <revoke> path. If it is safe to revoke the key, mktme key service
will turn off KEY_FLAG_KEEP and let the revoke continue (and succeed).
Otherwise, not safe, KEY_FLAG_KEEP stays on, and the revoke continues
(and fails).

I considered proposing a new keyctl <option> just for this mktme 'safe
revoke' request. I wonder if that might be the preferred method for
inserting this type specific behavior?

Hoping that from this description and the diff you can understand the
issue and suggest alternative solutions if needed. Thanks for looking!

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 security/keys/internal.h   |  6 ++++++
 security/keys/keyctl.c     | 14 ++++++++++++++
 security/keys/mktme_keys.c | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

 	ret = mktme_program_key(key->serial, kprog);
+	set_bit(KEY_FLAG_KEEP, &key->flags);
 	mktme_map_unlock();
 out:
 	kzfree(datablob);

diff --git a/security/keys/internal.h b/security/keys/internal.h
index 9f8208dc0e55..1b6425d0d1ab 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -316,4 +316,10 @@ static inline void key_check(const struct key *key)
 
 #endif
 
+#ifdef CONFIG_MKTME_KEYS
+extern int mktme_safe_revoke(struct key *key);
+#else
+static inline int mktme_safe_revoke(struct key *key) { return 0; }
+#endif /* CONFIG_MKTME_KEYS */
+
 #endif /* _INTERNAL_H */
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 1ffe60bb2845..7b5f98af1d54 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -387,6 +387,20 @@ long keyctl_revoke_key(key_serial_t id)
 
 	key = key_ref_to_ptr(key_ref);
 	ret = 0;
+
+	/*
+	 * MKTME (Multi-Key Total Memory Encryption) Keys require a
+	 * sanity check before allowing a revoke. If the sanity check
+	 * passes, the mktme key service will clear KEY_FLAG_KEEP bit
+	 * and the revoke will proceed.
+	 */
+	if (strcmp(key->type->name, "mktme") == 0)  {
+		if (mktme_safe_revoke(key)) {
+			ret = -EINVAL;
+			goto error;
+		}
+	}
+
 	if (test_bit(KEY_FLAG_KEEP, &key->flags))
 		ret = -EPERM;
 	else
diff --git a/security/keys/mktme_keys.c b/security/keys/mktme_keys.c
index b937bbe6bcdb..887b483d7b38 100644
--- a/security/keys/mktme_keys.c
+++ b/security/keys/mktme_keys.c
@@ -67,6 +67,39 @@ static int mktme_clear_programmed_key(int keyid)
 	return ret;
 }
 
+/*
+ * mktme_safe_revoke() is called during the revoke process to
+ * determine if it is safe to revoke a key. If this check passes,
+ * the revoke proceeds, otherwise an error is returned to userspace.
+ * The important error case here is outstanding memory mappings using
+ * the corresponding hardware keyid.
+ */
+int mktme_safe_revoke(struct key *key)
+{
+	int keyid, vma_count;
+	int ret = -1;
+
+	mktme_map_lock();
+	keyid = mktme_map_keyid_from_serial(key->serial);
+	if (keyid <= 0)
+		goto out;
+
+	vma_count = vma_read_encrypt_ref(keyid);
+	if (vma_count > 0) {
+		pr_debug("mktme not freeing keyid[%d] encrypt_count[%d]\n",
+			 keyid, vma_count);
+		goto out;
+	}
+	mktme_clear_programmed_key(keyid);
+	/* Clearing KEY_FLAG_KEEP flag allows the revoke to proceed */
+	clear_bit(KEY_FLAG_KEEP, &key->flags);
+	ret = 0;
+out:
+	mktme_map_unlock();
+	return ret;
+}
+
+
@@ -315,6 +348,7 @@ int mktme_instantiate(struct key *key, struct key_preparsed_payload *prep)
 
 	mktme_map_lock();