Skip to content

Conversation

@billphipps
Copy link
Contributor

@billphipps billphipps commented Apr 8, 2025

Description

Following the pattern in ecc.h for ECC_BUFSIZE to expose a reasonable buffer size when exporting a Curve25519 key to DER, an enum was added to allow static allocation of DER buffers: CURVE25519_MAX_KEY_TO_DER_SZ.

Testing

Updates were made to the curve25519 API tests to use this new constant, rather than the magic number of 128 or the overestimation of ONEK_BUF.

Checklist

  • [ x ] added tests
  • [ N/A ] updated/added doxygen
  • [ N/A ] updated appropriate READMEs
  • [ N/A ] Updated manual and documentation

@billphipps billphipps requested review from SparkiDev and Copilot April 8, 2025 19:23
@billphipps billphipps self-assigned this Apr 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

wolfssl/wolfcrypt/curve448.h:47

  • [nitpick] Consider renaming CURVE448_BUFSIZE to CURVE448_DER_BUFSIZE for clarity on its specific use for DER key exports.
    CURVE448_BUFSIZE = 128,  /* for DER exported keys temp buffer */

wolfssl/wolfcrypt/curve25519.h:49

  • [nitpick] Consider renaming CURVE25519_BUFSIZE to CURVE25519_DER_BUFSIZE to explicitly reflect its purpose in DER key export operations.
    CURVE25519_BUFSIZE = 128,  /* for exported DER keys temp buffer */

@billphipps
Copy link
Contributor Author

Retest this please Jenkins

@billphipps billphipps changed the title Update to expose reasonable DER buffer sizes for Curve448/25519 Update to expose reasonable DER buffer sizes for Curve25519 Apr 14, 2025
@SparkiDev SparkiDev assigned wolfSSL-Bot and SparkiDev and unassigned billphipps Apr 14, 2025
@SparkiDev SparkiDev requested a review from wolfSSL-Bot April 14, 2025 22:34
@SparkiDev SparkiDev merged commit 9106d12 into wolfSSL:master Apr 14, 2025
186 checks passed
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