Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MLDSA to fuzzing corpus #2174

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add MLDSA to fuzzing corpus #2174

wants to merge 2 commits into from

Conversation

jakemas
Copy link
Contributor

@jakemas jakemas commented Feb 6, 2025

Issues:

Resolves #PQCrypto-120

Description of changes:

This PR updates the fuzz tests to include seeds based upon ML-DSA private key encodings. I provided manual seeds to the corpus to fuzz specifically against private key import.

Seed1: ML-DSA-44 private key:

 -----BEGIN PRIVATE KEY-----
 MDICAQAwCwYJYIZIAWUDBAMRBCAAAQIDBAUGBwgJCgsMDQ4PEBESExQVFhcYGRob
 HB0eHw==
 -----END PRIVATE KEY-----

Seed2 ML-DSA-65 private key:

-----BEGIN PRIVATE KEY-----
MDICAQAwCwYJYIZIAWUDBAMSBCAAAQIDBAUGBwgJCgsMDQ4PEBESExQVFhcYGRob
HB0eHw==
-----END PRIVATE KEY-----

Seed1: ML-DSA-87 private key:

-----BEGIN PRIVATE KEY-----
MDICAQAwCwYJYIZIAWUDBAMTBCAAAQIDBAUGBwgJCgsMDQ4PEBESExQVFhcYGRob
HB0eHw==
-----END PRIVATE KEY-----

Methodology:

  • fuzz new seed values: ./fuzz/pkcs8 -max_len=2048 -jobs=32 -workers=32 mldsa-seed/
  • reduce down to a minimized corpus: ./fuzz/pkcs8 -merge=1 min-corpus/ mldsa-seed/
  • check coverage by introducing an error (emulated Fix issue with ML-DSA key parsing #2152 and verified these fuzzes)
  • merge into existing corpus: ./fuzz/pkcs8 -merge=1 merged/ min-corpus/ ../fuzz/pkcs8_corpus/
  • PR this new corpus.

As an example, the new file 2aedfe1f6ee6e99c5947577cc6d59a9611f2a085 contains the fuzz clearly generated from the ML-DSA seeded BASE64 encoding:

MDICAQAwKwYJKsOcSMOcy5wKAQEKMB7CogICAMucw5p+wq7Fk35+fsOHwrDLh8uHy4dCy4YwIwJe
y53Lh8WTAAAKMQQAy4c=

Call-outs:

Wasn't able to get a coverage report from https://github.com/aws/aws-lc/blob/main/docs/Coverage.md as lcov didn't want to play with clang.

We were at 231 fuzz vectors in pkcs8_corpus and now we are at 333.

Testing:

Testing coverage was tested by introducing an error (emulated #2152). These tests caught the error.

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.

@jakemas jakemas requested a review from a team as a code owner February 6, 2025 22:16
@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.00%. Comparing base (f407534) to head (540548e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2174      +/-   ##
==========================================
+ Coverage   78.98%   79.00%   +0.01%     
==========================================
  Files         611      611              
  Lines      105904   105905       +1     
  Branches    14982    14982              
==========================================
+ Hits        83651    83671      +20     
+ Misses      21598    21580      -18     
+ Partials      655      654       -1     

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

Copy link
Contributor

@andrewhop andrewhop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified new corpus resulted in a step up in pkcs8 block coverage.

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