| Summary: | AB-INT PTEST: openssl 80-test_ssl_new.t ptest failure | ||
|---|---|---|---|
| Product: | [QA/Testing] Package Testing (ptest) | Reporter: | Alexandre Belloni <alexandre.belloni> |
| Component: | ptest | Assignee: | William Lyu <william.lyu> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | Medium+ | CC: | alex.kanavin, alexandre.belloni, randy.macleod, richard.purdie, william.lyu |
| Version: | unspecified | ||
| Target Milestone: | 5.3 M4 | ||
| Hardware: | x86 | ||
| OS: | Multiple | ||
| Whiteboard: | AB-INT | ||
| OS type for building Yocto: | --- | Type of Regression: | --- |
| Verified: | Documentation change: | No (bug/feature does not impact docs) | |
|
Description
Alexandre Belloni
2023-10-05 21:57:54 UTC
Overview
========
In attempt to reproduce the reported issue, the OpenSSL tests were run on both on host and emulated targets (with x86-64 and ARM arch). Tests were run both while the host was being idle and while the host was being stressed ("stress-ng --cpu 8 --hdd 8" or "stress-ng --vm 8 --vm-bytes 30000000000"). The host has 8 cores with 1 thread per core and 32 GiB RAM. None of these attempted test runs were able to reproduce the issue.
Note that the reported bug happended on a Yocto autobuilder with ARM architecture and Ubuntu-18.04. It might be useful to take into account that this particular host is known to be slower.
Due to the difficulty of reproducing this issue, instead of investigating further and putting more effort to fix the issue, I plan to only modify the test code so that more internal states are printed should this or similar issues ever arise in future tests.
NOTE: The following provides technical details that are potentially relevant to reproducing and investigating this issue. Unless one is interested in reproducing and investigating this issue, these technical details can be ignored.
I. Attempts to Reproduce the Issue
==================================
To run the tests on host, the following steps are performed:
$ git clone https://github.com/openssl/openssl.git
$ cd openssl
$ git checkout openssl-3.1.3 # The tag corresponding to the release version of OpenSSL in bug report is used
$ ./Configure
$ make
$ make test TESTS='test_ssl_new' OPENSSL_TEST_RAND_ORDER=1695916525 V=1
Note that "TESTS='test_ssl_new'" specifies that only offending test case is run. "OPENSSL_TEST_RAND_ORDER=1695916525" specifies the random seed as given in the bug report. "V=1" enables verbosity of the test harness so that more test output is printed.
To run the tests on emulated targets using ptest:
$ export OPENSSL_TEST_RAND_ORDER=1695916525
$ export V=1
$ ptest-runner openssl
II. Structure of Tests
======================
The entry point of each top-level test cases are contained in "{some two digit number}-{test case name}.t" scripts under "{repo dir}/test/recipes/". In particular, test "test_ssl_new" is run from "{repo dir}/test/recipes/80-test_ssl_new.t"
For each ".in" file under "{repo dir}/test/ssl-tests/", one subtest is run against each of the following providers (OpenSSL providers) - "none", "default", and potentially "fips". The entry point of each of such subtest is one of the three "test_conf" subroutine call from "{repo dir}/test/recipes/80-test_ssl_new.t:134" to "{repo dir}/test/recipes/80-test_ssl_new.t:145".
For our reported bug, the test runs the ".t" configuration file "07-dtls-protocol-version.cnf.in" with provider "default". Notice that there are three sub-subtests in subroutine "test_conf". The reported bug corresponds to the third sub-subtest located from "{repo dir}/test/recipes/80-test_ssl_new.t:175" to "{repo dir}/test/recipes/80-test_ssl_new.t:185".
This sub-subtest invokes "{repo dir}/test/ssl_test" which is compiled from "{repo dir}/test/ssl_test.c". The sub-subtest runs on configuration file "{repo dir}/test-runs/test_ssl_new/07-dtls-protocol-version.cnf.default" which contains 64 different OpenSSL handshake tests with each testing a different server/client supported protocol version range. Note that "{repo dir}/test-runs/test_ssl_new/07-dtls-protocol-version.cnf.default" is automatically generated from running "{repo dir}/test/generate_ssl_tests.pl" with the aforementioned ".in" file ("07-dtls-protocol-version.cnf.in").
In "{repo dir}/test/ssl_test" (compiled from "{repo dir}/test/ssl_test.c"), setup_tests() is the test harness API that sets up all test function calls with each invocation of such test function corresponding to one of the 64 OpenSSL handshake test. The 64 OpenSSL handshake tests are numbered from "test-0" to "test-63". An invocation of "{repo dir}/test/ssl_test.c:test_handshake()" with int value x corresponds to running handshake test "test-x".
The above discussion covers the structures of all tests/subtests associated with the issue in the bug report.
III. Test Source Code Analysis
==============================
Since this issue or any similar issue is never successfully reproduced, only static analysis (only analyzing the test output in the bug report as opposed to analyzing test output from each new test runs) on the code is done to narrow down potential locations of the bug. The following is static analysis made so far.
In "{repo dir}/test/ssl_test.c:test_handshake()", by printing out the local int variable `idx`, one can observe that it equals to the iteration value in the test output log minus 1. In our bug report, the failing subtest had test result "not ok 18 - iteration 18", and this means that "test-17" of the 64 OpenSSL handshake tests failed.
In the bug report, an error occurs at ssl_test.c:36 which is in check_result(). check_result() is only invoked by check_test() which is only invoked at the end (ssl_test.c:529) of test_handshake(). What directly fails the test is `result->result` (where `result` is the local variable in test_handshake()) having value 3 (indicated by the line "[3] compared to [0]" in bug report). `result` is only set by invocation of do_handshake() at ssl_test.c:525.
do_handshake() is defined in "{repo dir}/test/helpers/handshake.c". We can look for what logic sets `result->result` to `SSL_TEST_INTERNAL_ERROR` (which has value 3) in do_handshake_internal(). Unfortunately, there are multiple different paths of execution that may set `result-result` to `SSL_TEST_INTERNAL_ERROR`. This could have been further narrowed down if the "step history" (this may include phase information, client state, and server state) of the "client-server handshake" is available. I plan to modify the test code so that, upon error, it prints the most recent "step history" performed during the "client-server handshake".
William, YP can take the patch locally since the upstream merge may be delayed by having to go through the openssl contributor agreement. Please include links to upstream submission as usual. Due to not being able to reproduce this issue after extensive testing, I have decided to improve error reporting so that more debugging information can be obtained should this issue or similar issues arise in the future. The following is the link to the patch improving error reporting. https://git.openembedded.org/openembedded-core/commit/?id=5bf9a70f580357badd01f39822998985654b0bfc Move to 5.0-M4. We might not see the problem before that and if not, then we can likely close the defect as can't reproduce/works for me at that time. Still dealing with openssl contributor paperwork... We are still working with people at Wind River to get the openssl CLA done. Ugh! Alex, Do you want William to send his patch to the oe-core list while we continue to work on the paperwork? This patch no longer applies to openssl 3.4.0, and meanwhile openssl upstream has closed the submission due to lack of activity and lack of cla on the submitter side: https://github.com/openssl/openssl/pull/22481 I'm going to drop the patch from the 3.4.0 update. Bulk move. Waiting on replacement management to get our corporate contributors agreement done. This is taking a while but if it's not done in a few months, I'll take another approach. Pinged new management after giving them a few weeks to settle in! https://git.openembedded.org/openembedded-core/commit/?id=5bf9a70f580357badd01f39822998985654b0bfc William may one day get permission from corporate to complete the patch submission. |