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

Implemented OpenSSL 1.1.0 TLS methods and deprecated SSLv23 ones. #2231

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

akotulu
Copy link
Contributor

@akotulu akotulu commented Apr 30, 2024

SSLv23_method(), SSLv23_server_method() and SSLv23_client_method() were deprecated and the preferred TLS_method(), TLS_server_method() and TLS_client_method() functions were added in OpenSSL 1.1.0.

All version-specific methods were deprecated in OpenSSL 1.1.0.

SSLv23_method

@jwillemsen
Copy link
Member

Doesn't this change break any user code which uses the SSLv23 ones?

@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Here is quote from docs.

SSLv23_method(), SSLv23_server_method(), SSLv23_client_method()
These functions do not exist anymore, they have been renamed to TLS_method(), TLS_server_method() and TLS_client_method() respectively. Currently, the old function calls are renamed to the corresponding new ones by preprocessor macros, to ensure that existing code which uses the old function names still compiles. However, using the old function names is deprecated and new code should call the new functions instead.

@jwillemsen
Copy link
Member

Ok, but you do change the enum provided by ACE in such a way that when someone uses it, it will not compile unless they change their code

@jwillemsen
Copy link
Member

#958

@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Ok, I will add the enum values back with corresponding TLS calls.

@jwillemsen
Copy link
Member

jwillemsen commented Apr 30, 2024

Mark the old enums as deprecated so that we know they are deprecated and can be removed at some point, maybe use the C++14 https://en.cppreference.com/w/cpp/language/attributes/deprecated, C++17 is now required for ACE/TAO

@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Made the changes. Is it ok now?

@jwillemsen
Copy link
Member

What when ssl version is smaller as 0x10100000L, than the new defines are there. Also in ACE_SSL_Context::load_trusted_ca the old enums are not tested

@jwillemsen jwillemsen added the needs review Needs to be reviewed label Apr 30, 2024
@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Huh, sorry about the mess. Haven't done such a backwards compatibility stuff before.
It should now be ok.

@jwillemsen
Copy link
Member

When < 0x10100000L the enums will still give a deprecated warning

@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Idk if this is the best way to remove the deprecation warning, but here it is.

@jwillemsen
Copy link
Member

Please fix fuzz errors

@akotulu
Copy link
Contributor Author

akotulu commented Apr 30, 2024

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Needs to be reviewed
Development

Successfully merging this pull request may close these issues.

2 participants