Skip to content

Key state consistency in PQDSA_KEY setter functions#3040

Open
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:fix-pqdsa-zeroize
Open

Key state consistency in PQDSA_KEY setter functions#3040
justsmth wants to merge 1 commit intoaws:mainfrom
justsmth:fix-pqdsa-zeroize

Conversation

@justsmth
Copy link
Contributor

Description of changes:

The PQDSA_KEY setter functions could leave the key structure in an inconsistent state where stale fields no longer corresponded to the current key material.

  • PQDSA_KEY_set_raw_private_key: Did not clear key->seed on success. If the key previously held a seed (or had an uninitialized seed buffer from PQDSA_KEY_init), pqdsa_priv_encode would serialize stale or uninitialized seed bytes, since it gates on key->seed != NULL.
  • PQDSA_KEY_init: Used OPENSSL_malloc (uninitialized) for all three buffers. Changed to OPENSSL_zalloc as a defense-in-depth measure.
  • PQDSA_KEY_get0_dsa: Added a NULL check on the input, consistent with other functions in this file.

Call-outs:

The primary fix is in PQDSA_KEY_set_raw_private_key. Current code paths through the public API are not affected because EVP_PKEY_pqdsa_set_params uses PQDSA_KEY_new (which zero-initializes all fields), but these functions are exposed via internal.h and the inconsistency could be triggered by internal callers.

Separately, pqdsa_priv_decode currently ignores the pubkey parameter from OneAsymmetricKey, unlike the Ed25519 implementation which validates the provided public key against the derived one. This could be addressed as a follow-up.

Testing:

All existing ML-DSA/PQDSA tests pass (76 tests).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.51%. Comparing base (0628190) to head (c60e56a).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/pqdsa/pqdsa.c 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3040      +/-   ##
==========================================
+ Coverage   78.35%   78.51%   +0.16%     
==========================================
  Files         689      689              
  Lines      121010   121015       +5     
  Branches    16992    16998       +6     
==========================================
+ Hits        94813    95019     +206     
+ Misses      25302    25097     -205     
- Partials      895      899       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth justsmth requested a review from nebeid February 25, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants