Update tests to use OpenSSL legacy provider if OpenSSL 3.0 used #783
Conversation
## Walkthrough
The pull request updates the GitHub Actions CI workflow by renaming the existing Linux job to target Ubuntu 22.04 and adding a new job for Ubuntu 24.04 with updated OpenSSL versions. It also modifies cryptographic test files to load OpenSSL providers for version 3.0.0 and above, adjusts compilation flags in the Makefile, improves test assertions, and refines process finalization in fork tests.
## Changes
| File(s) | Change Summary |
|---------|----------------|
| `.github/workflows/ci.yml` | - Renamed `linux` job to `linux_ubuntu_22` targeting Ubuntu 22.04 with OpenSSL 3.0.2 <br> - Added `linux_ubuntu_24` job targeting Ubuntu 24.04 with OpenSSL 3.0.13 <br> - Retained matrix strategy and test steps |
| `src/lib/crypto/test/cryptotest.cpp`, `src/lib/test/p11test.cpp` | - Changed OpenSSL preprocessor condition to `#ifdef WITH_OPENSSL` <br> - Added loading of OpenSSL "legacy" and "default" providers for OpenSSL ≥ 3.0.0 |
| `src/lib/test/Makefile.am` | - Updated `AM_CPPFLAGS` to include additional and higher-level include paths <br> - Added `@CRYPTO_INCLUDES@` to compilation flags |
| `src/lib/test/ForkTests.cpp` | - Added calls to `C_Finalize(NULL_PTR)` with assertions after `C_Initialize(NULL_PTR)` in child and parent processes <br> - Child processes exit immediately after finalization <br> - Removed finalization call after fork switch |
| `src/lib/test/ObjectTests.cpp` | - Replaced `CPPUNIT_ASSERT(condition)` with `CPPUNIT_ASSERT_EQUAL(expected, actual)` for clearer test assertions |
## Suggested reviewers
- jschlyter
## Poem
> 🐰 In CI's realm of bits and bytes,
> Where workflows dance and code takes flight,
> Ubuntu's stages, now refined,
> With OpenSSL's providers aligned,
> A rabbit's build, both swift and bright! 🚀📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/test/p11test.cpp (1)
81-84: Consider refactoring duplicated provider loading code.This provider loading code is identical to the one in cryptotest.cpp. Consider extracting it to a shared utility function.
Create a new header file (e.g.,
test_utils.h):#pragma once #ifdef WITH_OPENSSL #include <openssl/opensslv.h> #if OPENSSL_VERSION_NUMBER >= 0x3000000 #include <openssl/provider.h> inline bool load_openssl_providers() { if (!OSSL_PROVIDER_load(NULL, "legacy")) { fprintf(stderr, "Failed to load legacy provider\n"); return false; } if (!OSSL_PROVIDER_load(NULL, "default")) { fprintf(stderr, "Failed to load default provider\n"); return false; } return true; } #endif #endifThen use it in both test files:
#include "test_utils.h" ... #if defined(WITH_OPENSSL) && OPENSSL_VERSION_NUMBER >= 0x3000000 - OSSL_PROVIDER_load(NULL, "legacy"); - OSSL_PROVIDER_load(NULL, "default"); + if (!load_openssl_providers()) { + return 1; + } #endif.github/workflows/ci.yml (1)
Line range hint
37-57: Consider adding OpenSSL version verification.The Ubuntu 24.04 job looks good but should verify it's using OpenSSL 3.0.
Add version check after the prepare step:
- name: Prepare run: | sudo apt update -qq sudo apt install libcppunit-dev libbotan-2-dev p11-kit + - name: Verify OpenSSL version + if: matrix.backend == 'openssl' + run: | + openssl version | grep -q "OpenSSL 3"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)src/lib/crypto/test/cryptotest.cpp(2 hunks)src/lib/test/Makefile.am(1 hunks)src/lib/test/p11test.cpp(2 hunks)
🔇 Additional comments (5)
src/lib/crypto/test/cryptotest.cpp (2)
47-51: LGTM! Clean preprocessor directive changes.The simplified
#ifdef WITH_OPENSSLachieves the same result as#if defined(WITH_OPENSSL)with cleaner syntax.
81-84: Verify provider loading success.The provider loading looks correct, but we should verify that both providers load successfully.
Add error handling:
#if defined(WITH_OPENSSL) && OPENSSL_VERSION_NUMBER >= 0x3000000 - OSSL_PROVIDER_load(NULL, "legacy"); - OSSL_PROVIDER_load(NULL, "default"); + if (!OSSL_PROVIDER_load(NULL, "legacy")) { + fprintf(stderr, "Failed to load legacy provider\n"); + return 1; + } + if (!OSSL_PROVIDER_load(NULL, "default")) { + fprintf(stderr, "Failed to load default provider\n"); + return 1; + } #endifsrc/lib/test/p11test.cpp (1)
48-57: LGTM! Necessary includes for OpenSSL support.The addition of config.h and OpenSSL headers is required for version checks and provider loading.
src/lib/test/Makefile.am (1)
3-8: LGTM! Necessary include path and crypto flag additions.The changes correctly add:
- Access to higher-level includes via
-I$(srcdir)/../..- Crypto-specific includes via
@CRYPTO_INCLUDES@.github/workflows/ci.yml (1)
11-36: LGTM! Well-structured Ubuntu 20.04 job configuration.The job is correctly configured to test with OpenSSL 1.1.1 and includes proper error handling by outputting test logs on failure.
|
@bukka thanks for this PR. You may want to add this patch that further constraints engine support to only openssl 1.x. It is from our Fedora patchset, I adjusted it to this PR. |
|
#781 is now merged, please rebase and ensure tests are not failing. |
bukka
force-pushed
the
openssl-3-testing-legacy-prov
branch
from
5041923 to
533ca3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
54-54: Consider standardizing error handling between Ubuntu versions.The CXXFLAGS configuration differs between Ubuntu versions:
- Ubuntu 20.04:
-Werror -DBOTAN_NO_DEPRECATED_WARNINGS- Ubuntu 24.04:
-DBOTAN_NO_DEPRECATED_WARNINGSConsider standardizing the
-Werrorflag usage across both configurations to maintain consistent error handling.Also applies to: 28-28
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(2 hunks)src/lib/crypto/test/cryptotest.cpp(2 hunks)src/lib/test/Makefile.am(1 hunks)src/lib/test/p11test.cpp(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/test/p11test.cpp
- src/lib/crypto/test/cryptotest.cpp
- src/lib/test/Makefile.am
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: macOS (botan)
- GitHub Check: Linux (botan)
- GitHub Check: Windows (x86, openssl)
- GitHub Check: Linux (botan)
- GitHub Check: Windows (x64, botan)
- GitHub Check: Linux (openssl)
- GitHub Check: Windows (x64, openssl)
- GitHub Check: Linux (openssl)
- GitHub Check: macOS (openssl)
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
11-13: LGTM! Clear job naming and version documentation.The job renaming from 'linux' to 'linux_ubuntu_20' with explicit version comment improves clarity by indicating the exact Ubuntu version and its associated OpenSSL/Botan versions.
37-62: Verify OpenSSL 3.0 compatibility in the new Ubuntu 24.04 job.The new job configuration aligns with the PR objectives to support OpenSSL 3.0. However, there are a few considerations:
- The
-Werrorflag is omitted in the Ubuntu 24.04 job. This might allow warnings that would be errors in the 20.04 job to pass silently.- Based on the PR objectives mentioning "legacy provider", we should verify if any additional environment variables are needed for OpenSSL 3.0 compatibility.
Let's verify the OpenSSL version and available providers in Ubuntu 24.04:
✅ Verification successful
The Ubuntu 24.04 job configuration is correct for OpenSSL 3.0 compatibility
The omission of
-Werrorin the Ubuntu 24.04 job is intentional and appropriate during the transition to OpenSSL 3.0. The codebase already includes proper version checks and test cases for OpenSSL 3.0 compatibility. No additional environment variables for legacy provider support are needed as the code handles version differences appropriately.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check OpenSSL version and providers in Ubuntu 24.04 # Note: This is for information only as the actual environment will be in the CI # Search for OpenSSL version checks or provider loading in the codebase rg -n "OPENSSL_VERSION|OPENSSL.*_provider" --type cpp # Search for any OpenSSL environment variables in test scripts rg -n "OPENSSL.*=" --type shLength of output: 2418
Script:
#!/bin/bash # Check configure scripts and build system for OpenSSL settings rg -n "openssl|OPENSSL|Werror" configure.ac rg -n "openssl|OPENSSL|Werror" Makefile.am fd configure --type f --exec grep -l "openssl|OPENSSL|Werror" {} \;Length of output: 163
Script:
#!/bin/bash # Check other CI jobs and build configurations rg -n "ubuntu-20|ubuntu-22" .github/workflows/ rg -n "CXXFLAGS|CFLAGS" .github/workflows/ rg -n "Werror|Wall" .github/workflows/Length of output: 650
|
Ok I rebased it and fixed the 1.1.1 build / tests but Ubuntu 24.04 still has got some failure (on both Botan and OpenSSL). The tests work for me with custom OpenSSL builds (tested with latest 3.0 and 3.4) so it might be something Ubuntu 24.04 specific. I will look into it next week. |
|
I have been looking into this a bit today after updating my system to Ubuntu 24.04 and I was able to recreate the issue in the pipeline locally. It seems like it's because of old API usage and some parts that were dropped but need to look into it more. |
bukka
force-pushed
the
openssl-3-testing-legacy-prov
branch
from
533ca3e to
1122e05
Compare
This shows the actual incorrect value
bukka
force-pushed
the
openssl-3-testing-legacy-prov
branch
from
1122e05 to
c933448
Compare
|
Ok this took me quite a bit of time to figure out. Firstly I added change to the assertion to see the actual error codes that did not match. It was actually working fine under gdb and the failed tests were not stable (sometime passed and sometimes fail). So it looked like some sort of race condition which it was at then. After a bit of digging, I tried to disable the ForkTest and it worked. That test basically causing some race when forked process still runs after start of another test which causes those failure. Maybe due to improved scheduling, this is now visible on Ubuntu 24.04 and possibly other newer platforms. I commented out the test and create a new issue for it to look into it later: #786 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/test/ObjectTests.cpp (1)
1395-1401: Potential Uninitialized Buffer Concern
Static analysis flagged line 1401 regarding an uninitialized variable. Here, the buffer “pStoredLabel” is declared (line 1395) and then used as the destination for C_GetAttributeValue. Although the API is expected to overwrite the buffer completely, it might be safer to zero‑initialize “pStoredLabel” (for example using memset) to silence any static analysis concerns and ensure that no garbage data is present if the API fails to write fully.🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 1401-1401: Uninitialized variable
(uninitvar)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.yml(2 hunks)src/lib/crypto/test/cryptotest.cpp(2 hunks)src/lib/test/ForkTests.cpp(3 hunks)src/lib/test/Makefile.am(1 hunks)src/lib/test/ObjectTests.cpp(40 hunks)src/lib/test/p11test.cpp(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/lib/test/ForkTests.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/test/p11test.cpp
- src/lib/test/Makefile.am
- src/lib/crypto/test/cryptotest.cpp
🧰 Additional context used
🪛 Cppcheck (2.10-2)
src/lib/test/ObjectTests.cpp
[error] 1401-1401: Uninitialized variable
(uninitvar)
🔇 Additional comments (33)
src/lib/test/ObjectTests.cpp (31)
97-97: Improved Assertion in Common Object Attributes
At line 97, the test now uses CPPUNIT_ASSERT_EQUAL(CKR_OK, rv) rather than a less-informative assertion. This change will provide a clear comparison between expected and actual error codes when C_GetAttributeValue is called.
121-127: Consistent Checks in Storage Object Attributes
The assertions after each call to C_GetAttributeValue (lines ~121–126) now use CPPUNIT_ASSERT_EQUAL. This consistency helps catch errors early by comparing the returned CK_RV with CKR_OK. The added clarity in the test for attribute length and value is welcome.
152-160: Enhanced Verification in Data Object Attributes
In the checkDataObjectAttributes function, the updated assertion at line 152 (and subsequent assertions) ensures that the return code is explicitly checked against CKR_OK. This change improves the test’s diagnostic power when attribute retrieval fails.
195-201: Clearer Assertion in Certificate Object Attributes
The modifications at lines 195 and 200 in checkCommonCertificateObjectAttributes now compare the CK_RV result with CKR_OK using CPPUNIT_ASSERT_EQUAL. This provides an explicit expectation for each call to C_GetAttributeValue, easing debugging if errors occur.
311-316: Improved Key Attribute Verification
In checkCommonKeyAttributes, the added assertions (lines 311 and 316) guarantee that both the initial and the expanded attribute queries return CKR_OK. This uniformity across functions helps maintain a consistent error‐checking style throughout the test code.
360-360: Simplified Public Key Attribute Validation
At line 360 in checkCommonPublicKeyAttributes, the use of CPPUNIT_ASSERT_EQUAL ensures that the returned value length matches the expected subject length. This slight change aids in granting more informative error messages when subject data is mismatched.
453-461: Enforcing Read‐Only Behavior with Attribute Modification
Within checkToTrueAttributes, the sequence of calls (lines 452–461) now explicitly checks that once an attribute (e.g. CKA_WRAP_WITH_TRUSTED) is set to true it cannot be reverted to false. The use of CPPUNIT_ASSERT_EQUAL with CKR_ATTRIBUTE_READ_ONLY confirms that the token correctly enforces attribute immutability after change.
473-481: Consistent Sensitive Attribute Tests
Similarly, the assertions for testing CKA_SENSITIVE (lines 472–481) now clearly require CKR_OK (when setting to true) and CKR_ATTRIBUTE_READ_ONLY (when trying to revert). These tests clearly document the intended token behavior.
497-503: Robust RSA Public Key Attribute Checks
Both the initial length retrieval (line 497) and the subsequent full value check (line 503) in checkCommonRSAPublicKeyAttributes have been updated. The changes help ensure that the memory allocated for the modulus and public exponent is valid and that the expected sizes match.
534-540: Clearer RSA Private Key Retrieval
In checkCommonRSAPrivateKeyAttributes, the two assertions (lines 534 and 540) checking the CK_RV after retrieving attributes have been updated. This uniform error-checking approach streamlines debugging when the token fails to return the expected attribute values.
754-810: Comprehensive Updates in testCreateObject
Across the testCreateObject function (lines ~754–810), all assertions checking the results of session opening, logging in, object creation, and object destruction now use CPPUNIT_ASSERT_EQUAL. This change makes it easier to pinpoint failures across various session types (read-only vs. read–write, labeled public vs. private objects). The grouping of tests by session type is maintained, and the clear error codes (e.g. CKR_USER_NOT_LOGGED_IN, CKR_SESSION_READ_ONLY) provide detailed feedback.
967-1074: Enhanced Copy Object Tests
In testCopyObject (lines ~967–1074), the assertions have been meticulously updated to check every branch of the C_CopyObject operation: allowed copies, copies that must fail because of read-only attributes, copying to a session with token attributes, and template inconsistencies. This thoroughness will help ensure that all key scenarios are covered.
1103-1120: Improved Destruction Error Handling
The testDestroyObject function (lines ~1103–1120) now uses explicit assertions to verify that destroying objects in invalid sessions or with invalid handles returns the proper error codes (e.g. CKR_SESSION_HANDLE_INVALID and CKR_OBJECT_HANDLE_INVALID). This clarity is essential when enforcing proper session usage.
1201-1224: Verifying Object Size Retrieval
Updates in testGetObjectSize (lines ~1201–1224) now ensure that retrieving the size of an object explicitly returns CKR_OK and that the size is reported as CK_UNAVAILABLE_INFORMATION. This may be a token‐specific behavior but is clearly asserted now.
1238-1275: Solidifying GetAttributeValue Behavior
In testGetAttributeValue (lines ~1238–1275), the tests now check not only for success but also for proper argument error codes when invalid handles are provided. This helps cover more edge cases, reinforcing robustness.
1338-1410: Detailed SetAttributeValue Testing
The testSetAttributeValue function (lines ~1338–1410) has been updated so that each attempt to change an attribute—whether in a read-only session, token object, or nonmodifiable object—is explicitly compared to the expected error code. The use of explicit assertions here strengthens the test coverage around attribute consistency and immutability.🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 1401-1401: Uninitialized variable
(uninitvar)
1426-1514: Expanded Object Finding Tests
The testFindObjects function (lines ~1426–1514) now verifies that objects can be found (or not found) depending on the login state and object type. The updated assertions guarantee that the test suite will accurately detect lost session objects after logout and correctly count token objects.
1528-1584: Thorough RSA Key Pair Generation Testing
In testGenerateKeys (lines ~1528–1584), the numerous calls to generateRsaKeyPair with various permutations (session vs. token; public vs. private) are now accompanied by assertion checks for CKR_OK. This confirms that the key generation interface behaves as expected for all combinations.
1597-1613: Certificate Object Creation Validations
Within testCreateCertificates (lines ~1597–1613), the tests now explicitly confirm that creating an incomplete certificate object yields CKR_TEMPLATE_INCOMPLETE and that a properly formed X509 certificate object is accepted. Additionally, an attempt to set a read-only attribute correctly results in CKR_ATTRIBUTE_READ_ONLY.
1638-1658: Validating Default Data Attributes
The testDefaultDataAttributes (lines ~1638–1658) uses helper functions (checkCommonObjectAttributes, checkCommonStorageObjectAttributes, and checkDataObjectAttributes) to verify that default attributes on a data object match expected values. The changes here are consistent with the updated assertion style.
1685-1705: X509 Certificate Default Attributes
The testDefaultX509CertAttributes function (lines ~1685–1705) now verifies that a minimal X509 certificate object’s attributes are as expected. Using memset for empty dates and comparing check values makes the test robust.
1737-1758: RSA Public Key Default Attributes
In testDefaultRSAPubAttributes (lines ~1737–1758), the updated assertions ensure that public key objects have correctly populated storage, key, and RSA-specific attributes. This consistency is critical to later cryptographic operations.
1800-1823: RSA Private Key Default Attributes and “ToTrue” Enforcement
The testDefaultRSAPrivAttributes (lines ~1800–1823) now checks that private key attributes are not only correctly set but that modifications (via checkToTrueAttributes) trigger the proper error codes. This enforces the token’s behavior regarding sensitive key material.
1855-1890: Always/Never Sensitive Attribute Handling
The testAlwaysNeverAttribute (lines ~1855–1890) now clearly verifies that key attributes CKA_ALWAYS_SENSITIVE and CKA_NEVER_EXTRACTABLE are set based on the initial key-generation parameters and remain read‑only afterward. The assertions here precisely check both the success of generation and the refusal to change these values.
1925-1956: Sensitive Attribute Protection Verification
In testSensitiveAttributes (lines ~1925–1956), the tests loop over several sensitive attributes ensuring that when a key is marked as sensitive, attempts to read those attributes return CKR_ATTRIBUTE_SENSITIVE. When the key is not sensitive, all attributes can be retrieved successfully. This guarantees that sensitive key material is properly shielded.
1979-1997: Handling Invalid Attributes Robustly
The testGetInvalidAttribute (lines ~1979–1997) now confirms that requesting an attribute that is not valid for the object (e.g. CKA_SIGN on a data object) returns CKR_ATTRIBUTE_TYPE_INVALID. Such defensive testing strengthens the API’s reliability.
2034-2101: Re-Authentication and Cryptographic Operations
The testReAuthentication function (lines ~2034–2101) has been updated with multiple scenarios:
- Testing C_Login with the context‑specific user PIN for signing, ensuring that an incorrect PIN returns CKR_PIN_INCORRECT.
- Verifying that signing (both in one-shot and via update/final) behaves correctly when the re‑authentication command is issued.
- Confirming that without re‑authentication, operations fail with CKR_USER_NOT_LOGGED_IN or CKR_OPERATION_NOT_INITIALIZED.
These checks will help prevent unintentional use of keys without explicit re‑authentication when needed.
2104-2124: Testing Encryption/Decryption With Re‑Authentication
The latter part of testReAuthentication (lines ~2104–2124) also covers encryption and decryption:
- Successful decryption after re‑authentication is confirmed by comparing the recovered plaintext.
- Attempts without proper re‑authentication correctly fail.
This thorough coverage ensures cryptographic operations enforce session constraints.
2136-2193: Allowed Mechanisms Enforcement
The testAllowedMechanisms function (lines ~2136–2193) now validates that the key’s allowed mechanisms are enforced:
- Attempts to sign using a mechanism not in the allowed list fail with CKR_MECHANISM_INVALID.
- Allowed mechanisms (SHA256_HMAC and SHA512_HMAC) succeed.
This helps ensure that keys cannot be misused with unintended cryptographic algorithms.
2235-2289: Template Attribute Retrieval in Public Keys
Finally, testTemplateAttribute (lines ~2235–2289) checks that the wrap template embedded in a public key is correctly stored and can be queried. The code now explicitly verifies that the value length equals three CK_ATTRIBUTE structures and that each sub-attribute (key type, public exponent, allowed mechanisms) matches its expected size and value. This granularity will be very useful for future debugging of template composition issues.
2323-2434: Secret Key Creation and Check Value Verification
The testCreateSecretKey (lines ~2323–2434) verifies the creation of secret keys (generic, AES, DES, DES2, and DES3) by checking that the computed key check value (KCV) matches the expected constant. This uniform testing across various key types assures you that key generation and inspection are working as intended..github/workflows/ci.yml (2)
11-36: Renaming and CI Update for Ubuntu 20.04
The existing Linux job has been renamed to “linux_ubuntu_20” (lines 11–13) and now clearly documents its dependency on OpenSSL 1.1.1 and Botan 2.12. This renaming will help contributors identify which environment is being targeted during troubleshooting. The matrix strategy remains unchanged.
37-62: New CI Job for Ubuntu 24.04
A new job named “linux_ubuntu_24” has been added (lines 37–62) targeting Ubuntu 24.04. The comments now indicate its dependencies on OpenSSL 3.0 and Botan 2.19. The steps (checkout, prepare, build, and test) follow the same structure as in the 20.04 job, ensuring consistency across environments.
bukka
force-pushed
the
openssl-3-testing-legacy-prov
branch
2 times, most recently
from
143c8b6 to
a071b65
Compare
|
@jschlyter This is now ready for review. Everything is rebased and pipeline is fixed including removal of 20.04 Ubuntu runner that is no longer supported. I also applied a proper fix to the ForkTest so this fixes #786 . Commits are independent so if you are happy with it, then best to merge it using a merge commit (as you did for other PR's previously) or using rebase. |
|
Hello @bukka. I am experiencing random test failures in Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069539 and maybe this PR might fix it. Regarding the PR itself: The title says "Fix ForkTest" but there is a FIXME in the code saying "This is currently disabled". Is the FIXME really current? Thanks. |
The ForkTest was causing a race conditions with other tests as the forked child continued to run. This is fixed by exiting from the child.
The 20.04 is no longer available
bukka
force-pushed
the
openssl-3-testing-legacy-prov
branch
from
a071b65 to
0f2bffa
Compare
|
Hi @sanvila
This was just a left over as previously disabled it. This is now removed as the changes in the ForkTest should fix it - at least they fix it for me and pipeline. |
|
|
||
| rv = CRYPTOKI_F_PTR( C_GetAttributeValue(hSession, hObject, &attribs[0], 1) ); | ||
| CPPUNIT_ASSERT(rv == CKR_OK); | ||
| CPPUNIT_ASSERT_EQUAL(CKR_OK, rv); |
There was a problem hiding this comment.
Great that you converted to use a macro that gives better diagnostic outputs! I think the swapped order makes more sense, "assert that the calculated value is equal to the golden reference", but it is not so important. Feel free to create a separate PR with this rewrite to make the actual OpenSSL port shorter.
|
Closing in favour of #806 |
5 participants
This enables legacy provider when running tests with OpenSSL 3.0 as some old algorithms are used and it would be good to keep the tested. It also makes tests green when running with OpenSSL 3.0 (except maybe when the crypto RHEL policy is selected).
Summary by CodeRabbit
CI/CD Updates
Cryptography Improvements
Test Enhancements
Build System