Skip to content

Conversation

@effbiae
Copy link
Contributor

@effbiae effbiae commented Oct 10, 2025

Description

pull out common code to utility functions

  • AddPSKtoPreMasterSecret
  • MakePSKPreMasterSecret
  • MakeDhePSKPreMasterSecret

Testing

unit tests don't seem to cover this code very well

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@devin-ai-integration
Copy link
Contributor

🛟 Devin Lifeguard found 2 likely issues in this PR

  • check-all-return-codes snippet: Capture the return value of MakeDhePSKPreMasterSecret(ssl, 0) and, if it is non-zero, propagate the error (e.g., if (MakeDhePSKPreMasterSecret(ssl, 0) != 0) ERROR_OUT(PSK_KEY_ERROR, exit_scke);).
  • no-void-functions snippet: Change static void MakePSKPreMasterSecret(Arrays* arrays) to static int MakePSKPreMasterSecret(Arrays* arrays) and have it return 0 on success (or an error code on failure) so callers can propagate errors instead of assuming success.

@effbiae
please take a look at the above issues which Devin flagged. Devin will not fix these issues automatically.

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@dgarske
Copy link
Contributor

dgarske commented Oct 10, 2025

Contributor agreement on file. Okay to test

@dgarske
Copy link
Contributor

dgarske commented Oct 10, 2025

Jenkins retest this please: "fips_ready_part1only" failed

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Nice little cleanup.

@dgarske dgarske assigned effbiae and unassigned SparkiDev Oct 13, 2025
@effbiae effbiae force-pushed the make-pre-master-secret branch from 8b8d087 to 99a490d Compare October 15, 2025 03:48
@effbiae
Copy link
Contributor Author

effbiae commented Oct 15, 2025

what happened in the jenkins failures?

@dgarske
Copy link
Contributor

dgarske commented Oct 15, 2025

Jenkins retest this please.

./configure flags: --enable-all
(1 Error in this section)
FAIL: scripts/openssl.test

@effbiae effbiae requested review from SparkiDev and dgarske October 16, 2025 06:32
@effbiae effbiae force-pushed the make-pre-master-secret branch from 99a490d to fdcd324 Compare October 17, 2025 03:00
SparkiDev
SparkiDev previously approved these changes Oct 20, 2025
@SparkiDev
Copy link
Contributor

SparkiDev commented Oct 20, 2025

retest this please

openssl.test failures

@dgarske
Copy link
Contributor

dgarske commented Oct 20, 2025

Jenkins retest this please

@dgarske
Copy link
Contributor

dgarske commented Oct 20, 2025

Kernel module build errors not related to PR. Fixed in PR 9319.

@dgarske
Copy link
Contributor

dgarske commented Oct 20, 2025

Jenkins retest this please. Seeing some openssl.test failures.

@dgarske
Copy link
Contributor

dgarske commented Oct 21, 2025

Jenkins retest this please.

@dgarske
Copy link
Contributor

dgarske commented Oct 21, 2025

@effbiae will you please rebase and force push this PR to resolve the test errors? Thank you

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

@effbiae there appears to be a regression issue with interop. Steps to reproduce:

export WOLFSSL_OPENSSL_TEST=1
./autogen.sh
./configure --enable-all
make
make check
FAIL: scripts/openssl.test

# ./examples/client/client -p 57884 -g -r -l DHE-PSK-AES256-GCM-SHA384 -s  "-c/home/davidgarske/GitHub/wolfssl/scripts/../certs/client-cert.pem" "-k/home/davidgarske/GitHub/wolfssl/scripts/../certs/client-key.pem" "-A/home/davidgarske/GitHub/wolfssl/scripts/../certs/ca-cert.pem"
connecting to 127.0.0.1:57884
wolfSSL_connect error -313, received alert fatal error
wolfSSL error: wolfSSL_connect failed
client failed! Suite = DHE-PSK-AES256-GCM-SHA384 version =

@effbiae
Copy link
Contributor Author

effbiae commented Oct 23, 2025

i was too aggressive in the refactor. see latest commit for special casing of MakePSKPreMasterSecret
can you see a better way of special casing than passing the byte use_psk_key here?

note i need to squash the two commits before merge

@effbiae effbiae force-pushed the make-pre-master-secret branch from 7a42c24 to 51440c7 Compare October 23, 2025 03:10
@dgarske
Copy link
Contributor

dgarske commented Oct 23, 2025

Jenkins retest this please: "'PRB-long-runtime.txt_1"

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

This looks much better and seems to be passing the tests now.

@dgarske dgarske assigned effbiae and unassigned effbiae Oct 23, 2025
SparkiDev
SparkiDev previously approved these changes Oct 23, 2025
@dgarske dgarske merged commit 7bbe159 into wolfSSL:master Oct 27, 2025
269 of 270 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.

4 participants