Improve OpenSSL library version detection #938

2017-01-10T03:45:02+01:00

wfr commented

2017-01-10 03:45:02 +01:00

(Migrated from github.com)

PyBitmessage depends on OpenSSL 1.0 but some GNU/Linux systems have
multiple versions installed. Try to locate the correct version.

https://bitmessage.org/forum/index.php/topic,5280.msg11431.html

PyBitmessage depends on OpenSSL 1.0 but some GNU/Linux systems have multiple versions installed. Try to locate the correct version. https://bitmessage.org/forum/index.php/topic,5280.msg11431.html

PeterSurda commented

2017-01-10 20:55:14 +01:00

(Migrated from github.com)

I don't see anything obviously wrong with it, give me a couple days for a closer code review and testing.

bmng-dev commented

2017-01-11 02:27:45 +01:00

(Migrated from github.com)

@PeterSurda perhaps you should check the landscape.io integration because landscape does highlight an obvious issue which looks like a showstopper on Windows.

Landscape.io Code Review for Pull Request #938

Edit: It seems only Landscapes successes are showing up. There should be brown dots when a check is pending and red X's if there are issues with a commit/pull request. This is possibly a configuration issue as these indicators appear on other repositories that use Landscape.

@PeterSurda perhaps you should check the landscape.io integration because landscape does highlight an obvious issue which looks like a showstopper on Windows. [Landscape.io Code Review for Pull Request #938](https://landscape.io/github/Bitmessage/PyBitmessage/pulls/938) Edit: It seems only Landscapes successes are showing up. There should be brown dots when a check is pending and red X's if there are issues with a commit/pull request. This is possibly a configuration issue as these indicators appear on other repositories that use Landscape.

bmng-dev commented

2017-01-11 03:22:03 +01:00

(Migrated from github.com)

@wfr that was fast. I don't see any other issues preventing this being accepted as it is.

PeterSurda commented

2017-01-12 19:46:35 +01:00

(Migrated from github.com)

Wouldn't it be better to support both 1.0.x and 1.1.0? If SSLeay_version fails, try OpenSSL_version and analogously with SSLeay -> OpenSSL_version_num. Or did something else change in 1.1.0? I don't have a suitable VM available now, can you test it? src/bitmessageqt/support.py would also have to be changed analogously.

Wouldn't it be better to support both 1.0.x and 1.1.0? If `SSLeay_version` fails, try `OpenSSL_version` and analogously with `SSLeay` -> `OpenSSL_version_num`. Or did something else change in 1.1.0? I don't have a suitable VM available now, can you test it? `src/bitmessageqt/support.py` would also have to be changed analogously.

PeterSurda commented

2017-01-12 22:13:24 +01:00

(Migrated from github.com)

So I checked it with Debian testing. It looks like there are more changed in OpenSSL 1.1.0 that make pyelliptic incompatible:

ECDH_OpenSSL -> EC_KEY_OpenSSL
ECDH_set_method -> EC_KEY_set_method
EVP_CIPHER_CTX_cleanup -> EVP_CIPHER_CTX_reset
EVP_MD_CTX_create -> EVP_MD_CTX_new
EVP_MD_CTX_init -> EVP_MD_CTX_reset
EVP_MD_CTX_destroy -> EVP_MD_CTX_free

Just renaming these looks sufficient. The problem is:

EVP_ecdsa was deprecated and the relevant code needs to be rewritten to use the 'ecdsa-with-SHA1' method manually.

Anyone would like to fix that?

So I checked it with Debian testing. It looks like there are more changed in OpenSSL 1.1.0 that make pyelliptic incompatible: - `ECDH_OpenSSL` -> `EC_KEY_OpenSSL` - `ECDH_set_method` -> `EC_KEY_set_method` - `EVP_CIPHER_CTX_cleanup` -> `EVP_CIPHER_CTX_reset` - `EVP_MD_CTX_create` -> `EVP_MD_CTX_new` - `EVP_MD_CTX_init` -> `EVP_MD_CTX_reset` - `EVP_MD_CTX_destroy` -> `EVP_MD_CTX_free` Just renaming these looks sufficient. The problem is: - `EVP_ecdsa` was deprecated and the relevant code needs to be rewritten to use the `'ecdsa-with-SHA1'` method manually. Anyone would like to fix that?

PeterSurda commented

2017-01-12 23:30:56 +01:00

(Migrated from github.com)

So I tested it and other than the singing / signature verification it seems to work. If someone could fix that, that would be great.

PeterSurda commented

2017-01-13 10:11:37 +01:00

(Migrated from github.com)

Well, good news, EVP_ecdsa can be replaced with EVP_sha1. I tried it and it works both for signing and verification. So I'll provide my own patch instead and you can keep using OpenSSL 1.1.x.

Well, good news, `EVP_ecdsa` can be replaced with `EVP_sha1`. I tried it and it works both for signing and verification. So I'll provide my own patch instead and you can keep using OpenSSL 1.1.x.

This repo is archived. You cannot comment on pull requests.

No reviewers