Received: by 10.223.148.5 with SMTP id 5csp6632689wrq; Wed, 17 Jan 2018 16:40:32 -0800 (PST) X-Google-Smtp-Source: ACJfBovtte7cOOaE5nL38/JY8wLLmCmTLtjLfpydegoKgQVRmZR0y/td7VTz3m2caYFHJjC5QOYW X-Received: by 10.101.86.4 with SMTP id l4mr23391022pgs.276.1516236032411; Wed, 17 Jan 2018 16:40:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1516236032; cv=none; d=google.com; s=arc-20160816; b=D2HmN9wacAxh4sVTqSdZJpJGC9xygy2IQIAm1kDLLZoo9p4iASmCQee9u2sxUmnEAh MuJYEsyK/EU9oPoMc/r/KXhitoJU5D424LdyoGPAGI0nNCK3q3uzpC7LtVKKlKCrqSaC s6VDcfGyfvHZ58G1YJRV73Dnf+xr9RDOdsFp67Y8kpuaAjPCS2L0ah1q1QZeBzb8C3og CewPS5HTRfQTTBgxbKDeMULRKV2bwJGR/5a3VlsN6Ve+GlMmsVdo3JYt1bV+yDfut0xP Y6uftk9dkXXVoWMaRvLd2qu31cvjsgDAEVmlXPtPTQuvdbMRNWbzt0tV6Zf+oEJKIqRC MtHg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=hlDhubmeqhH1m/+by6UfLUI03AMXwP/SiTOlCgaK9Qs=; b=BKseus4MYekdfulk1vx9YhnJWV4hoNc9zv12ujABXJJCxOJHZWYWoqPkpHh6bHSP5+ aO0IDrRMDG7X5m+pJkkTk4N/4B/fgViwOWZOD1X/8EjQuC3JMgwZtsPB6GsgUYLqJ7jc TOCAAjAKhrf3Y00m96Iui3ntAXFnA6RvjR9+5dVOcehRRbKm9Ompt5XXoFeoljsju8GW pTujuysy6ujQke3ZCLAHeiN+zJ0IIbAu9V/LEu1wHVU19inuB8bm0LAlKzu062C96/dR ODKkoGQBclMKlmjpbis9AH/uDajRDep0LnWGa+QbnsGJFjbp9ydrqRAJeqmyevIyA19K aNUA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=TlOtR88R; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t23si5277295ply.251.2018.01.17.16.40.17; Wed, 17 Jan 2018 16:40:32 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=TlOtR88R; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754265AbeARAjy (ORCPT + 99 others); Wed, 17 Jan 2018 19:39:54 -0500 Received: from mail-io0-f193.google.com ([209.85.223.193]:37708 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752368AbeARAjw (ORCPT ); Wed, 17 Jan 2018 19:39:52 -0500 Received: by mail-io0-f193.google.com with SMTP id f89so14876502ioj.4; Wed, 17 Jan 2018 16:39:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=hlDhubmeqhH1m/+by6UfLUI03AMXwP/SiTOlCgaK9Qs=; b=TlOtR88RNvN2u7vPjaeaUXwy3dHAWaVd62z2p0jf9mtiBg5gDAqwymU811uBMW4WEC Jq53pgBQNQFhoTDm8Qxppc8Fi75azLM1WkbuVrMWvja+2M8M6DWdEP63+wUQzYYA++3n iuxghn6isf+ORdopEFmJ/5ysUFy7739/pmFXbRl/ncS0lo+25dxIR48iqSUwoW2FPZNi vIh0DmuUHI3KLyJU7LslmJ1a/WXm6LGy4XcTm+Ofy9/jJGhy5D7CZOEf+SpEI6INcBb8 /vpVOhRCyt+e+2PBH+XKs0QOdJpB0hTnHqaf+90MQDjc9WVhOoT5chOKnmWf3Cq5bNJl +Ubg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=hlDhubmeqhH1m/+by6UfLUI03AMXwP/SiTOlCgaK9Qs=; b=rJZudDTh3GjfIWiZRpO25QJKjVNcB6f2/hcvTKu+otSOGJMuz3EsdCVkRMzB+VnSkj ksPLYvUP7sQyUg960QyLA6KD2ZWezBPbVb7ESPYBkNecx4ZpyXaO9Gz5OPURdHUnJHR8 GchHDJQESmwux/bwvk1glqcrTHS4x1LwTP/VypOy1/K2z2Wf7mPcYI3NaIL7+Hk6NWRE sETpTHmMXHTzmreDOqbeU20fHrGG42oWilyY71gznnArv0FOtFLwBgtBu7f4V57NMav4 dJlHv80sdXRj9nkKm7diX84xjGaRc2XYB/qo2NKsIiYl7GVXRQjsBiQV0Ga7wctSnPwE iA+w== X-Gm-Message-State: AKwxytdH1U/9ULWnGD/Z2HSh5P2C3iBFkKqnad6LqRgJUEDMgYuVgXdd JWsMalZCR9DiNs6dOmOyarY= X-Received: by 10.107.222.20 with SMTP id v20mr21220790iog.82.1516235991939; Wed, 17 Jan 2018 16:39:51 -0800 (PST) Received: from gmail.com ([2620:15c:17:3:dc28:5c82:b905:e8a8]) by smtp.gmail.com with ESMTPSA id f63sm3125438ioj.43.2018.01.17.16.39.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 17 Jan 2018 16:39:51 -0800 (PST) Date: Wed, 17 Jan 2018 16:39:49 -0800 From: Eric Biggers To: =?iso-8859-1?Q?Andr=E9?= Draszik Cc: linux-kernel@vger.kernel.org, "Theodore Y. Ts'o" , Jaegeuk Kim , linux-fscrypt@vger.kernel.org, Eric Biggers Subject: Re: [PATCH v2 1/2] fscrypt: add support for the encrypted key type Message-ID: <20180118003949.snqq64wdbc2zgq3o@gmail.com> References: <20180111040022.GA943@zzz.localdomain> <20180117141319.8060-1-git@andred.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180117141319.8060-1-git@andred.net> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andr?, On Wed, Jan 17, 2018 at 02:13:18PM +0000, Andr? Draszik wrote: > We now try to acquire the key according to the > encryption policy from both key types, 'logon' > as well as 'encrypted'. > > Signed-off-by: Andr? Draszik > Cc: "Theodore Y. Ts'o" > Cc: Jaegeuk Kim > Cc: linux-fscrypt@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: Eric Biggers > > --- > changes in v2: > * dropped the previously added 'fscrypt' encrypted-key, > and just use the 'default' format The commit message needs to explain the purpose of the change, not just what the change is. It might also be helpful to combine this with the documentation patch, as some of the explanation can be in the documentation. Can you please also keep the keyrings mailing list and encrypted-keys maintainers Cc'ed on this series? I know it doesn't touch the keyrings code directly anymore, but people may still be interested. Historically there have also been a lot of bugs in code using the keyrings API, e.g. not using correct locking. > --- > fs/crypto/keyinfo.c | 72 +++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 50 insertions(+), 22 deletions(-) > > diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c > index 5e6e846f5a24..925af599f954 100644 > --- a/fs/crypto/keyinfo.c > +++ b/fs/crypto/keyinfo.c > @@ -10,6 +10,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -66,14 +67,20 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE], > return res; > } > > -static int validate_user_key(struct fscrypt_info *crypt_info, > +static inline struct key *fscrypt_get_encrypted_key(const char *sig) > +{ Call this 'description', not 'sig'. I think you copied the 'sig' naming from eCryptfs, but it doesn't make sense for fscrypt. > + if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS)) > + return request_key(&key_type_encrypted, sig, NULL); > + return ERR_PTR(-ENOKEY); > +} > + > +static int validate_keyring_key(struct fscrypt_info *crypt_info, > struct fscrypt_context *ctx, u8 *raw_key, > const char *prefix, int min_keysize) > { > char *description; > struct key *keyring_key; > - struct fscrypt_key *master_key; > - const struct user_key_payload *ukp; > + struct fscrypt_key *master_key, master_key_; > int res; > > description = kasprintf(GFP_NOFS, "%s%*phN", prefix, > @@ -83,28 +90,45 @@ static int validate_user_key(struct fscrypt_info *crypt_info, > return -ENOMEM; > > keyring_key = request_key(&key_type_logon, description, NULL); > + if (IS_ERR(keyring_key)) > + keyring_key = fscrypt_get_encrypted_key(description); I think the fallback to the "encrypted" key type should only happen if request_key() returns ERR_PTR(-ENOKEY), indicating that the "logon" key isn't present. Otherwise it will fall back for other types of errors as well which doesn't really make sense. > kfree(description); > if (IS_ERR(keyring_key)) > return PTR_ERR(keyring_key); > down_read(&keyring_key->sem); > > - if (keyring_key->type != &key_type_logon) { > + if (keyring_key->type == &key_type_logon) { > + const struct user_key_payload *ukp; > + > + ukp = user_key_payload_locked(keyring_key); > + if (!ukp) { > + /* key was revoked before we acquired its semaphore */ > + res = -EKEYREVOKED; > + goto out; > + } > + if (ukp->datalen != sizeof(struct fscrypt_key)) { > + res = -EINVAL; > + goto out; > + } > + master_key = (struct fscrypt_key *)ukp->data; > + } else if (keyring_key->type == &key_type_encrypted) { > + const struct encrypted_key_payload *ekp; > + > + ekp = keyring_key->payload.data[0]; > + master_key = &master_key_; > + > + master_key->mode = 0; > + memcpy (master_key->raw, ekp->decrypted_data, > + min (sizeof (master_key->raw), > + (size_t) ekp->decrypted_datalen)); > + master_key->size = ekp->decrypted_datalen; > + } else { > printk_once(KERN_WARNING > - "%s: key type must be logon\n", __func__); > + "%s: key type must be logon or encrypted\n", > + __func__); > res = -ENOKEY; > goto out; > } > - ukp = user_key_payload_locked(keyring_key); > - if (!ukp) { > - /* key was revoked before we acquired its semaphore */ > - res = -EKEYREVOKED; > - goto out; > - } > - if (ukp->datalen != sizeof(struct fscrypt_key)) { > - res = -EINVAL; > - goto out; > - } > - master_key = (struct fscrypt_key *)ukp->data; > BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE); > > if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE > @@ -113,9 +137,13 @@ static int validate_user_key(struct fscrypt_info *crypt_info, > "%s: key size incorrect: %d\n", > __func__, master_key->size); > res = -ENOKEY; > - goto out; > + goto out_clear_key; > } > res = derive_key_aes(ctx->nonce, master_key, raw_key); > + > +out_clear_key: > + if (master_key == &master_key_) > + memzero_explicit(master_key->raw, sizeof (master_key->raw)); > out: > up_read(&keyring_key->sem); > key_put(keyring_key); > @@ -302,12 +330,12 @@ int fscrypt_get_encryption_info(struct inode *inode) > if (!raw_key) > goto out; No need for the temporary 'struct fscrypt_key', just use ekp->decrypted_data and ekp->decrypted_datalen directly. Eric