Add support for OpenSSL 1.1.0
ClosedPublic

Authored by fvogt on Dec 19 2017, 10:03 PM.

Details

Test Plan

Ran the testsuite with OpenSSL 1.1.0g and 1.0.2j, all passed.
Using this code with kdeconnect and okteta successfully on my system now.

Diff Detail

Repository
R486 QCA Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fvogt requested review of this revision.Dec 19 2017, 10:03 PM
fvogt created this revision.
bero added a subscriber: bero.Dec 19 2017, 10:57 PM
cfeck added a subscriber: cfeck.Dec 20 2017, 3:40 AM
cfeck added inline comments.
unittest/tls/tlsunittest.cpp
94–104

Does TW not have these because they are too strong or because they are too insecure? In the former case, I suggest to keep the tests intact, and use a distribution specific patch to disable those test. In the latter case, I suggest to make the tests fail, if the library was compiled with insecure methods enabled.

fvogt added inline comments.Dec 20 2017, 6:45 AM
unittest/tls/tlsunittest.cpp
94–104

SSL 3 is deprecated: http://disablessl3.com/

So openSUSE disables it completely in openSSL 1.1.0 itself. openSSL 1.0.0 seems to still support SSL 3.

Note that the list of supported ciphers does not come from QCA, but openSSL itself.
So what you're suggesting is to make the QCA testsuite fail if openSSL was not compliled with specific flags, which seems unreasonable.

alonbl added a subscriber: alonbl.Dec 22 2017, 10:42 AM
alonbl added inline comments.
plugins/qca-ossl/ossl110-compat.h
29

you probably what to check this with libressl as in most cases libressl should be excluded

If there's no reply until Wednesday, I'll land this as it'll be two weeks old by then.

plugins/qca-ossl/ossl110-compat.h
29

I don't have libressl here, can you give it a try?

anthonyfieroni added inline comments.
plugins/qca-ossl/qca-ossl.cpp
2755–2764

None free ops and ops->name. This function acts like a static one, but it's not. Can you see its usage? It ops needs to be alive outside class scope?

fvogt added inline comments.Dec 31 2017, 12:00 PM
plugins/qca-ossl/qca-ossl.cpp
2755–2764

It looks like some kind of "optimization".
All instances of QCA_RSA_METHOD share the same RSA_METHOD (line 2738).

IMO this should be changed to be RAII-compatible and then moved into the QCA_RSA_METHOD class as static member. That's out-of-scope of this patch though.

cfeck added inline comments.Dec 31 2017, 1:57 PM
unittest/tls/tlsunittest.cpp
94–104

I still don't understand the reference to openSUSE in the source code. Either the patch is specific to a distribution (then it should be made locally by the distribution), or it is applicable upstream. In that case, referencing a distribution is confusing. Simply mention that OpenSSL 1.1 does no longer support SSL 3, because it is deprecated.

Additionally, why not remove the code completely instead of commenting it? If you feel someone would have the need to enable them, add a proper if() around the code.

fvogt added inline comments.Dec 31 2017, 1:59 PM
unittest/tls/tlsunittest.cpp
94–104

This is just for consistency, previous commits did the exact same: Comment out a couple of lines and add "Fedora does not support this".

This revision was not accepted when it landed; it landed in state Needs Review.Jan 4 2018, 7:02 PM
This revision was automatically updated to reflect the committed changes.