From patchwork Sun Jun 30 19:10:39 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 2804631 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D27869F3EB for ; Sun, 30 Jun 2013 19:10:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8DCD4200F4 for ; Sun, 30 Jun 2013 19:10:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 397E8200F0 for ; Sun, 30 Jun 2013 19:10:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752006Ab3F3TKk (ORCPT ); Sun, 30 Jun 2013 15:10:40 -0400 Received: from mail-pa0-f43.google.com ([209.85.220.43]:63092 "EHLO mail-pa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751529Ab3F3TKj (ORCPT ); Sun, 30 Jun 2013 15:10:39 -0400 Received: by mail-pa0-f43.google.com with SMTP id hz11so4185552pad.30 for ; Sun, 30 Jun 2013 12:10:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:cc:content-type; bh=FDDee2KUJQKzOL+NI6RD6PftT/AxRVoOcBrKq/Vdxmc=; b=a/+HjuOKFfBCXmzmy8jmIh0CRidScu88D4+wj5mYB5c5yDRl7I0h5VXV/fS+chJN1a jjPX2l1FcSa8xl867SrWmfYa6nvAI65H9HT3U7obe38igTiN6pZCqJVwc99krnZ+WzGX vPnk008mSE2UzWA/g8wtwLDZ9xsp/DiCyg8CjUbEQP0llCn+iPhXgoGnlCQo7/yG51lM PsNyGtrLcp2g/89zJpd71TU7fB6NqVGd5qOfRvfD5kZqJ3GHp3AIApPUM6B7KkmIJidE ul9dtjlSOXiFo9xEVM5YmD+fWdgX9h7juVnmBdfBVnqm1O4pb3AUMJi/KVu2Nniu1rnW +/rA== MIME-Version: 1.0 X-Received: by 10.66.248.68 with SMTP id yk4mr20897860pac.137.1372619439304; Sun, 30 Jun 2013 12:10:39 -0700 (PDT) Received: by 10.68.128.9 with HTTP; Sun, 30 Jun 2013 12:10:39 -0700 (PDT) Date: Sun, 30 Jun 2013 14:10:39 -0500 Message-ID: Subject: Limiting allocation of smb2 crypto structs to smb2 mounts From: Steve French To: Jeff Layton Cc: Shirish Pargaonkar , Pavel Shilovsky , sprabhu@redhat.com, linux-cifs@vger.kernel.org, samba-technical Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Updated patch to try to prevent allocation of smb2 or smb3 crypto secmech structures unless needed. There is probably more updates that could be done to cleanup cifs - but the more important part is to get the smb2/smb3 part cleaned up. if (rc) { @@ -129,6 +191,10 @@ generate_smb3signingkey(struct TCP_Server_Info *server) memset(prfhash, 0x0, SMB2_HMACSHA256_SIZE); memset(server->smb3signingkey, 0x0, SMB3_SIGNKEY_SIZE); + /* SMB3 essentially requires signing so no harm allocating it early */ + if (server->secmech.hmacsha256 == NULL) + smb3_crypto_shash_allocate(server); + rc = crypto_shash_setkey(server->secmech.hmacsha256, server->session_key.response, SMB2_NTLMV2_SESSKEY_SIZE); if (rc) { @@ -210,6 +276,9 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) return rc; } + /* we already allocate sdesccmacaes when we init smb3 signing key, + so unlike smb2 we do not have to check here if secmech + are initialized */ rc = crypto_shash_init(&server->secmech.sdesccmacaes->shash); if (rc) { cifs_dbg(VFS, "%s: Could not init cmac aes\n", __func__); On Sun, Jun 30, 2013 at 5:25 AM, Jeff Layton wrote: > On Sat, 29 Jun 2013 23:06:12 -0500 > Steve French wrote: > >> if we setup a socket we send a negotiate protocol >> and decide on the smb version at that point. If we mount a second >> user to the same server, he will use the same socket and thus the same >> dialect that we did the first mount on - i don't know a way to mix >> multiple dialects on the same socket and I don't think we should. >> > In any case...match_server does this: > > if ((server->vals != vol->vals) || (server->ops != vol->ops)) > return 0; > > ...which should make it impossible to share sockets when the versions > don't match. In principle, I guess we could probably share sockets > between (for instance) 2.1 and 2.002, but that's an optimization that > could be done later. I also don't think that that is a good idea to change the dialect of a user of the socket on the fly. We will send the negotiate and decide on the dialect when connecting to the socket(s), and each subsequent user will create an new smb2/smb3 session on the same socket, thus with the same smb2 dialect. I don't think there is any case where you expect the server to change from smb2.1 to smb3 or to smb3.02 without dropping the tcp session) - although I am open to the idea of allowing "upgrading security on the fly" to mandate signing (or encryption) on the fly if security needs change due to an emergency. > The way the crypto is allocated is a serious wart. The algorithms are > being allocated too early. It would be preferable even to delay > allocating the crypto stuff at all until it's actually needed. Well - the patch I proposed at least allocates the smb2 ones when we have negotiated smb2 or smb2.1, and smb3 ones when smb3 or smb3.02 is negotiated. The alternative is fine with me, but means checking on EVERY signing request (since you don't have to have signing on the first request(s) but then end up signing a later one - e.g. for the case of secure renogotiate) > If, for instance I mount with sec=krb5 and don't request signing, > there's no need to allocate any of this stuff. This is not harmless > either. We have at least one customer that boots their machines with > fips=1. They're basically unable to use cifs.ko at all currently > because the crypto allocations fail. OK - that is a good point. diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 3d8bf94..e0d94e1 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -745,20 +745,6 @@ cifs_crypto_shash_allocate(struct TCP_Server_Info *server) goto crypto_allocate_md5_fail; } - server->secmech.hmacsha256 = crypto_alloc_shash("hmac(sha256)", 0, 0); - if (IS_ERR(server->secmech.hmacsha256)) { - cifs_dbg(VFS, "could not allocate crypto hmacsha256\n"); - rc = PTR_ERR(server->secmech.hmacsha256); - goto crypto_allocate_hmacsha256_fail; - } - - server->secmech.cmacaes = crypto_alloc_shash("cmac(aes)", 0, 0); - if (IS_ERR(server->secmech.cmacaes)) { - cifs_dbg(VFS, "could not allocate crypto cmac-aes"); - rc = PTR_ERR(server->secmech.cmacaes); - goto crypto_allocate_cmacaes_fail; - } - size = sizeof(struct shash_desc) + crypto_shash_descsize(server->secmech.hmacmd5); server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL); @@ -779,45 +765,12 @@ cifs_crypto_shash_allocate(struct TCP_Server_Info *server) server->secmech.sdescmd5->shash.tfm = server->secmech.md5; server->secmech.sdescmd5->shash.flags = 0x0; - size = sizeof(struct shash_desc) + - crypto_shash_descsize(server->secmech.hmacsha256); - server->secmech.sdeschmacsha256 = kmalloc(size, GFP_KERNEL); - if (!server->secmech.sdeschmacsha256) { - rc = -ENOMEM; - goto crypto_allocate_hmacsha256_sdesc_fail; - } - server->secmech.sdeschmacsha256->shash.tfm = server->secmech.hmacsha256; - server->secmech.sdeschmacsha256->shash.flags = 0x0; - - size = sizeof(struct shash_desc) + - crypto_shash_descsize(server->secmech.cmacaes); - server->secmech.sdesccmacaes = kmalloc(size, GFP_KERNEL); - if (!server->secmech.sdesccmacaes) { - cifs_dbg(VFS, "%s: Can't alloc cmacaes\n", __func__); - rc = -ENOMEM; - goto crypto_allocate_cmacaes_sdesc_fail; - } - server->secmech.sdesccmacaes->shash.tfm = server->secmech.cmacaes; - server->secmech.sdesccmacaes->shash.flags = 0x0; - return 0; -crypto_allocate_cmacaes_sdesc_fail: - kfree(server->secmech.sdeschmacsha256); - -crypto_allocate_hmacsha256_sdesc_fail: - kfree(server->secmech.sdescmd5); - crypto_allocate_md5_sdesc_fail: kfree(server->secmech.sdeschmacmd5); crypto_allocate_hmacmd5_sdesc_fail: - crypto_free_shash(server->secmech.cmacaes); - -crypto_allocate_cmacaes_fail: - crypto_free_shash(server->secmech.hmacsha256); - -crypto_allocate_hmacsha256_fail: crypto_free_shash(server->secmech.md5); crypto_allocate_md5_fail: diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index afcb8a1..aa5bf23 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2108,14 +2108,15 @@ cifs_get_tcp_session(struct smb_vol *volume_info) goto out_err; } + tcp_ses->ops = volume_info->ops; + tcp_ses->vals = volume_info->vals; + rc = cifs_crypto_shash_allocate(tcp_ses); if (rc) { cifs_dbg(VFS, "could not setup hash structures rc %d\n", rc); goto out_err; } - tcp_ses->ops = volume_info->ops; - tcp_ses->vals = volume_info->vals; cifs_set_net_ns(tcp_ses, get_net(current->nsproxy->net_ns)); tcp_ses->hostname = extract_hostname(volume_info->UNC); if (IS_ERR(tcp_ses->hostname)) { diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c index 09b4fba..ca9d66e 100644 --- a/fs/cifs/smb2transport.c +++ b/fs/cifs/smb2transport.c @@ -39,6 +39,65 @@ #include "smb2status.h" #include "smb2glob.h" +static int +smb2_crypto_shash_allocate(struct TCP_Server_Info *server) +{ + unsigned int size; + + server->secmech.hmacsha256 = crypto_alloc_shash("hmac(sha256)", 0, 0); + if (IS_ERR(server->secmech.hmacsha256)) { + cifs_dbg(VFS, "could not allocate crypto hmacsha256\n"); + return PTR_ERR(server->secmech.hmacsha256); + } + + size = sizeof(struct shash_desc) + + crypto_shash_descsize(server->secmech.hmacsha256); + server->secmech.sdeschmacsha256 = kmalloc(size, GFP_KERNEL); + if (!server->secmech.sdeschmacsha256) { + crypto_free_shash(server->secmech.hmacsha256); + return -ENOMEM; + } + server->secmech.sdeschmacsha256->shash.tfm = server->secmech.hmacsha256; + server->secmech.sdeschmacsha256->shash.flags = 0x0; + + return 0; +} + +static int +smb3_crypto_shash_allocate(struct TCP_Server_Info *server) +{ + unsigned int size; + int rc; + + rc = smb2_crypto_shash_allocate(server); + if (rc) + return rc; + + server->secmech.cmacaes = crypto_alloc_shash("cmac(aes)", 0, 0); + if (IS_ERR(server->secmech.cmacaes)) { + cifs_dbg(VFS, "could not allocate crypto cmac-aes"); + kfree(server->secmech.sdeschmacsha256); + crypto_free_shash(server->secmech.hmacsha256); + return PTR_ERR(server->secmech.cmacaes); + } + + size = sizeof(struct shash_desc) + + crypto_shash_descsize(server->secmech.cmacaes); + server->secmech.sdesccmacaes = kmalloc(size, GFP_KERNEL); + if (!server->secmech.sdesccmacaes) { + cifs_dbg(VFS, "%s: Can't alloc cmacaes\n", __func__); + kfree(server->secmech.sdeschmacsha256); + crypto_free_shash(server->secmech.hmacsha256); + crypto_free_shash(server->secmech.cmacaes); + return -ENOMEM; + } + server->secmech.sdesccmacaes->shash.tfm = server->secmech.cmacaes; + server->secmech.sdesccmacaes->shash.flags = 0x0; + + return 0; +} + + int smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) { @@ -52,6 +111,9 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE); memset(smb2_pdu->Signature, 0x0, SMB2_SIGNATURE_SIZE); + if (server->secmech.hmacsha256 == NULL) + smb2_crypto_shash_allocate(server); + rc = crypto_shash_setkey(server->secmech.hmacsha256, server->session_key.response, SMB2_NTLMV2_SESSKEY_SIZE);