hw/virtio-crypto: add akcipher keylen validation

[PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

Posted by helei 1 day, 13 hours ago

The virtio-crypto spec does not dictate a maximum length limit for 
asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
the linux kernels's internal limit for akcipher keys (see 
keyctl framework and the add_key syscall).
Maybe we should update the virtio-spec and add a max_akcipher_key_len
field for virtio crypto devices.

helei (1):
  hw/virtio-crypto: enforce max akcipher key length

 hw/virtio/virtio-crypto.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

Posted by Michael S. Tsirkin 1 day, 12 hours ago

On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
> The virtio-crypto spec does not dictate a maximum length limit for 
> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
> the linux kernels's internal limit for akcipher keys (see 
> keyctl framework and the add_key syscall).

We have max_size - doesn't that apply?
backends/cryptodev-builtin.c actually sets it:
backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;

> Maybe we should update the virtio-spec and add a max_akcipher_key_len
> field for virtio crypto devices.

maybe

> 
> helei (1):
>   hw/virtio-crypto: enforce max akcipher key length
> 
>  hw/virtio/virtio-crypto.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> -- 
> 2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

Posted by helei 21 hours ago

On 6/6/26 16:37, Michael S. Tsirkin wrote:
> On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
>> The virtio-crypto spec does not dictate a maximum length limit for
>> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
>> the linux kernels's internal limit for akcipher keys (see
>> keyctl framework and the add_key syscall).
> We have max_size - doesn't that apply?
> backends/cryptodev-builtin.c actually sets it:
> backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
> backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;

Thanks for your review!  I have verified via testing that all processing 
requests in the dataq are strictly

bounded by max_size, but session creation requests in the ctrlq are not.

>
>> Maybe we should update the virtio-spec and add a max_akcipher_key_len
>> field for virtio crypto devices.
> maybe
>
>> helei (1):
>>    hw/virtio-crypto: enforce max akcipher key length
>>
>>   hw/virtio/virtio-crypto.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> -- 
>> 2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

Posted by Michael S. Tsirkin 18 hours ago

On Sun, Jun 07, 2026 at 08:15:19AM +0800, helei wrote:
> 
> On 6/6/26 16:37, Michael S. Tsirkin wrote:
> > On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
> > > The virtio-crypto spec does not dictate a maximum length limit for
> > > asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
> > > the linux kernels's internal limit for akcipher keys (see
> > > keyctl framework and the add_key syscall).
> > We have max_size - doesn't that apply?
> > backends/cryptodev-builtin.c actually sets it:
> > backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
> > backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;
> 
> Thanks for your review!  I have verified via testing that all processing
> requests in the dataq are strictly
> 
> bounded by max_size, but session creation requests in the ctrlq are not.



well if we read the spec it's vague

  max_size is defined as "the maximum size of the variable-length parameters of data operation of
  each crypto request's content." and
  The driver SHOULD read max_size to discover the maximum size of the variable-length parameters of
  data operation of the crypto request's content


  so data operation.

  however:

  "The device MUST set max_size to show the
  maximum size of crypto request the device supports".

  seems to cover all requests?



btw vhost user sets max_size to max u64 - is that sane?

  

> > 
> > > Maybe we should update the virtio-spec and add a max_akcipher_key_len
> > > field for virtio crypto devices.
> > maybe
> > 
> > > helei (1):
> > >    hw/virtio-crypto: enforce max akcipher key length
> > > 
> > >   hw/virtio/virtio-crypto.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > -- 
> > > 2.43.0