Re: [PATCH v2] crypto: Introduce SM4 symmetric cipher algorithm

[Yong Huang]

I'll try to understand the comment, if i misunderstood, please point out.

On Wed, Nov 29, 2023 at 12:20 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

On Tue, Nov 28, 2023 at 04:57:20PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Hyman,
>
> On 28/11/23 16:24, Hyman Huang wrote:
> > Introduce the SM4 cipher algorithms (OSCCA GB/T 32907-2016).
> >
> > SM4 (GBT.32907-2016) is a cryptographic standard issued by the
> > Organization of State Commercial Administration of China (OSCCA)
> > as an authorized cryptographic algorithms for the use within China.
> >
> > Use the crypto-sm4 meson build option for enabling this feature.
> >
> > Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> > ---
> >   crypto/block-luks.c             | 11 ++++++++
> >   crypto/cipher-gcrypt.c.inc      |  8 ++++++
> >   crypto/cipher-nettle.c.inc      | 49 +++++++++++++++++++++++++++++++++
> >   crypto/cipher.c                 |  6 ++++
> >   meson.build                     | 23 ++++++++++++++++
> >   meson_options.txt               |  2 ++
> >   qapi/crypto.json                |  5 +++-
> >   scripts/meson-buildoptions.sh   |  3 ++
> >   tests/unit/test-crypto-cipher.c | 13 +++++++++
> >   9 files changed, 119 insertions(+), 1 deletion(-)
>
>
> > diff --git a/meson.build b/meson.build
> > index ec01f8b138..256d3257bb 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1480,6 +1480,7 @@ endif
> >   gcrypt = not_found
> >   nettle = not_found
> >   hogweed = not_found
> > +crypto_sm4 = not_found
> >   xts = 'none'
> >   if get_option('nettle').enabled() and get_option('gcrypt').enabled()
> > @@ -1514,6 +1515,26 @@ if not gnutls_crypto.found()
> >         xts = 'private'
> >       endif
> >     endif
> > +  if get_option('crypto_sm4').enabled()
>
> We want to detect it by default (not only when explicitly enabled) ...
>
> > +    if get_option('gcrypt').enabled()
> > +      # SM4 ALG is available in libgcrypt >= 1.9
> > +      crypto_sm4 = dependency('libgcrypt', version: '>=1.9',
> > +                              method: 'config-tool',
> > +                              required: get_option('gcrypt'))
> > +      # SM4 ALG static compilation
> > +      if crypto_sm4.found() and get_option('prefer_static')
> > +        crypto_sm4 = declare_dependency(dependencies: [
> > +          crypto_sm4,
> > +          cc.find_library('gpg-error', required: true)],
> > +          version: crypto_sm4.version())
> > +      endif
> > +    else
> > +      # SM4 ALG is available in nettle >= 3.9
> > +      crypto_sm4 = dependency('nettle', version: '>=3.9',
> > +                              method: 'pkg-config',
> > +                              required: get_option('nettle'))
> > +    endif
>
> ... and if it was forced with --enable-crypto_sm4 AND not found,
> display an error.
>
> IIUC your config you try to find the best effort implementation then
> if not found, keep going silently.

Yes, ignore the get_option() calls, and instead look at .found()
in the library we just detected 

ie

  if nettle.found()
      ....check sm4 in nettle
  endif

  if gcrypt.found()
      ....check sm4 in crypt
  endif

To detect if sm4 is supported, there may be two methods:

One is to specify the version explicitly(ligcrypt >=1.9,nettle >= 3.9) 

as in patch

Another is to use the cc.link for a test. eg:

+      crypto_sm4 = gcrypt

+      if gcrypt.found() and not cc.links('''
+        #include <gcrypt.h>
+        void main(void) {
+          gcry_cipher_hd_t handler;
+          gcry_cipher_open(&handler, GCRY_CIPHER_SM4, GCRY_CIPHER_MODE_ECB, 0);
+        }''', dependencies: gcrypt)
+        crypto_sm4 = not_found
+      endif

Is the latter a better choice?

 

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 :|

--

Best regards