Skip to content

async fixes: dh and tests #6369

Merged
dgarske merged 2 commits intowolfSSL:masterfrom
rizlik:dh-async-fix
May 3, 2023
Merged

async fixes: dh and tests #6369
dgarske merged 2 commits intowolfSSL:masterfrom
rizlik:dh-async-fix

Conversation

@rizlik
Copy link
Copy Markdown
Contributor

@rizlik rizlik commented May 2, 2023

Description

This PR provides a bugfix for DH code in async mode and a fix for a wrong test in dtls 1.3 unit testing.

Testing

./async_check setup && ./configure --enable-async --enable-dtls13
then loop running make check. To isolate the DH bug I run the unit testing using this conf file:

-v 4
-l TLS13-AES128-GCM-SHA256

-v 4
-l TLS13-AES128-GCM-SHA256
-y

rizlik added 2 commits May 2, 2023 16:34
The `kse->pubKeyLen` parameter is used as an input parameter to `DhGenKeyPair`
to provide the size of the `pubKey` buffer (the same size as the prime p). After
that, `kse->pubKeyLen` is used to check that the public key generated is of the
same length as the prime p. If this is not the case, the public key is
padded. If the key generation is asynchronous, then `TLSX_KeyShare_GenDhKey` may
be invoked twice. The second time, the `kse->pubKeyLen` value, updated
asynchronously by the async code, is overwritten with the prime size at the
beginning of the function. When this happens, a wrong public key value is used,
and the shared secret computed is incorrect.

Similar reasoning can be applied to `kse->keyLen`
@rizlik rizlik self-assigned this May 2, 2023
@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented May 2, 2023

jenkins retest this please

1 similar comment
@rizlik
Copy link
Copy Markdown
Contributor Author

rizlik commented May 3, 2023

jenkins retest this please

@rizlik rizlik requested a review from dgarske May 3, 2023 09:04
@rizlik rizlik assigned dgarske and unassigned rizlik May 3, 2023
@dgarske dgarske merged commit 714ec82 into wolfSSL:master May 3, 2023
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.

2 participants