Fix build issue against LibreSSL #4

Merged
SlavekB merged 1 commits from feat/libressl into master 4 years ago
selk commented 4 years ago
Collaborator

Previous error(s) was:

/opt/trinity/lib/tqt3/include/ntqstring.h:770:15: note: because ‘TQCharRef’ has user-provided ‘TQCharRef TQCharRef::operator=(const TQCharRef&)’
  770 |     TQCharRef operator=(const TQCharRef& c ) { s.ref(p)=c.unicode(); return *this; }
      |               ^~~~~~~~
qca-tls.cpp: In function ‘bool lib_generateKeyIV(const EVP_CIPHER*, const TQByteArray&, const TQByteArray&, TQByteArray*, TQByteArray*, int)’:
qca-tls.cpp:66:24: error: ‘EVP_CIPHER_meth_dup’ was not declared in this scope; did you mean ‘EVP_CIPHER_mode’?
   66 |  EVP_CIPHER *loctype = EVP_CIPHER_meth_dup(_type);
      |                        ^~~~~~~~~~~~~~~~~~~
      |                        EVP_CIPHER_mode
qca-tls.cpp:79:2: error: ‘EVP_CIPHER_meth_free’ was not declared in this scope; did you mean ‘EVP_PKEY_meth_free’?
   79 |  EVP_CIPHER_meth_free(loctype);
      |  ^~~~~~~~~~~~~~~~~~~~
      |  EVP_PKEY_meth_free
make: *** [Makefile:71: qca-tls.o] Error 1
Previous error(s) was: ``` /opt/trinity/lib/tqt3/include/ntqstring.h:770:15: note: because ‘TQCharRef’ has user-provided ‘TQCharRef TQCharRef::operator=(const TQCharRef&)’ 770 | TQCharRef operator=(const TQCharRef& c ) { s.ref(p)=c.unicode(); return *this; } | ^~~~~~~~ qca-tls.cpp: In function ‘bool lib_generateKeyIV(const EVP_CIPHER*, const TQByteArray&, const TQByteArray&, TQByteArray*, TQByteArray*, int)’: qca-tls.cpp:66:24: error: ‘EVP_CIPHER_meth_dup’ was not declared in this scope; did you mean ‘EVP_CIPHER_mode’? 66 | EVP_CIPHER *loctype = EVP_CIPHER_meth_dup(_type); | ^~~~~~~~~~~~~~~~~~~ | EVP_CIPHER_mode qca-tls.cpp:79:2: error: ‘EVP_CIPHER_meth_free’ was not declared in this scope; did you mean ‘EVP_PKEY_meth_free’? 79 | EVP_CIPHER_meth_free(loctype); | ^~~~~~~~~~~~~~~~~~~~ | EVP_PKEY_meth_free make: *** [Makefile:71: qca-tls.o] Error 1 ```
SlavekB requested changes 4 years ago
SlavekB left a comment
Owner

At first glance it looked fine, at second glance the use of conditions is suspiciously inconsistent – they contradict each other. Please examine it.

At first glance it looked fine, at second glance the use of conditions is suspiciously inconsistent – they contradict each other. Please examine it.
qca-tls.cpp Outdated
{
if(type) {
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER < 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
Owner

This is weird. Makes sense the condition older openssl || libressl or condition newer openssl && !libressl – as are above. But the condition older openssl && !libressl is suspicious.

This is weird. Makes sense the condition *older openssl || libressl* or condition *newer openssl && !libressl* – as are above. But the condition *older openssl && !libressl* is suspicious.
qca-tls.cpp Outdated
type = getType(mode);
r.resize(0);
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER < 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
Owner

This is suspicious, just like the previous one.

This is suspicious, just like the previous one.
SlavekB requested changes 4 years ago
SlavekB left a comment
Owner

Reversing the condition is obviously wrong.

Reversing the condition is obviously wrong.
qca-tls.cpp Outdated
}
int res = EVP_BytesToKey(loctype, EVP_sha1(), (unsigned char *)salt.data(), (unsigned char *)data.data(), data.size(), 1, kp, ip);
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
#if OPENSSL_VERSION_NUMBER < 0x10100000L
Owner

Reversing this condition is meaningless. EVP_CIPHER_meth_free documentation says: The functions described here were added in OpenSSL 1.1.0. Reversing this condition is thus in direct contradiction and causes FTBFS.

Reversing this condition is meaningless. `EVP_CIPHER_meth_free` documentation says: The functions described here were added in OpenSSL 1.1.0. Reversing this condition is thus in direct contradiction and causes FTBFS.
SlavekB requested changes 4 years ago
SlavekB left a comment
Owner

There are still broken conditions for which it is necessary to revert it to the original sense.

There are still broken conditions for which it is necessary to revert it to the original sense.
qca-tls.cpp Outdated
unsigned char *kp = 0;
unsigned char *ip = 0;
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER >= 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
Owner

It is undesirable to reverse the meaning of this condition for the OpenSSL version.

It is undesirable to reverse the meaning of this condition for the OpenSSL version.
qca-tls.cpp Outdated
{
if(type) {
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER >= 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
Owner

It is undesirable to reverse the meaning of this condition for the OpenSSL version.

It is undesirable to reverse the meaning of this condition for the OpenSSL version.
SlavekB reviewed 4 years ago
SlavekB left a comment
Owner

Well, now I'm happy with the result – finally – I know I have high demands. However – one more requirement. There's no point in having so many commits. Please squash it into a single commit.

Well, now I'm happy with the result – finally – I know I have high demands. However – one more requirement. There's no point in having so many commits. Please squash it into a single commit.
selk commented 4 years ago
Poster
Collaborator

All ready. Thanks for the patience. :-)

All ready. Thanks for the patience. :-)
SlavekB approved these changes 4 years ago
SlavekB left a comment
Owner

It looks good, I have no further objections.

It looks good, I have no further objections.
Chris commented 4 years ago
Collaborator

@selk: That looks much better than before. Keep up your efforts. 👍

It's good you look at this LibreSSL, musl and other things and test them.

@selk: That looks much better than before. Keep up your efforts. :+1: It's good you look at this LibreSSL, musl and other things and test them.
selk closed this pull request 4 years ago
SlavekB closed this pull request 4 years ago
SlavekB deleted branch feat/libressl 4 years ago
SlavekB added this to the R14.0.8 release milestone 4 years ago
The pull request has been merged as 9cd1493c36.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tqca-tls#4
Loading…
There is no content yet.