gost-engine / engine

A reference implementation of the Russian GOST crypto algorithms for OpenSSL
Apache License 2.0
375 stars 170 forks source link

test_tls hangs and test_engine fails with OpenSSL-3.0 master #230

Closed mouse07410 closed 4 years ago

mouse07410 commented 4 years ago

MacOS 10.15.4, Xcode-10.15.4, current master of OpenSSL, current master of this engine.

Build succeeds. All tests for OpenSSL-1.1.1 pass, test_tls takes less than 4 seconds.

Tests for OpenSSL-3.0 hang on the test_tls:

Running tests...
/opt/local/bin/ctest --force-new-ctest-process 
Test project /Users/ur20980/src/grasshopper-engine/build
      Start  1: digest
 1/10 Test  #1: digest ...........................   Passed    0.53 sec
      Start  2: curves
 2/10 Test  #2: curves ...........................   Passed    0.18 sec
      Start  3: parameters
 3/10 Test  #3: parameters .......................   Passed    0.20 sec
      Start  4: sign/verify
 4/10 Test  #4: sign/verify ......................   Passed    0.22 sec
      Start  5: TLS

And you probably want to see this:

test 5
      Start  5: TLS

5: Test command: /Users/ur20980/src/grasshopper-engine/build/bin/test_tls
5: Test timeout computed to be: 10000000
5: C0:1D:85:0C:01:00:00:00:error:SSL routines:ssl_set_cert:unknown certificate type:ssl/ssl_rsa.c:343:
5: /Users/ur20980/src/grasshopper-engine/test_tls.c:76: OpenSSL internal error: SSL_CTX_use_certificate(ctx, cert)
5: Test rsa

Also, please feel free to guide me how to try other tests (6-10).

Update

First, it seems that the majority of the other tests are OK.

It appears that the reason for the test_tls to fail is this strange line, and commenting it out is a workaround:

diff --git a/test_tls.c b/test_tls.c
index d137602..2d369c3 100644
--- a/test_tls.c
+++ b/test_tls.c
@@ -303,7 +303,7 @@ int main(int argc, char **argv)
     T(ENGINE_init(eng));
     T(ENGINE_set_default(eng, ENGINE_METHOD_ALL));

-    ret |= test("rsa", NULL);
+    //ret |= test("rsa", NULL);
     ret |= test("gost2012_256", "A");
     ret |= test("gost2012_256", "B");
     ret |= test("gost2012_256", "C");

It appears that the (unnecessary in my humble opinion) name conversion from "Grasshopper" to "Kuznyechik" (amazing spelling, BTW) was incomplete, causing the engine test to fail:

10: # Failed test 'load engine without any config'
10: # at 00-engine.t line 51.
10: # +---------------------------------+----+---------------------------------+
10: # | GOT                             | OP | CHECK                           |
10: # +---------------------------------+----+---------------------------------+
10: # | (gost) Reference implementation | eq | (gost) Reference implementation |
10: # |  of GOST engine\n               |    |  of GOST engine\n               |
10: # |  [gost89, gost89-cnt, gost89-cn |    |  [gost89, gost89-cnt, gost89-cn |
10: # | t-12, gost89-cbc, kuznyechik-ec |    | t-12, gost89-cbc, grasshopper-e |
10: # | b, kuznyechik-cbc, kuznyechik-c |    | cb, grasshopper-cbc, grasshoppe |
10: # | fb, kuznyechik-ofb, kuznyechik- |    | r-cfb, grasshopper-ofb, grassho |
10: # | ctr, magma-cbc, magma-ctr, kuzn |    | pper-ctr, magma-cbc, magma-ctr, |
10: # | yechik-ctr-acpkm, md_gost94, go |    |  id-tc26-cipher-gostr3412-2015- |
10: # | st-mac, md_gost12_256, md_gost1 |    | kuznyechik-ctracpkm, md_gost94, |
10: # | 2_512, gost-mac-12, magma-mac,  |    |  gost-mac, md_gost12_256, md_go |
10: # | kuznyechik-mac, kuznyechik-ctr- |    | st12_512, gost-mac-12, magma-ma |
10: # | acpkm-omac, gost2001, gost-mac, |    | c, grasshopper-mac, id-tc26-cip |
10: # |  gost2012_256, gost2012_512, go |    | her-gostr3412-2015-kuznyechik-c |
10: # | st-mac-12, magma-mac, kuznyechi |    | tracpkm-omac, gost2001, gost-ma |
10: # | k-mac, magma-ctr-acpkm-omac, ku |    | c, gost2012_256, gost2012_512,  |
10: # | znyechik-ctr-acpkm-omac]\n      |    | gost-mac-12, magma-mac, grassho |
10: # |                                 |    | pper-mac, id-tc26-cipher-gostr3 |
10: # |                                 |    | 412-2015-magma-ctracpkm-omac, i |
10: # |                                 |    | d-tc26-cipher-gostr3412-2015-ku |
10: # |                                 |    | znyechik-ctracpkm-omac]\n       |
10: # +---------------------------------+----+---------------------------------+
10: 
10: # Failed test 'load engine with config'
10: # at 00-engine.t line 74.
10: # +---------------------------------+----+---------------------------------+
10: # | GOT                             | OP | CHECK                           |
10: # +---------------------------------+----+---------------------------------+
10: # | (gost) Reference implementation | eq | (gost) Reference implementation |
10: # |  of GOST engine\n               |    |  of GOST engine\n               |
10: # |  [gost89, gost89-cnt, gost89-cn |    |  [gost89, gost89-cnt, gost89-cn |
10: # | t-12, gost89-cbc, kuznyechik-ec |    | t-12, gost89-cbc, grasshopper-e |
10: # | b, kuznyechik-cbc, kuznyechik-c |    | cb, grasshopper-cbc, grasshoppe |
10: # | fb, kuznyechik-ofb, kuznyechik- |    | r-cfb, grasshopper-ofb, grassho |
10: # | ctr, magma-cbc, magma-ctr, kuzn |    | pper-ctr, magma-cbc, magma-ctr, |
10: # | yechik-ctr-acpkm, md_gost94, go |    |  id-tc26-cipher-gostr3412-2015- |
10: # | st-mac, md_gost12_256, md_gost1 |    | kuznyechik-ctracpkm, md_gost94, |
10: # | 2_512, gost-mac-12, magma-mac,  |    |  gost-mac, md_gost12_256, md_go |
10: # | kuznyechik-mac, kuznyechik-ctr- |    | st12_512, gost-mac-12, magma-ma |
10: # | acpkm-omac, gost2001, gost-mac, |    | c, grasshopper-mac, id-tc26-cip |
10: # |  gost2012_256, gost2012_512, go |    | her-gostr3412-2015-kuznyechik-c |
10: # | st-mac-12, magma-mac, kuznyechi |    | tracpkm-omac, gost2001, gost-ma |
10: # | k-mac, magma-ctr-acpkm-omac, ku |    | c, gost2012_256, gost2012_512,  |
10: # | znyechik-ctr-acpkm-omac]\n      |    | gost-mac-12, magma-mac, grassho |
10: # |                                 |    | pper-mac, id-tc26-cipher-gostr3 |
10: # |                                 |    | 412-2015-magma-ctracpkm-omac, i |
10: # |                                 |    | d-tc26-cipher-gostr3412-2015-ku |
10: # |                                 |    | znyechik-ctracpkm-omac]\n       |
10: # +---------------------------------+----+---------------------------------+
. . . . .
10: # DYLD_LIBRARY_PATH=/Users/ur20980/openssl-3/lib
10: # /Users/ur20980/openssl-3/bin/openssl
10: # OpenSSL 3.0.0-alpha2-dev  (Library: OpenSSL 3.0.0-alpha2-dev )
10: not ok 1 - load engine without any config
10: ok 2 - compute digest without config
10: not ok 3 - load engine with config
10: ok 4 - compute digest with config without explicit engine param
10: ok 5 - compute digest with both config and explicit engine param
10: ok 6 - display GOST2001-GOST89-GOST89 cipher
10: ok 7 - display GOST2012-GOST8912-GOST8912 cipher
10: Dubious, test returned 2 (wstat 512, 0x200)
10: Failed 2/7 subtests 
. . . . .

Here are the output of make test and make test ARGV='-V' correspondingly: test-3-out.txt test-3-long-out.txt

beldmit commented 4 years ago

The rsa test failure seems to be an openssl regression. @levitte, could you please take a look?

vt-alt commented 4 years ago

I agree that test needs to be reworked to not hang, But the primary cause is a bug in openssl. AFAIK, this one https://github.com/openssl/openssl/issues/11549

mouse07410 commented 4 years ago

The https://github.com/openssl/openssl/issues/11549 issue is closed. The problem persists, here at least.

Also, see the failures of test_engine with OpenSSL-3.0.

vt-alt commented 4 years ago

Maybe it's because the fix is not committed into OpenSSL_1_1_1-stable which we run Travis tests against. Or you tried on the master?

vt-alt commented 4 years ago

Ah you said OpenSSL-3.0 master excuse my unattentiveness.

vt-alt commented 4 years ago

I will try to rework test_tls to(night|day). Thanks,

vt-alt commented 4 years ago

Reworked test_tls in PR #231 so it does not hang. What is remaining is to fix that error:

00:57:50:60:01:7F:00:00:error:SSL routines:ssl_set_cert:unknown certificate type:ssl/ssl_rsa.c:343:
/home/abz/src/gost-engine/test_tls.c:76: OpenSSL internal error: SSL_CTX_use_certificate(ctx, cert)
mouse07410 commented 4 years ago

What is remaining is to fix that error...

Regarding test_tls.c - probably.

But there are problems in test_engine.c (or 00-engine.t) related to the half-done name conversion.

I must add that I don't know how @beldmit is planning to have both the old name (which appears required for OpenSSL-1.1.1) and the new one (which he seems to want for OpenSSL-3.0), and on top of that - take care of it in one test.

beldmit commented 4 years ago

Well, my plans are rather simple - I hope to commit the support of new GOST CMS soon and bump the minimal openssl for master.

mouse07410 commented 4 years ago

@beldmit do you mean you'll break compatibility with OpenSSL-1.1.1?

beldmit commented 4 years ago

I do. There is no way to continue support compatibility with 1.1.1 in master and add new features. New CTRL values, new NIDs, new flags — all this stuff is required.

mouse07410 commented 4 years ago

Ah... So you'll create a '1.1.1' branch? Do you plan any 1.1.1-related changes?

beldmit commented 4 years ago

There is 1.1.0 branch. It implements all the practically required functionality compatible with 1.1.0/1.1.1 - both in algorithm and protocol parts.

The new GOST standards that appeared after the 1.1.1 branch has bin stabilized are not worth supporting in the engine without any support in OpenSSL itself. I've provided support of some of these standards in the OpenSSL and in process of providing for others.

vt-alt commented 4 years ago

Why not keep as much compatibility with some #if/#ifdefs? (Like some other projects do.) Or there is so much incompatibility that this is not feasible?

beldmit commented 4 years ago

Why not keep as much compatibility with some #if/#ifdefs? (Like some other projects do.) Or there is so much incompatibility that this is not feasible?

There is so much incompatibility and will be even more after fixing up nits.

mouse07410 commented 4 years ago

OK. On the current master:

mouse07410 commented 4 years ago

There is 1.1.0 branch. It implements all the practically required functionality compatible with 1.1.0/1.1.1 - both in algorithm and protocol parts.

@beldmit there are three 1.1.0 branches:

Which of those should be used?

beldmit commented 4 years ago

openssl_1_1_0

mouse07410 commented 4 years ago

OK, tests fail on OpenSSL-1.1.1 with the branch openssl_1_1_0, and on OpenSSL-3.0 with the branch master:

OpenSSL-1.1.1 with openssl_1_1_0 branch

ossl111-build.txt test-out.txt test-long-out.txt

OpenSSL-3.0 with master branch

ossl3-build.txt test-3-out.txt test-3-long-out.txt

beldmit commented 4 years ago

OpenSSL-3.0 with master should be fixed now.

beldmit commented 4 years ago

@mouse07410 Could you please make a separate issue for 1.1.1?

mouse07410 commented 4 years ago

OpenSSL-3.0 with master should be fixed now.

Not quite:
cmake-3-out.txt make-3-out.txt

test-3-out.txt test-3-long-out.txt

Could you please make a separate issue for 1.1.1?

Sure.

beldmit commented 4 years ago

Please make sure that gost_grasshopper_cipher.c is updated.

mouse07410 commented 4 years ago

Please make sure that gost_grasshopper_cipher.c is updated.

$ git fetch --all
Fetching origin
Fetching upstream
$ git pull
Already up to date.
$ git merge upstream/master
Already up to date.
$ 

and

90% tests passed, 1 tests failed out of 10

Total Test time (real) =   2.29 sec

The following tests FAILED:
      7 - grasshopper (Child aborted)
Errors while running CTest
make: *** [test] Error 8

in the long output:

7: Decryption test from GOST R 34.13-2015 [ctr-no-acpkm] in-place
7:   d[64] =  1122334455667700ffeeddccbbaa9988 00112233445566778899aabbcceeff0a 112233445566778899aabbcceeff0a00 2233445566778899aabbcceeff0a0011
7: Test passed
7: Stream encryption test from GOST R 34.13-2015 [ctr-no-acpkm] 
 7/10 Test  #7: grasshopper ......................Child aborted***Exception:   0.01 sec
beldmit commented 4 years ago

Well. Please update it again, rebuild and run bin/test_grasshopper separately. Sorry, the bug is not reproduced both on my machine and on the Travis one.

mouse07410 commented 4 years ago

Updated. On one machine it consistently fails:

. . . . .
Decryption test from GOST R 34.13-2015 [ctr-no-acpkm] in-place
  d[64] =  1122334455667700ffeeddccbbaa9988 00112233445566778899aabbcceeff0a 112233445566778899aabbcceeff0a00 2233445566778899aabbcceeff0a0011
Test passed
Stream encryption test from GOST R 34.13-2015 [ctr-no-acpkm] 
/Users/ur20980/src/grasshopper-engine/test_grasshopper.c:312: OpenSSL internal error: EVP_CipherUpdate(ctx, c + i, &outlen, pt + i, sz)
Abort trap: 6

And the system-provided crash report for this run/abort:

Exception Type:        EXC_CRASH (SIGABRT)
Exception Codes:       0x0000000000000000, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Application Specific Information:
abort() called

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib          0x00007fff67eda33a __pthread_kill + 10
1   libsystem_pthread.dylib         0x00007fff67f96e60 pthread_kill + 430
2   libsystem_c.dylib               0x00007fff67e61808 abort + 120
3   libcrypto.3.dylib               0x000000010b122e3a OPENSSL_die + 26
4   test_grasshopper                0x000000010aa0cdf9 test_stream + 697 (test_grasshopper.c:313)
5   test_grasshopper                0x000000010aa0c0b4 main + 308 (test_grasshopper.c:392)
6   libdyld.dylib                   0x00007fff67d92cc9 start + 1
vt-alt commented 4 years ago

Since this is just abort in OPENSSL_die and not real crash what is important is just reference to test_grasshopper.c:312, which means that EVP_CipherUpdate return 0. It's unfortunate there is no further error message.

beldmit commented 4 years ago

I'm afraid that the only option I can suggest is trying to debug. Before that try to rebuild the engine from scratch and be sure that the test uses the freshly-built engine.

This diagnostic means that the EVP_CipherUpdate call has failed, the test causes abort() in this case. When this bug appeared on my Linux machine, I fixed the gost_grasshopper_cipher_do_ctracpkm function. If the problem still occurs, it's possible that there is a bug I did not find.

vt-alt commented 4 years ago

@mouse07410 Cannot reproduce it either. What is os/cpu of the target box?

mouse07410 commented 4 years ago

MacOS 10.15.4, Xcode-11.4.1.

$ sysctl -n machdep.cpu.brand_string
Intel(R) Core(TM) i7-7567U CPU @ 3.50GHz
$ botan cpuid
CPUID flags: sse2 ssse3 sse41 sse42 avx2 rdtsc bmi1 bmi2 adx aes_ni clmul rdrand rdseed

Also, check these: https://github.com/mouse07410/engine/runs/643962113

Update

F***ing nice. Rebuilt the current master with added -g, and voila - it passes the tests. :-(

mouse07410 commented 4 years ago

Since the current master passes all the tests with the current OpenSSL-3.0 (master dev), I guess this issue can (reluctantly ;) be closed.

Though it would be nice to figure why without -g it troubles some systems.

mouse07410 commented 4 years ago

@beldmit my Travis CI keeps failing the master branch - perhaps you can take a look?

https://travis-ci.com/github/mouse07410/engine/builds/163806097 shows all the failing jobs of the last build, and this https://travis-ci.com/github/mouse07410/engine/jobs/327787180 is probably the most relevant as it builds against the OpenSSL master.

beldmit commented 4 years ago

TCL regression tests show some regression because of reworking to providers. I hope to fix some SSL-related regression though