fix: memory leak in hybrid key share generation + doc comment

[srpatcha]

Changes

1. Clarify session cache size comment in ssl_sess.c

Documentation improvement to clarify the session cache size comment.

2. Fix memory leak in hybrid key share generation (src/tls.c)

In the hybrid key share generation path, if the second XMALLOC() for pqc_kse fails after ecc_kse was successfully allocated, ecc_kse is leaked. Added explicit XFREE(ecc_kse) cleanup in the error path.

[wolfSSL-Bot]

Can one of the admins verify this patch?

[dgarske]

Hi @srpatcha ,

Thank you for this pull request submission. I don't see you on file as an approved contributor. We are going to review this and provide some feedback. Can you tell us more about your project , what region you are located and if you plan future submissions? Also what techniques did you use to locate these issues?

Thanks,
David Garske, wolfSSL

[srpatcha]

Hi @dgarske, thank you for reviewing!

I'm Srikanth Patchava, a software engineer based in Sunnyvale, CA. I work on embedded systems and noticed this memory leak while studying the hybrid key share implementation. The fix ensures the allocated buffer is freed on error paths in TLSX_KeyShare_GenKey().

I'm interested in continuing to contribute to wolfSSL, particularly around memory safety and security hardening in the TLS implementation. Happy to provide any additional information needed for the contributor approval process.

[Frauschi]

Hi @srpatcha,

I reviewed your proposed changes and have some initial feedback:

1. The XXX placeholder in the comment for the different SESSION_CACHE options is intentional, and the actual valid macro options are documented right above this comment. So a reviewer or code analysis tool should easily understand this. I think no changes are required for this.
2. There isn't an actual memory leak in the TLSX_KeyShare_GenPqcHybridKeyClient() method. In case of an allocation failure for the pqc_kse, the already allocated ecc_kse is in fact properly freed in the TLSX_KeyShare_FreeAll(ecc_kse, ...) call at the end of the function. So no changes are required.
3. The potential memory leak in wc_ecc_mulmod_ex() is identified correctly, thanks for that! However, your suggested fix in incomplete and could cause undefined behavior in some error conditions. As this is a small fix unrelated to your other, much larger changes (see below), I went ahead and moved this fix out into a separate PR, see Prevent ECC tmp key leak and UB #10346. I credited you as the identifier of this issue.
4. Regarding your proposed change to the TLSX_SessionTicket_Parse() method: the ret < 0 branch doesn't guard any allocation or cache write; nothing in DoClientTicket's failure path leaks, and the cache isn't populated here at all, so the stated bug doesn't exist. Meanwhile, zeroing all unrecognized negative return codes swallows real fatal errors and, most importantly, WC_PENDING_E, breaking async-crypt ticket decrypt. So this change must be reverted.

So this leaves the most significant change of the PR, the feature addition of the TicketKeyRotation functionality. After analyzing your new code, I think I understand where this should go ultimately. However, the current state of this feature appears to be half-baked, as any integration into the actual wolfssl library is still missing (build and test infrastructure, usage within the library). Therefore, I think that there is some more work for you to do. Please also add some explanation about the feature in the PR description.

If you want to proceed with this feature addition, we would require you to sign a contributor agreement. For that, please email support@wolfssl.com to request that procedure. Please reference this GitHub PR in the message, too.

Thanks,
Tobias Frauenschläger, wolfSSL