ipsec-tools: compile with openssl 1.1, compile fix

[cotequeiroz]

Added compatibility with openssl 1.1, and also fixed a compiler
warning about implicit declaration.

Signed-off-by: Eneas U de Queiroz cote2004-github@yahoo.com

Maintainer: @nmeyerhans, @aTanW
Compile tested: bcm47xx/ramips/x86_64, openwrt master
Run tested: x86_64

[cotequeiroz]

I've just submitted this patch upstream.

[nmeyerhans]

Thanks for sending this in. It'll take some time to review. Can you tell me where you sent it upstream? I don't see anything on the ipsec-tools-devel sourceforge mailing list, which is what upstream uses for communication.

[cotequeiroz]

It's here: https://sourceforge.net/p/ipsec-tools/mailman/ipsec-tools-devel/thread/20180528120513.560-1-cote2004-github%40yahoo.com/#msg36327963. And I did CC you when I sent it. However, with DMARC active, list e-mails may be getting bounces. Should I resend it to Reinoud, who replied to your question about openssl 1.1 support. I though about doing it when I submitted the patch, but decided against it.

[nmeyerhans]

I've successfully compile-tested this on Debian, but not yet on OpenWRT.

I'd love to see upstream comment on this before we merge it, so let's give them some time to do so. I also want to get some more meaningful testing of it.

[nmeyerhans]

gcc on Debian reports errors related to -Werror=maybe-uninitialized on the 4 BIGNUM declarations starting here. The issue is that there's a possibility for the goto end statements on preceding lines to be executed before these declarations, and end can call BN_free() on them. The full errors are:

./crypto_openssl.c: In function 'eay_dh_compute':
./crypto_openssl.c:2390:3: error: 'priv_key' may be used uninitialized in this function [-Werror=maybe-uninitialized]
BN_free(priv_key);
^~~~~~~~~~~~~~~~~
./crypto_openssl.c:2388:3: error: 'pub_key' may be used uninitialized in this function [-Werror=maybe-uninitialized]
BN_free(pub_key);
^~~~~~~~~~~~~~~~
./crypto_openssl.c:2383:5: error: 'p' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (p != NULL)
^
./crypto_openssl.c:2385:5: error: 'BNg' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (BNg != NULL)
^

This can be fixed by moving these 4 declaration such that they occur before the first goto end.

[cotequeiroz]

You're right. I will update.

[cotequeiroz]

I sent the corrected patch upstream right away, but I had forgotten to update this until now.

[nmeyerhans]

Just wanted to follow up so you know this isn't languishing. I've done some testing on Debian and am entirely happy with the results so far, both with OpenSSL 1.1 and 1.0. I don't have any major concerns at this time, but still want to do more testing before I merge.

Also, it doesn't look like upstream has responded at all yet. 😞 It may be worth bugging them again...

[nmeyerhans]

Merged, finally. Still no response from upstream. Remind me to bring up the idea of a fork again someday...

For the record, I uploaded a package including this fix to Debian on June 30, so ipsec-tools will once again be included in the next stable release. It was previously slated to be excluded since openssl 1.0 will not be included in the next version.