Re: [RFC PATCH v2 1/3] cifs: introduce AES-GMAC signing support for SMB 3.1.1

[Stefan Metzmacher <metze@samba.org>]

Hi Enzo,

 > +static int smb311_crypt_sign(struct smb_rqst *rqst, int num_rqst, int enc,
 > +			     bool sign_only, struct crypto_aead *tfm, u8 *key,
 > +			     unsigned int keylen, u8 *iv, unsigned int assoclen,
 > +			     unsigned int cryptlen)
 > +{
 > +	struct smb2_hdr *shdr = (struct smb2_hdr *)rqst[0].rq_iov[0].iov_base;
 > +	struct smb2_transform_hdr *tr_hdr =
 > +		(struct smb2_transform_hdr *)rqst[0].rq_iov[0].iov_base;
 > +	u8 sig[SMB2_SIGNATURE_SIZE] = { 0 };
 > +	struct aead_request *aead_req;
 > +	DECLARE_CRYPTO_WAIT(wait);
 > +	struct scatterlist *sg;

I'd propose to keep the encryption and signing cases separate
as pdu layout of an encrypted TRANSFORM is completely different
from the signing an SMB2 message. So having them in one function
is just confusing.

> +static int smb311_aes_gmac_nonce(struct smb2_hdr *shdr, bool is_server,
> +				 u8 **out_nonce)
> +{
> +	struct {
> +		/* for MessageId (8 bytes) */
> +		__le64 mid;
> +		/* for role (client or server) and if SMB2 CANCEL (4 bytes) */
> +		__le32 role;
> +	} __packed nonce;
> +
> +	if (!shdr || !out_nonce)
> +		return -EINVAL;
>   
> -	if (!rc && enc)
> -		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
> +	*out_nonce = kzalloc(SMB3_AES_GCM_NONCE, GFP_KERNEL);
> +	if (!*out_nonce)
> +		return -ENOMEM;

Why wasting time to allocate/free a 12 byte buffer for every pdu?

Can't we have a named structure and pass in a reference from the
caller, which has it on the stack?

metze