Skip to content

fix: update test broken by Openssl dhe generation change#5580

Merged
lrstewart merged 3 commits intoaws:mainfrom
lrstewart:dhefix
Oct 24, 2025
Merged

fix: update test broken by Openssl dhe generation change#5580
lrstewart merged 3 commits intoaws:mainfrom
lrstewart:dhefix

Conversation

@lrstewart
Copy link
Copy Markdown
Contributor

Description of changes:

This Openssl commit broke our libcrypto drbg override test: openssl/openssl@d6510d9

My understanding is that basically that commit reduces the size of the generated DHE private key. Changing the private key changes the public key, and that test asserts an exact value of a generated DHE public key in order to prove that the DRBG is properly overridden. The test isn't specifically trying to test the DHE math: the DHE math is just a way to prove we can predict randomness.

Rather than dealing with "what libcrypto/version is this?" I just updated the test to accept either value.

Call-outs:

Before the Openssl change, the generated private key looks like:

36BA361FA14563FB1E8BCF88932F9FA739D265E3D8B621CD852645300AA4741BE0E4CBCFD90079D2B3D1A7C44B5064782D408151DFFA000781BCBD1390BC65F9E2FE9F84335D1262AC664454315BCB215ACE4D96D68E2085DF850F76756BE04C320C1021EFA5C8403F12DE7721629014A126E695C9C77826AD7B764E7598F013C709BEF1F1935FFFD5A0DE38DF8DD12EBBB5CE2712F9CEF54C93E1A6D72C4DC906036705E0EA1F5B02C4F0893E2D53E38D284E04C1F4C976B40F2BAFFAA75B1F0AE7AC9CE35E13972451E86C53EF7613B112884771326EDA9F2D1D3468E4F1B8E9BB53732F93FBCB9D38CE26AEB276F2FBC56A67ED16D330BA32717E9AEAC812

After the change, it looks like:

56BA361FA14563FB1E8BCF88932F9FA739D265E3D8B621CD852645300AA4741BE0E4CBCFD90079D2B3D1A7C44B5064782D408151DFFA000781BCBD1390BC65F9E2FE9F84335D1262AC664454315BCB215ACE4D96D68E2085DF850F76756BE04C320C1021EFA5C8403F12DE7721629014A126E695C9C77826AD7B764E7598F013C709BEF1F1935FFFD5A0DE38DF8DD12EBBB5CE2712F9CEF54C93E1A6D72C4DC906036705E0EA1F5B02C4F0893E2D53E38D284E04C1F4C976B40F2BAFFAA75B1F0AE7AC9CE35E13972451E86C53EF7613B112884771326EDA9F2D1D3468E4F1B8E9BB53732F93FBCB9D38CE26AEB276F2FBC56A67ED16D330BA32717E9AEAC812

So almost the same, except that first byte?

Testing:

I figured out which commit broke the test via git bisect.
Updated the existing test to allow the new value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Oct 24, 2025
@lrstewart lrstewart marked this pull request as ready for review October 24, 2025 05:53
@lrstewart lrstewart requested a review from boquan-fang October 24, 2025 16:25
@lrstewart lrstewart enabled auto-merge October 24, 2025 18:27
@lrstewart lrstewart added this pull request to the merge queue Oct 24, 2025
Merged via the queue into aws:main with commit 18c0f26 Oct 24, 2025
50 checks passed
@lrstewart lrstewart deleted the dhefix branch October 24, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants