linux-kernel - Re: [PATCH v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

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]

Date:	Tue, 14 Jun 2016 09:42:34 +0200
From:	Andrew Zaborowski <balrogg@...glemail.com>
To:	Stephan Mueller <smueller@...onox.de>
Cc:	Mat Martineau <mathew.j.martineau@...ux.intel.com>,
	Tadeusz Struk <tadeusz.struk@...el.com>,
	David Howells <dhowells@...hat.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	linux-api@...r.kernel.org, marcel@...tmann.org,
	linux-kernel@...r.kernel.org, keyrings@...r.kernel.org,
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
	David Woodhouse <dwmw2@...radead.org>, davem@...emloft.net
Subject: Re: [PATCH v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

Hi Stephan,

On 14 June 2016 at 07:12, Stephan Mueller <smueller@...onox.de> wrote:
> Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:
>> On 8 June 2016 at 21:14, Mat Martineau
>>
>> <mathew.j.martineau@...ux.intel.com> wrote:
>> > On Wed, 8 Jun 2016, Stephan Mueller wrote:
>> >> What is your concern?
>> >
>> > Userspace must allocate larger buffers than it knows are necessary for
>> > expected results.
>> >
>> > It looks like the software rsa implementation handles shorter output
>> > buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
>> > too small), however I see at least one hardware rsa driver that requires
>> > the output buffer to be the maximum size. But this inconsistency might be
>> > best addressed within the software cipher or drivers rather than in
>> > recvmsg.
>> Should the hardware drivers fix this instead?  I've looked at the qat
>> and caam drivers, they both require the destination buffer size to be
>> the key size and in both cases there would be no penalty for dropping
>> this requirement as far as I see.  Both do a memmove if the result
>> ends up being shorter than key size.  In case the caller knows it is
>> expecting a specific output size, the driver will have to use a self
>> allocated buffer + a memcpy in those same cases where it would later
>> use memmove instead.  Alternatively the sg passed to dma_map_sg can be
>> prepended with a dummy segment the right size to save the memcpy.
>>
>> akcipher.h only says:
>> @dst_len: Size of the output buffer. It needs to be at least as big as
>> the expected result depending on the operation
>>
>> Note that for random input data the memmove will be done about 1 in
>> 256 times but with PKCS#1 padding the signature always has a leading
>> zero.
>>
>> Requiring buffers bigger than needed makes the added work of dropping
>> the zero bytes from the sglist and potentially re-adding them in the
>> client difficult to justify.  RSA doing this sets a precedent for a
>> future pkcs1pad (or other algorithm) implementation to do the same
>> thing and a portable client having to always know the key size and use
>> key-sized buffers.
>
> I think we have agreed on dropping the length enforcement at the interface
> level.

Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users).  For RSA, the qat
implementation will fail while the software implementation won't.  For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.

Best regards