OpenSSL 1.1.x compatibility

[manover]

Netatalk currently does not build against OpenSSL 1.1.0+, producing
a half a dozen error messages like this:

error: dereferencing pointer to incomplete type ‘DH {aka struct dh_st}’

This is caused by OpenSSL making certain structs opaque, which forces the library users to employ special getters and setters when accessing these structs.
This PR makes Netatalk build against the new version of OpenSSL, but it cannot be merged as-is because it breaks compatibility with the older versions. It could be easily fixed with a couple of ifdef-s,
however I am not certain this project wants to hardcode external dependencies versions in the core code, so maybe there is a better way through m4 configure scripts, for example.

Please review.

[ghost]

I think the way of the following URL is no problem.
https://wiki.openssl.org/index.php/1.1_API_Changes

#if OPENSSL_VERSION_NUMBER < 0x10100000L

[ghost]

I tested your patch on Fedora rawhide (OpenSSL 1.1.0d).
Both of uams_dhx_pam.so and uams_dhx_passwd.so work right.

By the way, I think "#if OPENSSL_VERSION_NUMBER" is better than m4,
because it's simple.

Can you write new patch?

[ghost]

discussing on netatalk-devel ML.
https://sourceforge.net/p/netatalk/mailman/message/35677464/

[manover]

@hat001 Thanks for your review, Actually, my first though was to ifdef based on OpenSSL version
#if OPENSSL_VERSION_NUMBER < 0x10100000L, however after grepping through the entire code base I found not a single instance off OPENSSL_VERSION_NUMBER, which made me doubt my decision. You know it's always better for large projects to stick to clearly defined guidelines than spending developer-years clearing the fall-out and paying technical debt. So you are obviously long-term contributor and a member of the project and know better, your words enough for me. I'lll post a follow-up PR.
Also, please a close look of graceful resource deallocaton, I think I did it right, but another pair of eyes would not hurt.
As for the czech version you guys found, you don't need Czech to understand the code and that code has few nasty problems, I'll address them later.
And, by the way, thanks for your link regarding OpenSSL API changes. The patch itself took only 10 minutes, but I wasted 5 hours trying to avoid pulling in private key data from internal structures. I am almost obsessive about the best possible way of doing things, so you helped me out a lot.
Thank you.

[manover]

@hat001 It builds on OpenSSL 1.0.2 now, please review

[ghost]

git comment:

error: dereferencing pointer to incomplete type ‘DH {aka struct dh_st}’

quote sign is not ASCII.

[ghost]

On Fedora 25 (1.0.2k):

uams_dhx_passwd.c: In function 'pwd_login':
uams_dhx_passwd.c:144:10: warning: implicit declaration of function 'DH_set0_pqg' [-Wimplicit-function-declaration]
     if (!DH_set0_pqg(dh, pbn, NULL, gbn)) {
          ^~~~~~~~~~~
uams_dhx_passwd.c:154:5: warning: implicit declaration of function 'DH_get0_key' [-Wimplicit-function-declaration]
     DH_get0_key(dh, &pub_key, NULL);
     ^~~~~~~~~~~
  CCLD     uams_dhx_passwd.la
  CC       uams_dhx_pam_la-uams_dhx_pam.lo
uams_dhx_pam.c: In function 'dhx_setup':
uams_dhx_pam.c:241:10: warning: implicit declaration of function 'DH_set0_pqg' [-Wimplicit-function-declaration]
     if (!DH_set0_pqg(dh, pbn, NULL, gbn)) {
          ^~~~~~~~~~~
uams_dhx_pam.c:264:2: warning: implicit declaration of function 'DH_get0_key' [-Wimplicit-function-declaration]
  DH_get0_key(dh, &pub_key, NULL);
  ^~~~~~~~~~~

[ghost]

On Fedora 25 (1.0.2k):

Feb 22 01:02:12 pfedora25s64 afpd[21488]: uam_load(uams_dhx_pam.so): failed to load: /usr/local/lib/netatalk//uams_dhx_pam.so: undefined symbol: DH_set0_pqg
Feb 22 01:02:12 pfedora25s64 afpd[21488]: uam: uams_dhx_pam.so load failure

[manover]

I'll fix it, np.
BTW, what about non-ascii codes in git comment? Do you want me to rebase the branch removing them? The only reason I put that message in the comment verbatim is to allow people google it by the exact message given by gcc.

[ghost]

your patch file:

From d6d6b31af5ca15d37c625fbcf873bb02b8e0259e Mon Sep 17 00:00:00 2001
From: Denis Bychkov <manover@gmail.com>
Date: Sat, 18 Feb 2017 23:00:10 -0500
Subject: [PATCH 1/2] Netatalk currently does not build against OpenSSL 1.1.0+,
 producing a half a dozen error messages like this:
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

error: dereferencing pointer to incomplete type ‘DH {aka struct dh_st}’

[ghost]

See
DH {aka struct dh_st}

[manover]

ok, got it

[manover]

@hat001 Fixed and, hopefully, thoroughly tested this time. Built against 1.0.2k and 1.1.0e OpenSSL versions.
Please review and if you are happy with the code, I'll squash it to 1 commit to look pretty in the history and get rid of non-ascii characters in git comments.

[ghost]

@manover
tested on Fedora25 (1.0.2k) and Fedora rawhide (1.1.0d).
Both of uams_dhx_pam.so and uams_dhx_passwd.so work well.

[manover]

@hat001 rebased & squashed, ready to merge

[ghost]

@manover
There are neither license nor copyright in openssl_compat.h .
Is this GPLv2?

[manover]

@hat001 done

[slowfranklin]

Code look good. Thanks for contributing!

Can you please adjust the commit message as described here:
Netatalk Review Process

Ie, the commit message subject should be a short one line summary and at the end of the commit message please add your signed-off.

[ghost]

committed!

master:
b4a8025

branch-netatalk-3-1:
d3855e7

[manover]

@slowfranklin sorry, @hat001 had to do it for me, should have read the document before creating the PR. What now, should I close the github pull request, since, it's technically merged?

[slowfranklin]

On Tue, Mar 14, 2017 at 07:39:36PM -0700, Denis Bychkov wrote: @slowfranklin sorry, @hat001 had to do it for me, should have read the document before creating the PR.

no problem at all. You probably didn't know it existed in the first place. :)

What now, should I close the github pull request, since, it's technically merged?

yep, thanks! Cheerio! -slow

[manover]

Yey! Merged! (by a way of cherry-picking).