Skip unsupported EC curves in KeysInUse tests - #175
Merged
Maxwell Moyer-McKee (mamckee) merged 2 commits intoAug 26, 2026
Conversation
Maxwell Moyer-McKee (mamckee)
requested review from
MS-megliu and
Samuel Lee (samuel-lee-msft)
and
a lite review from Copilot
August 25, 2026 23:30
Copilot started reviewing on behalf of
Maxwell Moyer-McKee (mamckee)
August 25, 2026 23:30
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the KeysInUse test harness to runtime-skip EC curves that can’t be generated on the current platform (instead of compile-time skipping via hardcoded #ifdef checks), and bumps the project version accordingly.
Changes:
- Removes the compile-time
#ifdef NID_X9_62_prime192v1guard from the EC test key list. - Updates key generation to continue past EC curve failures and skips test execution for entries whose key generation was skipped.
- Bumps
SymCrypt-OpenSSLproject version from1.11.0to1.11.1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/KeysInUseTest/KeysInUseTest.cpp | Switches KeysInUse EC test coverage from hardcoded-NID assumptions to runtime skipping for unsupported curves. |
| CMakeLists.txt | Increments project version to 1.11.1. |
Suppressed comments (1)
test/KeysInUseTest/KeysInUseTest.cpp:1644
- This skip path has the same
OBJ_nid2sn()NULL-to-%srisk as above. It should also clear/freetestKeys[i]fields before continuing, so a failedEVP_PKEY_keygen()can’t leave a non-NULL/partialpkeythat would later be treated as a generated key.
if (testKeys[i].keyType == EVP_PKEY_EC)
{
printf("Skipping unsupported curve %s\n", OBJ_nid2sn(testKeys[i].keygenParams));
ERR_clear_error();
EVP_PKEY_CTX_free(ctx);
ctx = NULL;
continue;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
MS-megliu
reviewed
Aug 25, 2026
Maxwell Moyer-McKee (mamckee)
requested review from
MS-megliu
and
a lite review from Copilot
August 26, 2026 20:13
Copilot started reviewing on behalf of
Maxwell Moyer-McKee (mamckee)
August 26, 2026 20:13
View session
MS-megliu
approved these changes
Aug 26, 2026
Maxwell Moyer-McKee (mamckee)
deleted the
mamckee-robust-keysinuse-test
branch
August 26, 2026 23:17
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.
This PR skips EC curves for KeysInUse tests programmatically rather than relying on hardcoded NIDs