Feature request: For OpenSSL 3.0, modify the gost implementation to become an provider module

[levitte]

With OpenSSL 3.0, there is this new interface for dealing with extension, termed "provider". This is intended to be a more flexible interface, and most importantly, both upward and downward compatible with different OpenSSL versions (starting with 3.0). The ENGINE API will eventually go away, and you might as well start working on the provider module form now.

[beldmit]

I've started to implement the provider, taking the legacy provider as an example.
md2_functions is defined via the IMPLEMENT_digest_functions macro. It seems to be not a public one.

What is a relevant way to set the implementation for the 3rd-party providers?

[levitte]

It might be better to look at a simpler example: https://github.com/provider-corner/vigenere

(because openssl's three providers are quite entangled, they are not a good starting example)

[levitte]

BTW, I'm working that provider up to be a bit more complete, including individual error reporting, which was more difficult than I had anticipated.

[levitte]

Done, please have a look and tell me if there's anything that needs clarification (actually, feel free to raise issues there)

[beldmit]

I've converted the digests to a provider form but found some problems.

When I try to define micalg as gettable param, I get an error:

/home/beldmit/crypto/engine/gost_md2012.c: In function ‘STREEBOG_gettable_params’:
/home/beldmit/crypto/openssl/include/openssl/params.h:61:49: error: lvalue required as unary ‘&’ operand
     OSSL_PARAM_DEFN((key), OSSL_PARAM_UTF8_PTR, &(addr), sz)
                                                 ^
/home/beldmit/crypto/openssl/include/openssl/params.h:25:23: note: in definition of macro ‘OSSL_PARAM_DEFN’
     { (key), (type), (addr), (sz), 0 }
                       ^~~~
/home/beldmit/crypto/engine/gost_md2012.c:94:9: note: in expansion of macro ‘OSSL_PARAM_utf8_ptr’
         OSSL_PARAM_utf8_ptr("micalg", NULL, strlen(micalg_256)+1),
         ^~~~~~~~~~~~~~~~~~~

So what is the pattern of using the OSSL_PARAM_utf8_ptr here?
(

engine/gost_md2012.c

Line 94 in 011e15b

/* OSSL_PARAM_utf8_ptr("micalg", NULL, strlen(micalg_256)+1), */

)

I tried to copy the pattern from fipsprov.c, but did no succeed.
See also openssl/openssl#10399

[vt-alt]

So parameter will be NULL of size strlen(micalg_256)+1 which is strange.

[beldmit]

I've tried many variants but none of them made the compiler happy. It means I do not get the point.

[levitte]

@beldmit, you need to update your openssl 3.0-dev installation, this is an issue that was fixed quite a while ago, for exactly this sort of reason (ever since we started using the OSSL_PARAM array as a descriptor).

Here's what that macro looks like right now:

https://github.com/openssl/openssl/blob/master/include/openssl/params.h#L56

[beldmit]

Hmmm. The macro looks exactly as you say, and the copy is up-to-date. Could it be a compiler issue?

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 8.3.0-6' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 8.3.0 (Debian 8.3.0-6) 

[levitte]

Are you sure you're looking at the right one? This is very specific:

/home/beldmit/crypto/openssl/include/openssl/params.h:61:49: error: lvalue required as unary ‘&’ operand
     OSSL_PARAM_DEFN((key), OSSL_PARAM_UTF8_PTR, &(addr), sz)
                                                 ^

If you look at the line I pointed at, the & is gone, so it is not exactly the same.

[beldmit]

Sure. The parameter was described as OSSL_PARAM_utf8_ptr, not OSSL_PARAM_utf8_string, and you've provided the link to the line defining the OSSL_PARAM_utf8_string macro.

But after I changed it to OSSL_PARAM_utf8_string, I got an error

/home/beldmit/crypto/engine/gost_md2012.c: In function ‘STREEBOG_gettable_params’:
/home/beldmit/crypto/openssl/include/openssl/params.h:25:22: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
     { (key), (type), (addr), (sz), 0 }
                      ^
/home/beldmit/crypto/openssl/include/openssl/params.h:56:5: note: in expansion of macro ‘OSSL_PARAM_DEFN’
     OSSL_PARAM_DEFN((key), OSSL_PARAM_UTF8_STRING, (addr), sz)
     ^~~~~~~~~~~~~~~
/home/beldmit/crypto/engine/gost_md2012.c:94:9: note: in expansion of macro ‘OSSL_PARAM_utf8_string’
         OSSL_PARAM_utf8_string("micalg", micalg_256, strlen(micalg_256)+1),
         ^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Casting the string to char * eliminates this error, but it seems wrong for this case:

        OSSL_PARAM_utf8_string("micalg", (char *)micalg_256, strlen(micalg_256)+1),

So it means I do not understand:

1. When I should use utf8_string and when I should use utf8_ptr
2. What shell I use when I want to use the constant string. I can just add -Wno-discarded-qualifiers but it seems a wrong solution.

[levitte]

Oh silly me, I didn't notice that you used the _PTR form.
To your question, the _PTR variant is intended to be used when the pointer itself is the data, rather than what it's pointing at. That doesn't seem important in this case.

Either way, if we're still talking about a gettable, this is what I would specify:

    OSSL_PARAM_utf8_string("micalg", NULL, 0),

(the 0 can be replaced with any number if you want to specify a maximum size)