Differential D6665

Make kssl compile against OpenSSL 1.1.0
ClosedPublic
Actions

Authored by dvratil on Jul 13 2017, 12:45 AM.

Details

Reviewers

dfaure

Group Reviewers

Frameworks

Commits

R239:9a990c69c606: Make kssl compile against OpenSSL 1.1.0

Summary

OpenSSL 1.1.0 contains some source-incompatible changes, most notably making most of the
structures opaque and introducing new getter/setter functions to modify the structures. This
patch adds some of the newly introduced functions to the KOpenSSL class and modifies the code to call them. The implementation of those newly introduced methods contains both OpenSSL < 1.1 compatible code (direct structure member access) and calls to real functions resolved from OpenSSL>= 1.1 library. Which implementation is used is decided at compile time. Some of the existing methods were renamed to match the OpenSSL 1.1 naming and to avoid conflicts with backward-compatibility names provided by OpenSSL 1.1.

KSSLCertificate::toNetscape() returns empty result when built against OpenSSL 1.1 since I wasn't able to find a proper equivalent in OpenSSL 1.1 API (and there does not seem to be any).

BUG: 370223

Test Plan

The code compiles under both OpenSSL 1.1 and OpenSSL 1.0.x. I did not test the actual functionality.

Diff Detail

Repository

R239 KDELibs4Support

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

dvratil created this revision.Jul 13 2017, 12:45 AM

rdieter added a subscriber: rdieter.Jul 13 2017, 4:02 AM

dvratil edited the summary of this revision. (Show Details)Jul 13 2017, 8:31 AM

Thanks for this work, but I can't review it, I don't know anything about openssl APIs or this code.

Does it require Qt to be compiled with the same version of openssl?

@dfaure neither do I :-) The patch does not require understanding of the API, I just want a formal review. Feel free to add someone who knows openssl if you want - no-one really touched the code for 10 years, so I don't know who to add for review.

@ltoscano Not sure. If both Qt and KSSL use dlsym() to resolve symbols from the library instead of linking it directly, then there should be no problem? But it's probably safer to use the same versions in both.

fvogt added a subscriber: fvogt.Aug 24 2017, 1:04 PM

I added this patch to the openSUSE packages for testing, it seems to work fine so far, with both openSSL 1.1 and 1.0. I only tested kcm_ssl though, I'm not aware of anything else using it...

So archlinux is using https://git.archlinux.org/svntogit/packages.git/tree/trunk/kdelibs4support-openssl-1.1.patch?h=packages/kdelibs4support

@arojas why did you guys not try to contribute that?

No idea if it can be used to at least compare your version with theirs to see how "good" it is.

In D6665#141883, @aacid wrote:

So archlinux is using https://git.archlinux.org/svntogit/packages.git/tree/trunk/kdelibs4support-openssl-1.1.patch?h=packages/kdelibs4support

@arojas why did you guys not try to contribute that?

No idea if it can be used to at least compare your version with theirs to see how "good" it is.

Because it's not our patch, we took it from OpenMandriva, which is the only one I could find at the time.

One important difference is that OM's patch makes kdelibs4support link to openssl, unlike the current code (and this patch).

I would say that the testing by @fvogt is enough, and given that most distributions are trying to phase out OpenSSL 1.0, and that they are compiling Qt with OpenSSL 1.1, I would suggest to push this and see fix any breakages later.
It's low risk, as those functions from KDELibs4Support are probably not the most used around (and it's a porting-frameworks only anyway).

@dfaure , what do you think?

Last chance before 5.39 :)

I'm too clueless to review this, but I'd say, it's too late to put this into 5.39. Better have it in master for a few weeks so that developers can catch problems on their various distros before this gets released.

Now is the right time ;)

This revision is now accepted and ready to land.Oct 16 2017, 7:52 AM

Closed by commit R239:9a990c69c606: Make kssl compile against OpenSSL 1.1.0 (authored by dvratil). · Explain WhyOct 17 2017, 7:43 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/kssl/kopenssl.h (76 lines)
M			src/kssl/kopenssl.cpp (306 lines)
M			src/kssl/kssl.cpp (4 lines)
M			src/kssl/ksslcallback.c (4 lines)
M			src/kssl/ksslcertchain.cpp (51 lines)
M			src/kssl/ksslcertificate.cpp (68 lines)