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

tls: Use auto DH selection with OpenSSL 1.1.0 and later #5156

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

Conversation

notroj
Copy link

@notroj notroj commented Dec 2, 2024

The current code only uses auto DH parameter selection with OpenSSL 3.0+

SSL_CTX_set_dh_auto was introduced in OpenSSL 1.1.0 in this commit openssl/openssl@09599b5

@dilyanpalauzov
Copy link
Contributor

This needs adjustment:

tls_server_cert. Used by OpenSSL before version 3.0. */

@notroj notroj force-pushed the tls-auto-dh-openssl110 branch from 1b17f97 to 48d35dc Compare December 2, 2024 14:09
@notroj
Copy link
Author

notroj commented Dec 2, 2024

Thanks @dilyanpalauzov - updated

@notroj
Copy link
Author

notroj commented Dec 2, 2024

One other note: the approach taken in the existing code is to ignore any manually configured DH parameters for OpenSSL versions which support SSL_CTX_set_dh_auto.

It would be possible to change the logic to:
a) use manually configured params if present, or else
b) fall back to the automatic logic otherwise.
but it'd be more complex change.

@notroj notroj force-pushed the tls-auto-dh-openssl110 branch from 48d35dc to fcd241a Compare December 6, 2024 10:21
@notroj
Copy link
Author

notroj commented Dec 6, 2024

Rebased to hopefully address the compiler warnings which were triggering CI failures.

@notroj notroj force-pushed the tls-auto-dh-openssl110 branch from fcd241a to b9c94e2 Compare December 10, 2024 13:17
@notroj
Copy link
Author

notroj commented Dec 10, 2024

Apparently too difficult for me to track all the places using that #define combination. I've adjusted it but if there is a preferred coding style here I can change that. Also would welcome any feedback on comment above before iterating further. #5156 (comment)

@dilyanpalauzov
Copy link
Contributor

While here absolutely nothing depends on me, I would prefer if the only supported OpenSSL version by future versions of Cyrus IMAP (so 3.12 and beyond) is OpenSSL 3.0+, as upstream OpenSSL does not support lower versions.

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