1. Patchew
  2. QEMU
  3. [PATCH v3 0/6] Support akcipher for virtio-crypto
  4. View patch

[PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi

zhenwei pi posted 6 patches 3 years, 10 months ago
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi
Posted by zhenwei pi 3 years, 10 months ago 
From: Lei He <helei.sig11@bytedance.com>

Introduce akcipher types, also include RSA & ECDSA related types.

Signed-off-by: Lei He <helei.sig11@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 qapi/crypto.json | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/qapi/crypto.json b/qapi/crypto.json
index 1ec54c15ca..d44c38e3b1 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -540,3 +540,89 @@
   'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
             '*sanity-check': 'bool',
             '*passwordid': 'str' } }
+##
+# @QCryptoAkcipherAlgorithm:
+#
+# The supported algorithms for asymmetric encryption ciphers
+#
+# @rsa: RSA algorithm
+# @ecdsa: ECDSA algorithm
+#
+# Since: 7.0
+##
+{ 'enum': 'QCryptoAkcipherAlgorithm',
+  'prefix': 'QCRYPTO_AKCIPHER_ALG',
+  'data': ['rsa', 'ecdsa']}
+
+##
+# @QCryptoAkcipherKeyType:
+#
+# The type of asymmetric keys.
+#
+# Since: 7.0
+##
+{ 'enum': 'QCryptoAkcipherKeyType',
+  'prefix': 'QCRYPTO_AKCIPHER_KEY_TYPE',
+  'data': ['public', 'private']}
+
+##
+# @QCryptoRsaHashAlgorithm:
+#
+# The hash algorithm for RSA pkcs1 padding algothrim
+#
+# Since: 7.0
+##
+{ 'enum': 'QCryptoRsaHashAlgorithm',
+  'prefix': 'QCRYPTO_RSA_HASH_ALG',
+  'data': [ 'md2', 'md3', 'md4', 'md5', 'sha1', 'sha256', 'sha384', 'sha512', 'sha224' ]}
+
+##
+# @QCryptoRsaPaddingAlgorithm:
+#
+# The padding algorithm for RSA.
+#
+# @raw: no padding used
+# @pkcs1: pkcs1#v1.5
+#
+# Since: 7.0
+##
+{ 'enum': 'QCryptoRsaPaddingAlgorithm',
+  'prefix': 'QCRYPTO_RSA_PADDING_ALG',
+  'data': ['raw', 'pkcs1']}
+
+##
+# @QCryptoCurveId:
+#
+# The well-known curves, referenced from https://csrc.nist.gov/csrc/media/publications/fips/186/3/archive/2009-06-25/documents/fips_186-3.pdf
+#
+# Since: 7.0
+##
+{ 'enum': 'QCryptoCurveId',
+  'prefix': 'QCRYPTO_CURVE_ID',
+  'data': ['nist-p192', 'nist-p224', 'nist-p256', 'nist-p384', 'nist-p521']}
+
+##
+# @QCryptoRsaOptions:
+#
+# Specific parameters for RSA algorithm.
+#
+# @hash-algo: QCryptoRsaHashAlgorithm
+# @padding-algo: QCryptoRsaPaddingAlgorithm
+#
+# Since: 7.0
+##
+{ 'struct': 'QCryptoRsaOptions',
+  'data': { 'hash-algo':'QCryptoRsaHashAlgorithm',
+            'padding-algo': 'QCryptoRsaPaddingAlgorithm'}}
+
+##
+# @QCryptoEcdsaOptions:
+#
+# Specific parameter for ECDSA algorithm.
+#
+# @curve-id: QCryptoCurveId
+#
+# Since: 7.0
+##
+{ 'struct': 'QCryptoEcdsaOptions',
+  'data': { 'curve-id': 'QCryptoCurveId' }}
-- 
2.25.1
Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi
Posted by Daniel P. Berrangé 3 years, 10 months ago 
On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He <helei.sig11@bytedance.com>
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He <helei.sig11@bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  qapi/crypto.json | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>    'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>              '*sanity-check': 'bool',
>              '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:
> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}

What were your intentions wrt  ecdsa - the nettle impl in this patch
series doesn't appear to actually support ecdsa. Are you intending to
add this in later versions of this patch series, or do it as separate
work at a later date ?


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi
Posted by Daniel P. Berrangé 3 years, 10 months ago 
On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He <helei.sig11@bytedance.com>
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He <helei.sig11@bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  qapi/crypto.json | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>    'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>              '*sanity-check': 'bool',
>              '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:

Should be named  QCryptoAkCipherAlgorithm

> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}
> +
> +##
> +# @QCryptoAkcipherKeyType:

Should be named  QCryptoAkCipherKeyType

> +#
> +# The type of asymmetric keys.
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherKeyType',
> +  'prefix': 'QCRYPTO_AKCIPHER_KEY_TYPE',
> +  'data': ['public', 'private']}
> +
> +##
> +# @QCryptoRsaHashAlgorithm:
> +#
> +# The hash algorithm for RSA pkcs1 padding algothrim
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaHashAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_HASH_ALG',
> +  'data': [ 'md2', 'md3', 'md4', 'md5', 'sha1', 'sha256', 'sha384', 'sha512', 'sha224' ]}

We already have QCryptoHashAlgorithm and I don't see the
benefit in duplicating it here.

We don't have md2, md3, and md4 in QCryptoHashAlgorithm, but
that doesn't look like a real negative as I can't imagine
those should be used today.

> +##
> +# @QCryptoRsaPaddingAlgorithm:
> +#
> +# The padding algorithm for RSA.
> +#
> +# @raw: no padding used
> +# @pkcs1: pkcs1#v1.5
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaPaddingAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_PADDING_ALG',
> +  'data': ['raw', 'pkcs1']}
> +
> +##
> +# @QCryptoCurveId:

Should be named  QCryptoCurveID

> +#
> +# The well-known curves, referenced from https://csrc.nist.gov/csrc/media/publications/fips/186/3/archive/2009-06-25/documents/fips_186-3.pdf
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoCurveId',
> +  'prefix': 'QCRYPTO_CURVE_ID',
> +  'data': ['nist-p192', 'nist-p224', 'nist-p256', 'nist-p384', 'nist-p521']}


> +
> +##
> +# @QCryptoRsaOptions:

This should be named  QCryptoAkCipherOptionsRSA

> +#
> +# Specific parameters for RSA algorithm.
> +#
> +# @hash-algo: QCryptoRsaHashAlgorithm
> +# @padding-algo: QCryptoRsaPaddingAlgorithm
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoRsaOptions',
> +  'data': { 'hash-algo':'QCryptoRsaHashAlgorithm',
> +            'padding-algo': 'QCryptoRsaPaddingAlgorithm'}}

Our naming convention is  'XXX-alg' rather than 'XXX-algo'.

> +
> +##
> +# @QCryptoEcdsaOptions:

This should be named  QCryptoAkCipherOptionsECDSA

> +#
> +# Specific parameter for ECDSA algorithm.
> +#
> +# @curve-id: QCryptoCurveId
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoEcdsaOptions',
> +  'data': { 'curve-id': 'QCryptoCurveId' }}

Having these two structs standalone looks wrong to me. I suspect that
callers will need to be able to conditionally pass in either one, and
so require the API to use a discriminated union

  { 'union': 'QCryptoAkCipherOptions'
    'base': { 'algorithm': 'QCryptoAkCipherAlgorithm' },
    'discriminator': 'algorithm',
    'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' ,
              'ecdsa': 'QCryptoAkCipherOptionsECDSA' } }


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.