Check parameters before comparing pqdsa public keys

[samuel40791765]

Issues:

Addresses V2196133741

Description of changes:

pqdsa_pub_cmp memcmp'd two public keys using the length from a's variant, with no check that a and b were the same ML-DSA parameter set. Adds a pqdsa_cmp_parameters helper (mirroring kem_cmp_parameters in p_kem_asn1.c) that validates both keys are populated and share the same NID, and makes pqdsa_pub_cmp return early when parameters don't match.

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]

Codecov Report

❌ Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.11%. Comparing base (9651480) to head (39ccb42).

Files with missing lines	Patch %	Lines
crypto/evp_extra/p_pqdsa_asn1.c	76.19%	5 Missing ⚠️
crypto/evp_extra/p_kem_asn1.c	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3229      +/-   ##
==========================================
- Coverage   78.12%   78.11%   -0.01%     
==========================================
  Files         689      689              
  Lines      123214   123232      +18     
  Branches    17137    17141       +4     
==========================================
+ Hits        96257    96261       +4     
- Misses      26047    26061      +14     
  Partials      910      910              

☔ 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.

[WillChilds-Klein]

do we need NULL checks on a and b here before the dereference?

also, can we document this functions return values in a comment above? why return -2 instead of 0? callers might inadvertently do !pqdsa_cmp_parameters(...) and have unexpected behavior

[samuel40791765]

Added NULL checks and a doc comment.

I was confused about this at first too, but the tri-state return is intentional to align with EVP_PKEY_cmp's convention (1 = equal, 0 = not equal, negative = error). Also mirrored the same NULL check and docstring onto kem_cmp_parameters since it's applicable there as well.

aws-lc/include/openssl/evp.h

Lines 56 to 61 in 9651480

	// EVP_PKEY_cmp compares |a| and |b| and returns one if they are equal, zero if
	// not and a negative number on error.
	//
	// WARNING: this differs from the traditional return value of a "cmp"
	// function.
	OPENSSL_EXPORT int EVP_PKEY_cmp(const EVP_PKEY *a, const EVP_PKEY *b);

[WillChilds-Klein]

thanks. if we need to return negative on error, -2 still "feels" like an odd return value vs. -1 but if "negative" is the documented contract it doesn't really matter.

[WillChilds-Klein]

thanks. if we need to return negative on error, -2 still "feels" like an odd return value vs. -1 but if "negative" is the documented contract it doesn't really matter.