From patchwork Wed Jul 12 21:00:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 787426 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3x7BGf68LPz9s0g for ; Thu, 13 Jul 2017 07:02:18 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="MGaepYXL"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753511AbdGLVCR (ORCPT ); Wed, 12 Jul 2017 17:02:17 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:36315 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751862AbdGLVCL (ORCPT ); Wed, 12 Jul 2017 17:02:11 -0400 Received: by mail-pf0-f196.google.com with SMTP id z6so4556647pfk.3; Wed, 12 Jul 2017 14:02:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=YbabpfRdAgWpIeouC0E2GKDhpj0FvuTDFdC9XrM0cCg=; b=MGaepYXLflXsA2teEG1zaeelBG1tssN5tyERNUfWQh0U7vXyeq7DGSi+GWzz0hRIRm +2szGGnFNWDh/VEg/GHxjzpRtt7+Jh2A8VYW8IouORKlp4tZjBSNku+kQ6ZJvbs3zBqG Gi+0aoiMEggUHRS9/J7KWFEJIUoT4720S2cSHT3D8HI2qfZN41l1b/JLjIN3JoX4vvz1 c27LLYKeCYmESfSSRjSVT1FLDKQ9Je+oicYGyu0+7Wc8Bg+aF5M6RXstYJiCGUFsMpnT 1LqPF/SuxSXiM9dmVcqId0e7qjvgQG2Z3UAwZ1W5EcuPh/YlYLAqdStro+Ih54YfaVfA Bdiw== 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:in-reply-to :references; bh=YbabpfRdAgWpIeouC0E2GKDhpj0FvuTDFdC9XrM0cCg=; b=PpRGE2cMlI79Jxm+vSXsGJgrReLudRF008MVKXV5AiEGn0D9gr2VjNq5CiTIRNSS4o M+bfTX8gqFMIgp+/troJv/BNpDM+GfcW6trhy5ymsc7wWCjoc2BO08Tj2x/3m6udqVsp O7P0c/xX+k7C8GuvKVHT37N/K1sw1BCphcRuR8E9sdcEtQRKJ5gc14gioAn+QLPvU8Sx aqgnjiSMLdQ9Gr1H+yfPWkx5gTPleP9n/phh+fOwBz1JT1FFe8ONFpgEzj8pDUH99NTD UE041SQS/A25uHZ63OkoLXcaE+PBoxsMa+hMwfaLjmteSlW9WroUmWazO+r9cWWawrEc UVpg== X-Gm-Message-State: AIVw111d3xFJUAW1//4HMknCAB8T4xcNWLF6G0+oLq6xWHujGpK1ClO0 cWE/9Uo4xZ0lgHe4zs6rSw== X-Received: by 10.98.156.210 with SMTP id u79mr57242676pfk.71.1499893329737; Wed, 12 Jul 2017 14:02:09 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.66.174.81]) by smtp.gmail.com with ESMTPSA id m16sm8002488pfj.130.2017.07.12.14.02.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 12 Jul 2017 14:02:08 -0700 (PDT) From: Eric Biggers To: linux-fscrypt@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org, "Theodore Y . Ts'o" , Jaegeuk Kim , Alex Cope , Eric Biggers Subject: [PATCH 4/6] fscrypt: verify that the correct master key was supplied Date: Wed, 12 Jul 2017 14:00:33 -0700 Message-Id: <20170712210035.51534-5-ebiggers3@gmail.com> X-Mailer: git-send-email 2.13.2.932.g7449e964c-goog In-Reply-To: <20170712210035.51534-1-ebiggers3@gmail.com> References: <20170712210035.51534-1-ebiggers3@gmail.com> Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org From: Eric Biggers Currently, while a fscrypt master key is required to have a certain description in the keyring, its payload is never verified to be correct. While sufficient for well-behaved userspace, this is insecure in a multi-user system where a user has been given only read-only access to an encrypted file or directory. Specifically, if an encrypted file or directory does not yet have its key cached by the kernel, the first user who accesses it can provide an arbitrary key in their own keyring, which the kernel will then associate with the inode and use for read(), write(), readdir(), etc. by other users as well. Consequently, it's trivial for a user with *read-only* access to an encrypted file or directory to make it appear as garbage to everyone. Creative users might be able to accomplish more sophisticated attacks by careful choice of the key, e.g. choosing a key causes certain bytes of file contents to have certain values or that causes filenames containing the '/' character to appear. Solve the problem for v2 encryption policies by storing a "hash" of the master encryption key in the encryption xattr and verifying it before accepting the user-provided key. We generate the "hash" using HKDF-SHA512 by passing a distinct application-specific info string. This produces a value which is cryptographically isolated and can be stored in the clear without leaking any information about the master key or any other derived keys (in a computational sense). Reusing HKDF is better than doing e.g. SHA-512(master_key) because it avoids passing the same key into different cryptographic primitives. We make the hash field 16 bytes long, as this should provide sufficient collision and preimage resistance while not wasting too much space for the encryption xattr. Signed-off-by: Eric Biggers Acked-by: Michael Halcrow --- fs/crypto/fscrypt_private.h | 4 ++++ fs/crypto/keyinfo.c | 46 +++++++++++++++++++++++++++++++++++++ fs/crypto/policy.c | 55 ++++++++++++++++++++++++++++++++++++--------- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h index 095e7c16483a..a7baeac92575 100644 --- a/fs/crypto/fscrypt_private.h +++ b/fs/crypto/fscrypt_private.h @@ -92,6 +92,7 @@ fscrypt_valid_context_format(const struct fscrypt_context *ctx, int size) struct fscrypt_master_key { struct crypto_shash *mk_hmac; unsigned int mk_size; + u8 mk_hash[FSCRYPT_KEY_HASH_SIZE]; }; /* @@ -155,6 +156,9 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx, gfp_t gfp_flags); /* keyinfo.c */ +extern int fscrypt_compute_key_hash(const struct inode *inode, + const struct fscrypt_policy *policy, + u8 hash[FSCRYPT_KEY_HASH_SIZE]); extern void __exit fscrypt_essiv_cleanup(void); #endif /* _FSCRYPT_PRIVATE_H */ diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c index 7ed1a7fb1308..12a60eacf819 100644 --- a/fs/crypto/keyinfo.c +++ b/fs/crypto/keyinfo.c @@ -39,8 +39,11 @@ static struct crypto_shash *essiv_hash_tfm; * * Keys derived with different info strings are cryptographically isolated from * each other --- knowledge of one derived key doesn't reveal any others. + * (This property is particularly important for the derived key used as the + * "key hash", as that is stored in the clear.) */ #define HKDF_CONTEXT_PER_FILE_KEY 1 +#define HKDF_CONTEXT_KEY_HASH 2 /* * HKDF consists of two steps: @@ -212,6 +215,12 @@ alloc_master_key(const struct fscrypt_key *payload) err = crypto_shash_setkey(k->mk_hmac, prk, sizeof(prk)); if (err) goto fail; + + /* Calculate the "key hash" */ + err = hkdf_expand(k->mk_hmac, HKDF_CONTEXT_KEY_HASH, NULL, 0, + k->mk_hash, FSCRYPT_KEY_HASH_SIZE); + if (err) + goto fail; out: memzero_explicit(prk, sizeof(prk)); return k; @@ -537,6 +546,31 @@ void __exit fscrypt_essiv_cleanup(void) crypto_free_shash(essiv_hash_tfm); } +int fscrypt_compute_key_hash(const struct inode *inode, + const struct fscrypt_policy *policy, + u8 hash[FSCRYPT_KEY_HASH_SIZE]) +{ + struct fscrypt_master_key *k; + unsigned int min_keysize; + + /* + * Require that the master key be long enough for both the + * contents and filenames encryption modes. + */ + min_keysize = + max(available_modes[policy->contents_encryption_mode].keysize, + available_modes[policy->filenames_encryption_mode].keysize); + + k = load_master_key_from_keyring(inode, policy->master_key_descriptor, + min_keysize); + if (IS_ERR(k)) + return PTR_ERR(k); + + memcpy(hash, k->mk_hash, FSCRYPT_KEY_HASH_SIZE); + put_master_key(k); + return 0; +} + int fscrypt_get_encryption_info(struct inode *inode) { struct fscrypt_info *crypt_info; @@ -613,6 +647,18 @@ int fscrypt_get_encryption_info(struct inode *inode) goto out; } + /* + * Make sure the master key we found has the correct hash. + * Buggy or malicious userspace may provide the wrong key. + */ + if (memcmp(crypt_info->ci_master_key->mk_hash, ctx.key_hash, + FSCRYPT_KEY_HASH_SIZE)) { + pr_warn_ratelimited("fscrypt: wrong encryption key supplied for inode %lu\n", + inode->i_ino); + res = -ENOKEY; + goto out; + } + res = derive_key_hkdf(crypt_info->ci_master_key, &ctx, derived_key, derived_keysize); } diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index 81c59f8e45c0..2934bc2bff4b 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -40,7 +40,8 @@ static u8 context_version_for_policy(const struct fscrypt_policy *policy) */ static bool is_encryption_context_consistent_with_policy( const struct fscrypt_context *ctx, - const struct fscrypt_policy *policy) + const struct fscrypt_policy *policy, + const u8 key_hash[FSCRYPT_KEY_HASH_SIZE]) { return (ctx->version == context_version_for_policy(policy)) && (memcmp(ctx->master_key_descriptor, @@ -50,11 +51,14 @@ static bool is_encryption_context_consistent_with_policy( (ctx->contents_encryption_mode == policy->contents_encryption_mode) && (ctx->filenames_encryption_mode == - policy->filenames_encryption_mode); + policy->filenames_encryption_mode) && + (ctx->version == FSCRYPT_CONTEXT_V1 || + (memcmp(ctx->key_hash, key_hash, FSCRYPT_KEY_HASH_SIZE) == 0)); } static int create_encryption_context_from_policy(struct inode *inode, - const struct fscrypt_policy *policy) + const struct fscrypt_policy *policy, + const u8 key_hash[FSCRYPT_KEY_HASH_SIZE]) { struct fscrypt_context ctx; @@ -74,7 +78,7 @@ static int create_encryption_context_from_policy(struct inode *inode, BUILD_BUG_ON(sizeof(ctx.nonce) != FS_KEY_DERIVATION_NONCE_SIZE); get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE); if (ctx.version != FSCRYPT_CONTEXT_V1) - memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE); + memcpy(ctx.key_hash, key_hash, FSCRYPT_KEY_HASH_SIZE); return inode->i_sb->s_cop->set_context(inode, &ctx, fscrypt_context_size(&ctx), @@ -87,6 +91,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg) struct inode *inode = file_inode(filp); int ret; struct fscrypt_context ctx; + u8 key_hash[FSCRYPT_KEY_HASH_SIZE]; if (copy_from_user(&policy, arg, sizeof(policy))) return -EFAULT; @@ -98,6 +103,25 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg) policy.version != FS_POLICY_VERSION_HKDF) return -EINVAL; + if (policy.version == FS_POLICY_VERSION_ORIGINAL) { + /* + * Originally no key verification was implemented, which was + * insufficient for scenarios where multiple users share + * encrypted files. The new encryption policy version fixes + * this and also implements an improved key derivation function. + * So as long as the key can be in the keyring at the time the + * policy is set and compatibility with old kernels isn't + * required, it's recommended to use the new policy version + * (fscrypt_policy.version = 2). + */ + pr_warn_once("%s (pid %d) is setting less secure v0 encryption policy; recommend upgrading to v2.\n", + current->comm, current->pid); + } else { + ret = fscrypt_compute_key_hash(inode, &policy, key_hash); + if (ret) + return ret; + } + ret = mnt_want_write_file(filp); if (ret) return ret; @@ -112,10 +136,12 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg) ret = -ENOTEMPTY; else ret = create_encryption_context_from_policy(inode, - &policy); + &policy, + key_hash); } else if (ret >= 0 && fscrypt_valid_context_format(&ctx, ret) && is_encryption_context_consistent_with_policy(&ctx, - &policy)) { + &policy, + key_hash)) { /* The file already uses the same encryption policy. */ ret = 0; } else if (ret >= 0 || ret == -ERANGE) { @@ -232,7 +258,11 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child) (parent_ci->ci_data_mode == child_ci->ci_data_mode) && (parent_ci->ci_filename_mode == child_ci->ci_filename_mode) && - (parent_ci->ci_flags == child_ci->ci_flags); + (parent_ci->ci_flags == child_ci->ci_flags) && + (parent_ci->ci_context_version == FSCRYPT_CONTEXT_V1 || + (memcmp(parent_ci->ci_master_key->mk_hash, + child_ci->ci_master_key->mk_hash, + FSCRYPT_KEY_HASH_SIZE) == 0)); } res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx)); @@ -251,7 +281,10 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child) child_ctx.contents_encryption_mode) && (parent_ctx.filenames_encryption_mode == child_ctx.filenames_encryption_mode) && - (parent_ctx.flags == child_ctx.flags); + (parent_ctx.flags == child_ctx.flags) && + (parent_ctx.version == FSCRYPT_CONTEXT_V1 || + (memcmp(parent_ctx.key_hash, child_ctx.key_hash, + FSCRYPT_KEY_HASH_SIZE) == 0)); } EXPORT_SYMBOL(fscrypt_has_permitted_context); @@ -286,8 +319,10 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child, memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor, FS_KEY_DESCRIPTOR_SIZE); get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE); - if (ctx.version != FSCRYPT_CONTEXT_V1) - memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE); + if (ctx.version != FSCRYPT_CONTEXT_V1) { + memcpy(ctx.key_hash, ci->ci_master_key->mk_hash, + FSCRYPT_KEY_HASH_SIZE); + } BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE); res = parent->i_sb->s_cop->set_context(child, &ctx,