Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:
Hi Harsh,
> >> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
> >> *err)
> >> +{
> >> + u8 temp[SHA512_DIGEST_SIZE];
> >> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> >> + int authsize = crypto_aead_authsize(tfm);
> >> + struct cpl_fw6_pld *fw6_pld;
> >> + int cmp = 0;
> >> +
> >> + fw6_pld = (struct cpl_fw6_pld *)input;
> >> + if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
> >> + (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
> >> + cmp = memcmp(&fw6_pld->data[2], (fw6_pld + 1), authsize);
> >> + } else {
> >> +
> >> + sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
> >> + authsize, req->assoclen +
> >> + req->cryptlen - authsize);
> >
> > I am wondering whether the math is correct here in any case. It is
> > permissible that we have an AAD size of 0 and even a zero-sized
> > ciphertext. How is such scenario covered here?
>
> Here we are trying to copy user supplied tag to local buffer(temp) for
> decrypt operation only. relative index of tag in src sg list will not
> change when AAD is zero and in decrypt operation cryptlen > authsize.
I am just wondering where this is checked. Since all of these implementations
are directly accessible from unprivileged user space, we should be careful.
> >> + cmp = memcmp(temp, (fw6_pld + 1), authsize);
> >
> > I would guess in both cases memcmp should be replaced with crypto_memneq
>
> Yes can be done
>
> >> + }
> >> + if (cmp)
> >> + *err = -EBADMSG;
> >> + else
> >> + *err = 0;
> >
> > What do you think about memzero_explicit(tmp)?
>
> No Idea why we needs explicitly setting of zero for local variable. Please
> share some online resources to understand this.
In dumps, the stack is also produced. Yet I see that stack memory is very
volatile and thus will be overwritten soon. Thus my common approach for
sensitive data is that heap variables must be zeroized. Stack variables are
suggested to be zeroized. As far as I understand the code, temp will hold a
copy of the tag value, i.e. a public piece of information. If this is correct,
that I concur that a memset may not be needed after all.
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
