linux-kernel - Re: [PATCH v8 6/6] crypto: AF_ALG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.OSX.2.20.1607061211410.2149@mjmartin-mac01.local>
Date:	Wed, 6 Jul 2016 12:38:56 -0700 (PDT)
From:	Mat Martineau <mathew.j.martineau@...ux.intel.com>
To:	Tadeusz Struk <tadeusz.struk@...el.com>
cc:	Mat Martineau <mathew.j.martineau@...ux.intel.com>,
	dhowells@...hat.com, herbert@...dor.apana.org.au,
	smueller@...onox.de, linux-api@...r.kernel.org,
	marcel@...tmann.org, linux-kernel@...r.kernel.org,
	keyrings@...r.kernel.org, linux-crypto@...r.kernel.org,
	dwmw2@...radead.org, davem@...emloft.net
Subject: Re: [PATCH v8 6/6] crypto: AF_ALG - add support for key_id


On Tue, 5 Jul 2016, Tadeusz Struk wrote:

> Hi Mat,
> On 06/29/2016 11:43 AM, Mat Martineau wrote:
>>> +    ret = verify_signature(key, &sig);
>>> +    if (!ret) {
>>> +        req->dst_len = sizeof(digest);
>>
>> I think you fixed the BUG_ON() problem but there's still an issue with
>> the handling of the digest. Check the use of sig->digest in
>> public_key_verify_signature(), it's an input not an output. Right now it
>> looks like 20 uninitialized bytes are compared with the computed digest
>> within verify_signature, and then the unintialized bytes are copied to
>> req->dst here.
>>
>> With some modifications to public_key_verify_signature you could get the
>> digest you need, but I'm not sure if verification with a hardware key
>> (like a key in a TPM) can or can not provide the digest needed. Maybe
>> this is why the verify_signature hook in struct asymmetric_key_subtype
>> is optional.
>>
>>> +        scatterwalk_map_and_copy(digest, req->dst, 0, req->dst_len, 1);
>>> +    }
>
> So it looks like the only thing that we need to return to the user in
> this case is the return code. Do you agree?

The way verify_signature is implemented today, the only output is the 
return code. For verify, maybe no read is required (just sendmsg() and 
check the return code).

But this isn't the extent of the problem: verify_signature needs both the 
signature to be verified and the expected hash as inputs. How is the 
expected hash provided? Would you include it as a cmsg header?
ALG_OP_VERIFY should have consistent inputs and outputs whether the key 
was set with ALG_SET_KEY_ID or ALG_SET_KEY.


--
Mat Martineau
Intel OTC