Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752754AbdFPHjS convert rfc822-to-8bit (ORCPT ); Fri, 16 Jun 2017 03:39:18 -0400 Received: from mail.sigma-star.at ([95.130.255.111]:45996 "EHLO mail.sigma-star.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752369AbdFPHjQ (ORCPT ); Fri, 16 Jun 2017 03:39:16 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: Re: [PATCH v4] fscrypt: Add support for AES-128-CBC From: David Gstir In-Reply-To: <20170615204841.GA66403@gmail.com> Date: Fri, 16 Jun 2017 09:39:07 +0200 Cc: "Theodore Ts'o" , Michael Halcrow , Jaegeuk Kim , Richard Weinberger , herbert@gondor.apana.org.au, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fscrypt@vger.kernel.org, Daniel Walter Content-Transfer-Encoding: 8BIT Message-Id: <5565BD72-986F-4338-8562-6B5F9FB72110@sigma-star.at> References: <20170517180850.GA91213@gmail.com> <20170523051120.15698-1-david@sigma-star.at> <20170615204129.GA23647@google.com> <20170615204841.GA66403@gmail.com> To: Eric Biggers X-Mailer: Apple Mail (2.3273) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1499 Lines: 37 > On 15 Jun 2017, at 22:48, Eric Biggers wrote: > > On Thu, Jun 15, 2017 at 01:41:29PM -0700, Michael Halcrow wrote: >>> static int validate_user_key(struct fscrypt_info *crypt_info, >>> struct fscrypt_context *ctx, u8 *raw_key, >>> - const char *prefix) >>> + const char *prefix, int min_keysize) >>> { >>> char *description; >>> struct key *keyring_key; >>> @@ -111,50 +116,60 @@ static int validate_user_key(struct fscrypt_info *crypt_info, >>> 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 != FS_AES_256_XTS_KEY_SIZE) { >>> + if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE >>> + || master_key->size % AES_BLOCK_SIZE != 0) { >> >> I suggest validating the provided key size directly against the mode. >> Else, it looks to me that this code will accept a 128-bit key for >> AES-256. >> > > It's doing that already; min_keysize depends on the mode. We are a bit more forgiving than the code was before: In case AES-128-CBC is selected, we accept a longer key and use the first 128 bits of the derived key. (see fscrypt_get_encryption_info()) The alternative is to make this check as strict as it was and just check for master_key->size != min_keysize. IMO the current check is okay. I will however add a comment that documents this. We could also add a pr_warn_once(), but I don't think this is really necessary. David