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

remove deprecate warnings when using openssl v3 #1826

Merged

Conversation

MarcelJordense
Copy link
Contributor

Some of the Openssl v1.1.1 function used by the authentication security plugin have become deprecated in Openssl v3. These functions have been replaced by the corresponding function provided by Openssl v3.

@MarcelJordense MarcelJordense force-pushed the update_security_to_opensslv3 branch from c8edcd4 to d9eba58 Compare September 14, 2023 07:53
@MarcelJordense MarcelJordense force-pushed the update_security_to_opensslv3 branch from d9eba58 to 13e6185 Compare March 6, 2024 14:30
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

I think I should've merged this a long time ago already ... or at least reviewed it properly because I do have two questions/remarks. They're totally trivial details, to be honest.

Now that the CI is working again on Windows on master — the problems were caused by a combination of CMake, OpenSSL 3.2.1 and Chocolatey — it would be also be nice to see those builds green with this PR.

Perhaps you would be willing to consider my two comments, possibly update the code and rebase it once more to master?

@@ -38,7 +38,7 @@
#include <WinSock2.h>
#endif

#define OPENSSL_API_COMPAT 10101
#define OPENSSL_API_COMPAT 30000
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this should be defined to 10101 when using OpenSSL < 3.0.0, but I checked and OpenSSL 1.1.1 doesn't care if it is set too high. Making it conditional on the OpenSSL version number seems overkill, but a comment to this effect might be useful.

@MarcelJordense MarcelJordense force-pushed the update_security_to_opensslv3 branch from 13e6185 to c014f8e Compare March 19, 2024 13:05
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

LGTM, but now there is an issue with the email address used in the commits: the short one is registered with the Eclipse Foundation, the long one isn't. Could you please edit the commits to use the correct address? Thank you!

@MarcelJordense MarcelJordense force-pushed the update_security_to_opensslv3 branch from c014f8e to 37baf0c Compare March 22, 2024 13:18
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

Thanks!

@eboasson eboasson merged commit 648a898 into eclipse-cyclonedds:master Mar 25, 2024
18 of 22 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.

2 participants