Adapt to OpenSSL 1.1.0

[ppisar]

OpenSSL 1.1.0 hid structure internals and provided methods for reading
and writing them. This patch adapts to the changes so that it's
possible to build it against the new as well as as old OpenSSL
library.

Because the new OpenSSL does not support setting each prime number
separately, this patch fakes somes otherwise undefined values. This
looks ugly but this cannot be done better.

I recommend to add new Perl subroutines for setting the prime numbers in a
bulk as new OpenSSL does. These will be a strightforward binding with
less code and higher performance.

CPAN RT#118346

[kmx]

I am getting this failure on MS Windows + openssl-1.1.0b:

t/10-selftest.t ........ (null) at t/10-selftest.t line 12.
t/10-selftest.t ........ Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 36/36 subtests
t/90-openssl-compat.t .. (null) at t/90-openssl-compat.t line 28.
t/90-openssl-compat.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 84/84 subtests

I will try to investigate it a bit more.

[kmx]

What do you think about dcd4020 ?

[ppisar]

On Tue, Nov 01, 2016 at 08:10:06AM -0700, kmx wrote:

What do you think about
dcd4020
?

It does not work for me (openssl-1.1.0b-3.fc26.x86_64 on Fedora Linux
distribution). The tests fail, e.g.:

$ perl -Iblib/{lib,arch} t/10-selftest.t
1..36

Running under perl version 5.024000 for linux

Current time local: Tue Nov 1 16:49:13 2016

Current time GMT: Tue Nov 1 15:49:13 2016

Using Test.pm version 1.28_01

q not prime at t/10-selftest.t line 12.

That could be caused by Fedora's OpenSSL patches applied on top. I will have
to check more what's going on.

[kmx]

I have found https://www.openssl.org/news/cl110.txt

DSA_generate_parameters_ex, if the provided seed is too short, return an error

And this https://www.openssl.org/docs/man1.1.0/crypto/DSA_generate_parameters.html

If seed_len is less than the length of q, an error is returned.

so the added test for seed_len is not completely correct

[kmx]

According: https://www.openssl.org/docs/man1.1.0/crypto/DSA_generate_parameters_ex.html

bits is the length of the prime p to be generated. For lengths under 2048 bits, 
the length of q is 160 bits; for lengths greater than or equal to 2048 bits, the 
length of q is set to 256 bits.

If seed is NULL, the primes will be generated at random. If seed_len is less 
than the length of q, an error is returned.

so I have changed the test to

        if (seed_len > 0) {
          if (bits < 2048 && seed_len < 20)
            croak("seed_len %d is too small, required min. 20", seed_len);
          if (bits >= 2048 && seed_len < 32)
            croak("seed_len %d is too small, required min. 32", seed_len);
        }

But I do not understand why DSA_generate_parameters does not return a relevant error code (it only retuns NULL as a retrun value).

[ppisar]

Maybe you should use DSA_generate_parameters_ex() with new OpenSSL. The DSA_generate_parameters() is deprecated there. Nonetheless the issue on my system has different cause because it returns error from OpenSSL, not a croak from the length check in DSA.xs. Even with you latest commit.

[ppisar]

On Tue, Nov 01, 2016 at 11:45:13AM -0700, kmx wrote:

But I do not understand why DSA_generate_parameters does not return
a relevant error code (it only retuns NULL as a retrun value).

Because this is what dsa_builtin_paramgen() from OpenSSL-1.1.0b source does.
There are cases when OpenSSL only returns without setting error code. E.g.
when the seed length is smaller than size of the generated prime.

[ppisar]

On Wed, Nov 02, 2016 at 09:57:32AM +0100, Petr Pisar wrote:

Maybe you should use DSA_generate_parameters_ex() with new OpenSSL. The
DSA_generate_parameters() is deprecated there.

This will not help. DSA_generate_parameters() is only a wrapper around
DSA_generate_parameters_ex().

Nonetheless the issue on my system has different cause because it returns
error from OpenSSL, not a croak from the length check in DSA.xs. Even with
you latest commit.

The issue on my system is very probably caused by FIPS patches that Fedora
applies. It uses dsa_builtin_paramgen2() instead of dsa_builtin_paramgen().

Moreover the perl code fails from with "fooo", "fo" and even with "abc". It
passes only with "foo". So it's not only a matter of length, but it also
depends on exact content of the seed.

I grepped OpenSSL-1.1.0b sources for DSA_generate_parameters_ex() calls and
none of them exhibits DSA_generate_parameters_ex() call with non-NULL seed.
It looks like the code path was never properly tested and we found a bug.

I will ask OpenSSL maintainer in Fedora.

[kmx]

With openssl-1.1.0b built for MS Win/64bit without any patches I see the failure also with "foo".

[ppisar]

On Wed, Nov 02, 2016 at 08:38:52AM -0700:

With openssl-1.1.0b built for MS Win/64bit without any patches I see the
failure also with "foo".

Fedora maintainer responded
https://bugzilla.redhat.com/show_bug.cgi?id=1391144 that it is a bug in
Fedora's patch and he will fix.

The reason why old OpenSSL worked for any seed while the new one fails is that
some seeds cannot produce a prime (in a reasonable number of iterations) and
new OpenSSL fails in that case while the old one internally retried the
generation from a random seed.

Therefore do not bother with the "foo" case.

Once Fedora's OpenSSL gets fixed, I will try your code again. Maybe it will be
necessary to lenghten the seed more as the Fedora's FIPS patch emplyoes SHA2
which requires longer seed for cryptographically strong PRNG application.

-- Petr

[kmx]

Do you think we should put the test:

if (seed_len > 0) {
  ...
}

inside #if block like this:

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
...
#endif

or keep the seed_len test also for older openssl?

[ppisar]

I would remove the checks for seed length completely and let it on OpenSSL. (Otherwise you had to follow OpenSSL for any internal changes to match the range checks.) If you care about silent OpenSSL failures without an error code, I would only rephrase the croak message after DSA_generate_parameters() to indicate that a possible reason is invalid seed length with reference to the OpenSSL manual page. But if you want to cover this case in the Perl binding, I would keep the check only for the new OpenSSL as you suggested.

…

-- Petr

[ppisar]

On Thu, Nov 03, 2016 at 02:33:10PM +0100, Petr Pisar wrote:

Once Fedora's OpenSSL gets fixed, I will try your code again.

Fedora OpenSSL maintainer described the fix like this:

use a random seed if the supplied one did not generate valid
parameters in dsa_builtin_paramgen2()

Therefore Fedora's OpenSSL does not fail with any seed now. This of course
does not affect the vanilla OpenSSL.

[kmx]

I have released 0.18 with slightly modified handling of NULL retval from DSA_generate_parameters

see d370b5a