From patchwork Tue Sep 20 12:35:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 9341593 X-Patchwork-Delegate: herbert@gondor.apana.org.au 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 5585B6077A for ; Tue, 20 Sep 2016 12:36:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 464032964A for ; Tue, 20 Sep 2016 12:36:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 38ED529651; Tue, 20 Sep 2016 12:36:05 +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 57BCB2964A for ; Tue, 20 Sep 2016 12:36:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752007AbcITMgC (ORCPT ); Tue, 20 Sep 2016 08:36:02 -0400 Received: from helcar.hengli.com.au ([209.40.204.226]:60372 "EHLO helcar.hengli.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751449AbcITMgB (ORCPT ); Tue, 20 Sep 2016 08:36:01 -0400 Received: from gondolin.me.apana.org.au ([192.168.0.6]) by norbury.hengli.com.au with esmtp (Exim 4.80 #3 (Debian)) id 1bmKHE-0007TR-LJ; Tue, 20 Sep 2016 22:35:56 +1000 Received: from herbert by gondolin.me.apana.org.au with local (Exim 4.80) (envelope-from ) id 1bmKHD-0005oB-F1; Tue, 20 Sep 2016 20:35:55 +0800 Date: Tue, 20 Sep 2016 20:35:55 +0800 From: Herbert Xu To: Mimi Zohar Cc: David Howells , linux-crypto , keyrings@vger.kernel.org Subject: Re: [PATCH] trusted-keys: skcipher bug info Message-ID: <20160920123555.GA22272@gondor.apana.org.au> References: <1474373511.14532.9.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1474373511.14532.9.camel@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Sep 20, 2016 at 08:11:51AM -0400, Mimi Zohar wrote: > Hi Herbert, > > The initial random iv value, initialized in encrypted_init(), should > not be modified. Commit c3917fd "KEYS: Use skcipher", which replaced > the blkcipher with skcipher, modifies the iv in > crypto_skcipher_encrypt()/decrypt(). > > The following example creates an encrypted key, writes the key to a > file, and then loads the key from the file. To illustrate the problem, > this patch provides crypto_skcipher_encrypt()/decrypt() with a copy of > the iv. With this change, the resulting test-key and test-key1 keys > are the same. Sorry, I missed the subtlety. This patch should fix the problem. ---8<--- Subject: KEYS: Fix skcipher IV clobbering The IV must not be modified by the skcipher operation so we need to duplicate it. Fixes: c3917fd9dfbc ("KEYS: Use skcipher") Cc: stable@vger.kernel.org Reported-by: Mimi Zohar Signed-off-by: Herbert Xu diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 5adbfc3..17a0610 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -478,6 +479,7 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, struct crypto_skcipher *tfm; struct skcipher_request *req; unsigned int encrypted_datalen; + u8 iv[AES_BLOCK_SIZE]; unsigned int padlen; char pad[16]; int ret; @@ -500,8 +502,8 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, sg_init_table(sg_out, 1); sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen); - skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, - epayload->iv); + memcpy(iv, epayload->iv, sizeof(iv)); + skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv); ret = crypto_skcipher_encrypt(req); tfm = crypto_skcipher_reqtfm(req); skcipher_request_free(req); @@ -581,6 +583,7 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, struct crypto_skcipher *tfm; struct skcipher_request *req; unsigned int encrypted_datalen; + u8 iv[AES_BLOCK_SIZE]; char pad[16]; int ret; @@ -599,8 +602,8 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, epayload->decrypted_datalen); sg_set_buf(&sg_out[1], pad, sizeof pad); - skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, - epayload->iv); + memcpy(iv, epayload->iv, sizeof(iv)); + skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv); ret = crypto_skcipher_decrypt(req); tfm = crypto_skcipher_reqtfm(req); skcipher_request_free(req);