Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758406AbcDMFHb (ORCPT ); Wed, 13 Apr 2016 01:07:31 -0400 Received: from mail-vk0-f67.google.com ([209.85.213.67]:33309 "EHLO mail-vk0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758314AbcDMFH3 (ORCPT ); Wed, 13 Apr 2016 01:07:29 -0400 MIME-Version: 1.0 In-Reply-To: <20160409205056.GH25498@ZenIV.linux.org.uk> References: <20160409204301.GF25498@ZenIV.linux.org.uk> <20160409205056.GH25498@ZenIV.linux.org.uk> Date: Wed, 13 Apr 2016 00:07:28 -0500 Message-ID: Subject: Re: [PATCH 2/6] cifs: merge the hash calculation helpers From: Shirish Pargaonkar To: Al Viro Cc: linux-cifs , LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13632 Lines: 323 Looks correct. Tested with -o vers=1.0, -o vers=2.0, and -o vers=2.1. Will test using mount option vers=3.0 as soon as I find a server. Reviewed-by: Shirish Pargaonkar Tested-by: Shirish Pargaonkar On Sat, Apr 9, 2016 at 3:50 PM, Al Viro wrote: > three practically identical copies... > > Signed-off-by: Al Viro > --- > fs/cifs/cifsencrypt.c | 97 ++++++++++++++++++++++++------------------- > fs/cifs/cifsproto.h | 3 ++ > fs/cifs/smb2transport.c | 107 +++++------------------------------------------- > 3 files changed, 67 insertions(+), 140 deletions(-) > > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index 4897dac..6aeb8d4 100644 > --- a/fs/cifs/cifsencrypt.c > +++ b/fs/cifs/cifsencrypt.c > @@ -66,45 +66,15 @@ cifs_crypto_shash_md5_allocate(struct TCP_Server_Info *server) > return 0; > } > > -/* > - * Calculate and return the CIFS signature based on the mac key and SMB PDU. > - * The 16 byte signature must be allocated by the caller. Note we only use the > - * 1st eight bytes and that the smb header signature field on input contains > - * the sequence number before this function is called. Also, this function > - * should be called with the server->srv_mutex held. > - */ > -static int cifs_calc_signature(struct smb_rqst *rqst, > - struct TCP_Server_Info *server, char *signature) > +int __cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature, > + struct shash_desc *shash) > { > int i; > int rc; > struct kvec *iov = rqst->rq_iov; > int n_vec = rqst->rq_nvec; > > - if (iov == NULL || signature == NULL || server == NULL) > - return -EINVAL; > - > - if (!server->secmech.sdescmd5) { > - rc = cifs_crypto_shash_md5_allocate(server); > - if (rc) { > - cifs_dbg(VFS, "%s: Can't alloc md5 crypto\n", __func__); > - return -1; > - } > - } > - > - rc = crypto_shash_init(&server->secmech.sdescmd5->shash); > - if (rc) { > - cifs_dbg(VFS, "%s: Could not init md5\n", __func__); > - return rc; > - } > - > - rc = crypto_shash_update(&server->secmech.sdescmd5->shash, > - server->session_key.response, server->session_key.len); > - if (rc) { > - cifs_dbg(VFS, "%s: Could not update with response\n", __func__); > - return rc; > - } > - > for (i = 0; i < n_vec; i++) { > if (iov[i].iov_len == 0) > continue; > @@ -117,12 +87,10 @@ static int cifs_calc_signature(struct smb_rqst *rqst, > if (i == 0) { > if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ > break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update(&server->secmech.sdescmd5->shash, > + rc = crypto_shash_update(shash, > iov[i].iov_base + 4, iov[i].iov_len - 4); > } else { > - rc = > - crypto_shash_update(&server->secmech.sdescmd5->shash, > + rc = crypto_shash_update(shash, > iov[i].iov_base, iov[i].iov_len); > } > if (rc) { > @@ -134,21 +102,64 @@ static int cifs_calc_signature(struct smb_rqst *rqst, > > /* now hash over the rq_pages array */ > for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > + void *kaddr = kmap(rqst->rq_pages[i]); > + size_t len = rqst->rq_pagesz; > + > + if (i == rqst->rq_npages - 1) > + len = rqst->rq_tailsz; > + > + crypto_shash_update(shash, kaddr, len); > > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdescmd5->shash, > - p_iov.iov_base, p_iov.iov_len); > kunmap(rqst->rq_pages[i]); > } > > - rc = crypto_shash_final(&server->secmech.sdescmd5->shash, signature); > + rc = crypto_shash_final(shash, signature); > if (rc) > - cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__); > + cifs_dbg(VFS, "%s: Could not generate hash\n", __func__); > > return rc; > } > > +/* > + * Calculate and return the CIFS signature based on the mac key and SMB PDU. > + * The 16 byte signature must be allocated by the caller. Note we only use the > + * 1st eight bytes and that the smb header signature field on input contains > + * the sequence number before this function is called. Also, this function > + * should be called with the server->srv_mutex held. > + */ > +static int cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature) > +{ > + int rc; > + > + if (!rqst->rq_iov || !signature || !server) > + return -EINVAL; > + > + if (!server->secmech.sdescmd5) { > + rc = cifs_crypto_shash_md5_allocate(server); > + if (rc) { > + cifs_dbg(VFS, "%s: Can't alloc md5 crypto\n", __func__); > + return -1; > + } > + } > + > + rc = crypto_shash_init(&server->secmech.sdescmd5->shash); > + if (rc) { > + cifs_dbg(VFS, "%s: Could not init md5\n", __func__); > + return rc; > + } > + > + rc = crypto_shash_update(&server->secmech.sdescmd5->shash, > + server->session_key.response, server->session_key.len); > + if (rc) { > + cifs_dbg(VFS, "%s: Could not update with response\n", __func__); > + return rc; > + } > + > + return __cifs_calc_signature(rqst, server, signature, > + &server->secmech.sdescmd5->shash); > +} > + > /* must be called with server->srv_mutex held */ > int cifs_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server, > __u32 *pexpected_response_sequence_number) > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index eed7ff5..d9b4f44 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -512,4 +512,7 @@ int cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon, > struct cifs_sb_info *cifs_sb, > const unsigned char *path, char *pbuf, > unsigned int *pbytes_written); > +int __cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature, > + struct shash_desc *shash); > #endif /* _CIFSPROTO_H */ > diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c > index 8732a43..bc9a7b6 100644 > --- a/fs/cifs/smb2transport.c > +++ b/fs/cifs/smb2transport.c > @@ -135,11 +135,10 @@ smb2_find_smb_ses(struct smb2_hdr *smb2hdr, struct TCP_Server_Info *server) > int > smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) > { > - int i, rc; > + int rc; > unsigned char smb2_signature[SMB2_HMACSHA256_SIZE]; > unsigned char *sigptr = smb2_signature; > struct kvec *iov = rqst->rq_iov; > - int n_vec = rqst->rq_nvec; > struct smb2_hdr *smb2_pdu = (struct smb2_hdr *)iov[0].iov_base; > struct cifs_ses *ses; > > @@ -171,53 +170,11 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) > return rc; > } > > - for (i = 0; i < n_vec; i++) { > - if (iov[i].iov_len == 0) > - continue; > - if (iov[i].iov_base == NULL) { > - cifs_dbg(VFS, "null iovec entry\n"); > - return -EIO; > - } > - /* > - * The first entry includes a length field (which does not get > - * signed that occupies the first 4 bytes before the header). > - */ > - if (i == 0) { > - if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ > - break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update( > - &server->secmech.sdeschmacsha256->shash, > - iov[i].iov_base + 4, iov[i].iov_len - 4); > - } else { > - rc = > - crypto_shash_update( > - &server->secmech.sdeschmacsha256->shash, > - iov[i].iov_base, iov[i].iov_len); > - } > - if (rc) { > - cifs_dbg(VFS, "%s: Could not update with payload\n", > - __func__); > - return rc; > - } > - } > - > - /* now hash over the rq_pages array */ > - for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > - > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdeschmacsha256->shash, > - p_iov.iov_base, p_iov.iov_len); > - kunmap(rqst->rq_pages[i]); > - } > - > - rc = crypto_shash_final(&server->secmech.sdeschmacsha256->shash, > - sigptr); > - if (rc) > - cifs_dbg(VFS, "%s: Could not generate sha256 hash\n", __func__); > + rc = __cifs_calc_signature(rqst, server, sigptr, > + &server->secmech.sdeschmacsha256->shash); > > - memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > + if (!rc) > + memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > > return rc; > } > @@ -395,12 +352,10 @@ generate_smb311signingkey(struct cifs_ses *ses) > int > smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) > { > - int i; > int rc = 0; > unsigned char smb3_signature[SMB2_CMACAES_SIZE]; > unsigned char *sigptr = smb3_signature; > struct kvec *iov = rqst->rq_iov; > - int n_vec = rqst->rq_nvec; > struct smb2_hdr *smb2_pdu = (struct smb2_hdr *)iov[0].iov_base; > struct cifs_ses *ses; > > @@ -431,54 +386,12 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) > cifs_dbg(VFS, "%s: Could not init cmac aes\n", __func__); > return rc; > } > + > + rc = __cifs_calc_signature(rqst, server, sigptr, > + &server->secmech.sdesccmacaes->shash); > > - for (i = 0; i < n_vec; i++) { > - if (iov[i].iov_len == 0) > - continue; > - if (iov[i].iov_base == NULL) { > - cifs_dbg(VFS, "null iovec entry"); > - return -EIO; > - } > - /* > - * The first entry includes a length field (which does not get > - * signed that occupies the first 4 bytes before the header). > - */ > - if (i == 0) { > - if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ > - break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update( > - &server->secmech.sdesccmacaes->shash, > - iov[i].iov_base + 4, iov[i].iov_len - 4); > - } else { > - rc = > - crypto_shash_update( > - &server->secmech.sdesccmacaes->shash, > - iov[i].iov_base, iov[i].iov_len); > - } > - if (rc) { > - cifs_dbg(VFS, "%s: Couldn't update cmac aes with payload\n", > - __func__); > - return rc; > - } > - } > - > - /* now hash over the rq_pages array */ > - for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > - > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdesccmacaes->shash, > - p_iov.iov_base, p_iov.iov_len); > - kunmap(rqst->rq_pages[i]); > - } > - > - rc = crypto_shash_final(&server->secmech.sdesccmacaes->shash, > - sigptr); > - if (rc) > - cifs_dbg(VFS, "%s: Could not generate cmac aes\n", __func__); > - > - memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > + if (!rc) > + memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > > return rc; > } > -- > 2.8.0.rc3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html