From patchwork Sat Feb 4 14:24:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 9555635 X-Patchwork-Delegate: johannes@sipsolutions.net 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 7E8B360424 for ; Sat, 4 Feb 2017 14:24:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 675D624DA2 for ; Sat, 4 Feb 2017 14:24:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 57811262FF; Sat, 4 Feb 2017 14:24:50 +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.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM 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 3290324DA2 for ; Sat, 4 Feb 2017 14:24:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751066AbdBDOYi (ORCPT ); Sat, 4 Feb 2017 09:24:38 -0500 Received: from mail-it0-f46.google.com ([209.85.214.46]:35200 "EHLO mail-it0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750851AbdBDOYh (ORCPT ); Sat, 4 Feb 2017 09:24:37 -0500 Received: by mail-it0-f46.google.com with SMTP id 203so29708294ith.0 for ; Sat, 04 Feb 2017 06:24:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=korVcQWYXSIpDce+6zpGq2TVuzm3NsGkJJh2UQazZ3o=; b=SP+N7r+0lBUolDiEX1EeaT++8ee9VkQYULOA6HtoD+qDWNl416gjmdsXHJ3jIXioNJ 0748PggxMLxmYjeLIqtLr1rJbiDL7DaFtGWX5pbWhlJivWmevIRgWqmg4hsZrvo35kqR UGqwKki29PibhCcLMLKTh40iV2k1p90iUoWxQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=korVcQWYXSIpDce+6zpGq2TVuzm3NsGkJJh2UQazZ3o=; b=lDADwpnPdUTZO0O0MIW/lNCjKyC7vOlDGQDM/JUKHkJK/toeba9p8hpcMAHYVRRnWp O65RrEgGgjA0oe/vyXFoXIuv0MPVBoqB04FGx5nrW8p5cdx0d8tY+zaWmbzB+vokuWW0 3i/9vSJm4QFkj/Fx1pjbX5x4zaLHtqMXcX5BEktsRRS3sVxHtRfrx0iBC8tF8wbu5o1V jMVmD3jCqfa+5oPtRETz4da7uMCr7PY3SgND0nyu3ddPoCKnCb6c16571gk5MmpPkMhJ PGLj2NTGyghZGdIC6t7vtuQhdnWwy87WyiLpabGBF8Z7qDWAiGypfBCPLB/eP/tQLFTR rCyw== X-Gm-Message-State: AIkVDXLvONvh8LyPTYNYf4whbe9/ONzV2lFCGWt0DYkRxTiwWNv3AUnLMV9cgWvG8qZTsIjpckcrMD3AI2zT4A2N X-Received: by 10.36.115.7 with SMTP id y7mr1406312itb.63.1486218276471; Sat, 04 Feb 2017 06:24:36 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.139 with HTTP; Sat, 4 Feb 2017 06:24:36 -0800 (PST) In-Reply-To: <20170204113514.GA4350@jouni.qca.qualcomm.com> References: <1486149955-11825-1-git-send-email-ard.biesheuvel@linaro.org> <20170203214707.GA14147@jouni.qca.qualcomm.com> <20170204113514.GA4350@jouni.qca.qualcomm.com> From: Ard Biesheuvel Date: Sat, 4 Feb 2017 14:24:36 +0000 Message-ID: Subject: Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac To: "Malinen, Jouni" Cc: "johannes@sipsolutions.net" , "linux-wireless@vger.kernel.org" , "davem@davemloft.net" , "netdev@vger.kernel.org" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 4 February 2017 at 11:35, Malinen, Jouni wrote: > On Fri, Feb 03, 2017 at 09:55:36PM +0000, Ard Biesheuvel wrote: >> OK, that looks like something I could figure out how to use. But are >> you saying the CMAC code is never called in practice? > > It will get called if there is a frame that were to need to use BIP. > There are some APs that do send broadcast addressed Deauthentication and > Disassociation frames when terminating the network and those would get > here. In theory, someone could come up with a new Action frame use case > as well with a group addressed Robust Action frame, but no such thing is > defined today. Anyway, this will be needed for certification purposes > and to block DoS with broadcast addressed Deauthentication and > Disassociation frames even if there is not really a common use case > using BIP frames today. > >> I did spot something peculiar when looking at the code: if I am >> reading the following sequence correctly (from >> fils_encrypt_assoc_req()) > >> addr[0] = mgmt->sa; > ... >> addr[4] = capab; >> len[4] = encr - capab; >> >> crypt_len = skb->data + skb->len - encr; >> skb_put(skb, AES_BLOCK_SIZE); >> return aes_siv_encrypt(assoc_data->fils_kek, assoc_data->fils_kek_len, >> encr, crypt_len, 1, addr, len, encr); >> >> the addr[]/len[] arrays are populated with 5 (addr, len) pairs, but >> only one is actually passed into aes_siv_encrypt()? This is actually >> the main reason I stopped looking into whether I could convert it to >> CMAC, because I couldn't figure it out. > > Argh.. Thanks for finding this! > > That is indeed supposed to have num_elems == 5. This "works" since the > other end of the exchange is actually in user space (hostapd) and a > similar error is there.. > Interesting. > This comes from the development history of FILS support.. The draft > standard changed this AES-SIV AAD design from P802.11ai/D7.0 to D8.0 > from a single AAD vector with concatenated fields to separate vectors. > Clearly I added the vectors here in addr[] and len[] arrays, but forgot > to update the num_elems parameter in this direction (STA->AP); the other > direction for Association Response frame is actually using correct > number of AAD vectors. > > I'll fix this in hostapd and mac80211. > There is another issue I spotted: the skcipher you allocate may be of the async variant, which may return from skcipher_encrypt() with -EINPROGRESS as soon as it has queued the request. Since you don't deal with that result, you should allocate a sync cipher explicitly: > I did not actually remember that the AP side was still in hostapd for > FILS AEAD in case of mac80211, but with that in mind, the answer to your > earlier question about testing these becomes much simpler.. All you need > to do is this with hostap.git hwsim tests: > > hostap/tests/hwsim/vm$ ./vm-run.sh ap_cipher_bip fils_sk_erp > Starting test run in a virtual machine > ./run-all.sh: passing the following args to run-tests.py: ap_cipher_bip fils_sk_erp > START ap_cipher_bip 1/2 > PASS ap_cipher_bip 0.703048 2017-02-04 11:21:35.341174 > START fils_sk_erp 2/2 > PASS fils_sk_erp 0.667927 2017-02-04 11:21:36.029205 > passed all 2 test case(s) > ALL-PASSED > > > ap_cipher_bip uses wlantest to verify the BIP (AES-128-CMAC) protection > in the frames and fils_sk_erp goes through FILS AEAD exchange with one > side of the operation in mac80211 and the other one in hostapd. This > should be able to discover if something is broken in the AES related > changes in crypto. > > And this particular example run here was with your two patches applied, > to the proposed net/mac80211/aes_cmac.c change seems to work fine. Thanks for the instructions and thanks for testing. If I manage to run this locally, I will follow up with an alternative patch #1 that switches FILS to use cmac(aes) as well (which looks reasonably feasible now that I understand the code) Regards, Ard. diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c index ecfdd97758a3..a31be5262283 100644 --- a/net/mac80211/fils_aead.c +++ b/net/mac80211/fils_aead.c @@ -124,7 +124,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len, /* CTR */ - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(tfm2)) { kfree(tmp); return PTR_ERR(tfm2); @@ -183,7 +183,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len, /* CTR */ - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0); + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(tfm2)) return PTR_ERR(tfm2); /* K2 for CTR */