Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Diffie-Hellman into WeakKeyEx category #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

slm4996
Copy link

@slm4996 slm4996 commented Jan 19, 2023

Noticed that we had a category WeakKeyEx, but were not using it for Diffie-Hellman Key Exchange Algorithm and options. This pull request places Diffie-Hellman and associated settings for Key Exchange Algorithms into the Weak Key Exchange Algorithms category.

@Harvester57
Copy link
Contributor

But depending on the choice you make for the key length, DH is not weak, maybe it would be more useful to deprecate smaller key lengths (2048 bits and less)

@slm4996
Copy link
Author

slm4996 commented Jan 19, 2023

I may be misunderstanding the DH key exchange, but isn't it capped at 2048 key lengths? To use better would require the use of Elliptic-Curve Diffie-Hellman (ECDH), which is broken out as its own entity?

Edit:
If we do not shift DH to Weak Key Exchange Algorithms then the project readme should be updated, since it refers to Diffie-Hellman as a weak Key Exchange Algorithm.

@Crosse
Copy link
Owner

Crosse commented Jan 19, 2023

DH can have the following values:

<enum id="DHServer_MinLength" valueName="ServerMinKeyBitLength" >
<item displayName="$(string.DH_Value1024)" >
<value>
<decimal value="1024" />
</value>
</item>
<item displayName="$(string.DH_Value2048)">
<value>
<decimal value="2048" />
</value>
</item>
<item displayName="$(string.DH_Value3072)">
<value>
<decimal value="3072" />
</value>
</item>
<item displayName="$(string.DH_Value4096)">
<value>
<decimal value="4096" />
</value>
</item>
</enum>

Of those, only 1024 (as I understand it) is considered weak. Moreover, in the description field for the Diffie-Hellman settings, we have this warning:

<string id="DHServer_Help">Sets the minimum Diffie-Hellman ephemeral key size for TLS servers.
Please see Microsoft Security Advisory 3174644 for more information on DH modulus length. 2048 is the currently recommended minimum value.
</string>

I think the best course of action would be to update the README, since 75% of the values one can choose for the key length are considered safe and we do our best to call out which ones those are. Does that sound okay?

@slm4996
Copy link
Author

slm4996 commented Jan 21, 2023

While I do agree with everything @Crosse has said, leaving Diffie-Hellman active but increasing the minimum key length could lead to lower performance VS ECDH and other key exchanges.

IBM has a good article with relevant information in the "additional information" section of https://www.ibm.com/support/pages/how-disable-ssltls-diffie-hellman-keys-less-2048-bits .

If we do not designate Diffie-Hellman as weak (at least with the default key length of 1024), then much additional information should be added the the Readme to prevent a novice user from making security or performance impacting choices.

Maybe the category shouldn't be "Weak Key Exchanges", but instead "Legacy Key Exchanges"?

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