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

Support OpenSSL 1.1.0 #2223

Merged
merged 1 commit into from Dec 11, 2017
Merged

Conversation

CyberShadow
Copy link
Collaborator

Fixes #2151.

m_iPubKeyBits = EVP_PKEY_bits(p);
m_szPubKeyType = (p->type == NID_undef) ? __tr("Unknown") : OBJ_nid2ln(p->type);
m_szPubKeyType = (type == NID_undef) ? __tr("Unknown") : OBJ_nid2ln(type);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure about this one. There is no longer a way to get p->type - EVP_PKEY_base_id(p) replaces EVP_PKEY_type(p->type), however I suspect that the original code was wrong and this was the intent.

@CyberShadow CyberShadow changed the title Support OpesSSL 1.1.0 Support OpenSSL 1.1.0 May 24, 2017
@ctrlaltca
Copy link
Contributor

ctrlaltca commented May 24, 2017

Travis currently bundles openssl 1.0.1f, so compilation failed because new functions implemented in 1.1 are used in this PR, eg. EVP_MD_CTX_new.

The easiest solution is probably to #if OPENSSL_VERSION_NUMBER < 0x10100000L .. #else ... #endif calls to new functions.

KVIrc depends on Qt that is linked to OpenSSL itself; this needs a bit of investigation, will it work if KVIrc is linked to OpenSSL 1.1 while Qt still links to a previous version?

@CyberShadow
Copy link
Collaborator Author

Travis currently bundles openssl 1.0.1f, so compilation failed because new functions implemented in 1.1 are used in this PR, eg. EVP_MD_CTX_new.

Ah crud, those were renamed in 1.1.0. Fixed.

KVIrc depends on Qt that is linked to OpenSSL itself; this needs a bit of investigation, will it work if KVIrc is linked to OpenSSL 1.1 while Qt still links to a previous version?

I don't know. You may want to detect that in the CMake script.

With this PR, it builds and works fine for me (on Arch Linux).

@Dessa
Copy link
Member

Dessa commented Jun 2, 2017

see this: mumble-voip/mumble@dff1557
and this: mumble-voip/mumble@f544524

@CyberShadow
Copy link
Collaborator Author

CyberShadow commented Aug 11, 2017

Ping

As I said above, everything works fine on Arch Linux. Any additional changes for distributions in which Qt links to an older OpenSSL would need to be looked into by someone else.

@CyberShadow
Copy link
Collaborator Author

Ping.

Is there any actionable feedback to be addressed here?

Are the Qt concerns theoretical, or is anyone actually having trouble with this patch? We shouldn't attempt to fix what we can't even reproduce.

@ctrlaltca
Copy link
Contributor

I agree this one could me merged, as it:

  • doesn't break any existing configuration
  • adds support for a new version of a library KVIrc depends on

@ctrlaltca ctrlaltca merged commit ed2a156 into kvirc:master Dec 11, 2017
@CyberShadow
Copy link
Collaborator Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants