test: add regression tests for issue #118 (error queue pollution)#926
Merged
Conversation
Add sign/verify tests that deliberately dirty the OpenSSL error queue with pbkdf2, hash, hmac, and cipher operations before signing. Covers RSA, Ed25519, and ECDSA key types with 10 iterations each to catch intermittent failures.
Contributor
🤖 End-to-End Test Results - AndroidStatus: ✅ Passed 📸 Final Test ScreenshotScreenshot automatically captured from End-to-End tests and will expire in 30 days This comment is automatically updated on each test run. |
Contributor
🤖 End-to-End Test Results - iOSStatus: ✅ Passed 📸 Final Test ScreenshotScreenshot automatically captured from End-to-End tests and will expire in 30 days This comment is automatically updated on each test run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Adds regression tests for #118 —
sign()failing with "Failed to read private key" after calling other crypto operations due to OpenSSL error queue pollution.Analysis: Why #118 Is Resolved
The original bug was caused by OpenSSL error queue pollution. OpenSSL maintains a per-thread error queue — when crypto operations like
pbkdf2Sync()run, they can leave stale errors in the queue. Ifsign()then parses a private key without first clearing that queue, the stale errors causeEVP_PKEYparsing to fail with "Failed to read private key", even though the key is perfectly valid.Since #118 was filed, the entire native layer has been rewritten. The fix is defensive
ERR_clear_error()calls before every key parsing operation inKeyObjectData.cpp:TryParsePrivateKey()(PEM private key path)TryParsePrivateKey()inGetPublicOrPrivateKey()(PKCS8/SEC1/PKCS1 fallback)TryParsePrivateKey()attempt (PEM auto-detect path)TryParsePrivateKey()inGetPrivateKey()(DER private key path)Additionally, the utility helper
getOpenSSLError()now drains and clears the error queue after retrieving an error message, preventing residual pollution.This directly addresses the root cause: no matter what crypto operations ran before
sign(), the error queue is always clean when key parsing begins.Changes
example/src/tests/keys/sign_verify_error_queue.tswith 7 regression tests:signafterpbkdf2Sync(repeated)signaftercreateHash(repeated)signaftercreateHmac(repeated)signafter AES cipher/decipher (repeated)signafter mixed crypto operations (repeated)signafter mixed crypto operationssignafter mixed crypto operationsuseTestsList.tsTesting
Run the
keys.sign/verifytest suite in the example app. All 7 new tests should pass.Closes #118