Bug 229021 - devel/p4 Fails to build with OpenSSL 1.1
Summary: devel/p4 Fails to build with OpenSSL 1.1
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Alan Somers
URL:
Keywords:
: 232214 (view as bug list)
Depends on:
Blocks: 228865
  Show dependency treegraph
 
Reported: 2018-06-14 19:57 UTC by Bernard Spil
Modified: 2018-11-18 16:10 UTC (History)
4 users (show)

See Also:


Attachments
svn-diff-p4 (14.33 KB, patch)
2018-10-28 02:29 UTC, Walter Schwarzenfeld
no flags Details | Diff
Fix the build on 12.0-BETA3 (948 bytes, patch)
2018-11-06 21:16 UTC, Alan Somers
no flags Details | Diff
Patch to fix the build with OpenSSL 1.1.1 (541 bytes, patch)
2018-11-06 22:06 UTC, Kenneth D. Merry
no flags Details | Diff
Fix the build on 12.0-BETA3 (964 bytes, patch)
2018-11-07 00:15 UTC, Alan Somers
no flags Details | Diff
Fix build by conditionally applying Ken's patch (1.93 KB, patch)
2018-11-08 01:44 UTC, Alan Somers
no flags Details | Diff
New version of patch that works on old and new OpenSSL versions (798 bytes, patch)
2018-11-08 18:23 UTC, Kenneth D. Merry
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bernard Spil freebsd_committer freebsd_triage 2018-06-14 19:57:17 UTC
> p4-bin/librpc.a(netssltransport.o): In function `NetSslTransport::SslClientInit(Error*)':
> net/netssltransport.cc:(.text+0x3ce): undefined reference to `SSL_load_error_strings'
> net/netssltransport.cc:(.text+0x4c2): undefined reference to `SSL_library_init'

During BSDCan 2018 the intention to update OpenSSL in base to 1.1.x branch was documented.

Intention is to update 12-STABLE to current 1.1.0 and subsequently update it to 1.1.1 when that is released. 

Poudriere log: https://keg.brnrd.eu/data/111amd64-default-openssl110/2018-06-11_10h42m37s/logs/errors/p4-2016.1.1492381_2.log
Comment 1 Nathan 2018-09-16 02:52:41 UTC
Do you mean devel/p4 ?
Comment 2 Alan Somers freebsd_committer freebsd_triage 2018-10-26 19:13:27 UTC
*** Bug 232214 has been marked as a duplicate of this bug. ***
Comment 3 Kenneth D. Merry freebsd_committer freebsd_triage 2018-10-26 19:45:24 UTC
It looks like we're still using 2016.1, and they have 2018.1 up.

Have we confirmed that the latest version also won't work with OpenSSL 1.1.1?
Comment 4 Alan Somers freebsd_committer freebsd_triage 2018-10-26 19:58:18 UTC
I've tried every newer version of p4.  2018.1 is actually worse.  I think our options are:
1) Ask P4 for an update (Ken, you would have to be the one to do this), or
2) Modify the port to link against ports' OpenSSL.
Comment 5 Kenneth D. Merry freebsd_committer freebsd_triage 2018-10-26 20:02:30 UTC
Ok.  I just sent email to Perforce support.  I explained that they would have this problem with all platforms, not just FreeBSD.
Comment 6 Kenneth D. Merry freebsd_committer freebsd_triage 2018-10-26 21:22:24 UTC
Here is what I got from Perforce support:

Hi Ken,

There is an existing enhancement request for this:

job095008 - Update OpenSSL to the 1.1 codeline

The developer notes that the 1.0.2 release will loose support on 2019-12-31.

I have attached your contact information to this request, so you will be notified when it is implemented.

Please let me know if you need any further assistance.

-Norman
Comment 7 Bernard Spil freebsd_committer freebsd_triage 2018-10-27 23:38:30 UTC
(In reply to Kenneth D. Merry from comment #6)

> The developer notes that the 1.0.2 release will loose support on 2019-12-31.

That is the current plan for OpenSSL.
Comment 8 Walter Schwarzenfeld freebsd_triage 2018-10-28 02:29:43 UTC
Created attachment 198708 [details]
svn-diff-p4

Alan Somers said: 2018.1 is actually worse:
We need only to patch one file:: netssltransport.cc.

I patched against it. 
If you think that makes no sense, obsolete it. But it builds on 11.2 with openssl111.
Comment 9 Alan Somers freebsd_committer freebsd_triage 2018-10-28 03:28:54 UTC
w.schwarzenfeld@utanet.at your patch doesn't work for me, on either 11.2 or 12.0-ALPHA9.  Here are the errors on 12.0-ALPHA9:

net/netsslcredentials.cc:221:16: error: member access into incomplete type 'EVP_PKEY'
      (aka 'evp_pkey_st')
        if (privateKey->type != EVP_PKEY_RSA)

net/netssltransport.cc:117:36: error: use of undeclared identifier 'CRYPTO_NUM_LOCKS'
static pthread_mutex_t mutexArray[ CRYPTO_NUM_LOCKS ];
                                   ^
net/netssltransport.cc:259:18: error: out-of-line definition of                       
      'CreateAndInitializeSslContext' does not match any declaration in
      'NetSslTransport'
NetSslTransport::CreateAndInitializeSslContext(const char *conntypename)
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/netssltransport.cc:267:5: error: use of undeclared identifier                     
      'TRANSPORT_PRINT_VAR'
    TRANSPORT_PRINT_VAR( SSLDEBUG_FUNCTION, msgbuf );
    ^
net/netssltransport.cc:296:5: error: use of undeclared identifier                     
      'TRANSPORT_PRINT_VAR'
    TRANSPORT_PRINT_VAR( SSLDEBUG_FUNCTION, msgbuf );
    ^
net/netssltransport.cc:320:33: error: use of undeclared identifier                    
      'P4TUNE_SSL_TLS_VERSION_MIN'
    int tlsmin = p4tunable.Get( P4TUNE_SSL_TLS_VERSION_MIN );
                                ^
net/netssltransport.cc:321:33: error: use of undeclared identifier                    
      'P4TUNE_SSL_TLS_VERSION_MAX'
    int tlsmax = p4tunable.Get( P4TUNE_SSL_TLS_VERSION_MAX );
net/netssltransport.cc:890:19: error: no member named 'SslProtocolError' in 'MsgRpc'  
                e->Set( MsgRpc::SslProtocolError ) << sslErrorBuf;
                        ~~~~~~~~^
net/netssltransport.cc:1053:30: error: no member named 'PollMs' in 'KeepAlive'        
                    int p = breakCallback->PollMs();
                            ~~~~~~~~~~~~~  ^


And here are the errors on 11.2:

net/netssltransport.cc:259:18: error: out-of-line definition of 'CreateAndInitializeSslContext' does not match any declaration in 'NetSslTransport'
NetSslTransport::CreateAndInitializeSslContext(const char *conntypename)
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/netssltransport.cc:267:5: error: use of undeclared identifier 'TRANSPORT_PRINT_VAR'
    TRANSPORT_PRINT_VAR( SSLDEBUG_FUNCTION, msgbuf );
    ^
net/netssltransport.cc:296:5: error: use of undeclared identifier 'TRANSPORT_PRINT_VAR'
    TRANSPORT_PRINT_VAR( SSLDEBUG_FUNCTION, msgbuf );
net/netssltransport.cc:320:33: error: use of undeclared identifier 'P4TUNE_SSL_TLS_VERSION_MIN'
    int tlsmin = p4tunable.Get( P4TUNE_SSL_TLS_VERSION_MIN );
net/netssltransport.cc:321:33: error: use of undeclared identifier 'P4TUNE_SSL_TLS_VERSION_MAX'
    int tlsmax = p4tunable.Get( P4TUNE_SSL_TLS_VERSION_MAX );
                                ^
net/netssltransport.cc:890:19: error: no member named 'SslProtocolError' in 'MsgRpc'
                e->Set( MsgRpc::SslProtocolError ) << sslErrorBuf;
                        ~~~~~~~~^
net/netssltransport.cc:1053:30: error: no member named 'PollMs' in 'KeepAlive'
                    int p = breakCallback->PollMs();
                            ~~~~~~~~~~~~~  ^

Are you suggesting that with your patch, p4 should use the openssl from ports?  If so, that requires patching the Makefile, too.
Comment 10 Walter Schwarzenfeld freebsd_triage 2018-10-28 03:52:25 UTC
Sorry, I tested with the wrong "version" (I forgot a change I made). You are right.
Comment 11 Alan Somers freebsd_committer freebsd_triage 2018-11-06 21:16:36 UTC
Created attachment 199027 [details]
Fix the build on 12.0-BETA3

This patch fixes the build on 12.0-BETA3 and 11.2-RELEASE.  But I don't have the ability to test it at runtime.  Can one of the CCd people test it?  If not, I'll commit anyway in a few days.
Comment 12 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-06 22:04:36 UTC
(In reply to Alan Somers from comment #11)
The patch doesn't build for me.

The first problem is that it uses .if ${OSVERSION} before including <bsd.port.pre.mk> in the makefile.

I moved that up, and that fixed it.

Then, at least as far as I can tell, the OpenSSL include line doesn't seem to be having an effect.  I still get a failure here on the build:

net/netsslcredentials.cc:221:16: error: member access into incomplete type
      'EVP_PKEY' (aka 'evp_pkey_st')
        if (privateKey->type != EVP_PKEY_RSA)
                      ^
/usr/include/openssl/ossl_typ.h:93:16: note: forward declaration of
      'evp_pkey_st'
typedef struct evp_pkey_st EVP_PKEY;
               ^

I will attach a patch that fixes the build for me with OpenSSL 1.1.1 in the base on a stable/12 box.  It's a one line fix for the change above.  I have my doubts that it'll build on older versions of OpenSSL, though.
Comment 13 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-06 22:06:06 UTC
Created attachment 199028 [details]
Patch to fix the build with OpenSSL 1.1.1

This patch fixes the build for me with OpenSSL 1.1.1 in stable/12.  I have my doubts that it'll build with older versions of OpenSSL.
Comment 14 Nathan 2018-11-06 22:49:44 UTC
(In reply to Kenneth D. Merry from comment #13)
if you use if statements around new abi and old abi, it will work for both
Comment 15 Alan Somers freebsd_committer freebsd_triage 2018-11-06 23:39:40 UTC
Ken, it's very odd that my patch doesn't fix the build for you, because it works in Poudriere.  Is there anything unusual about your environment or your ports tree?
Comment 16 Alan Somers freebsd_committer freebsd_triage 2018-11-07 00:15:36 UTC
Created attachment 199033 [details]
Fix the build on 12.0-BETA3

Ok, I can reproduce the problem with OSVERSION when I build by hand, just not with poudriere.  And moving that section downward fixes the problem.  But I don't know why the SSLINCLUDE line has no effect for you.  Could you show me what your compile line looks like?  Just run "make", then scroll up and copy any line of compiler output, like this:

clang++ -c -o p4-bin/objects/net/netsslcredentials.o -O2 -pipe -fstack-protector -fno-strict-aliasing   -DNDEBUG -Wno-parentheses -Wno-switch -Wreturn-type -Werror=return-type -pipe -I/usr/local/include -O2 -Wno-parentheses -Wno-switch -fwrapv -DOS_FREEBSD -DOS_FREEBSD130 -DOS_FREEBSDX86_64 -DOS_FREEBSD130X86_64 -DUSE_SSL -Inet -Izlib -Imsgs -Isupport -Isys net/netsslcredentials.cc
Comment 17 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-07 16:30:15 UTC
Here is the build line for netsslcredentials.cc

clang++ -c -o p4-bin/objects/net/netsslcredentials.o -O2 -pipe -fstack-protector -fno-strict-aliasing   -DNDEBUG -Wno-parentheses -Wno-switch -Wreturn-type -Werror=return-type -pipe -I -O2 -Wno-parentheses -Wno-switch -fwrapv -DOS_FREEBSD -DOS_FREEBSD120 -DOS_FREEBSDX86_64 -DOS_FREEBSD120X86_64 -DUSE_SSL -Inet -Izlib -Imsgs -Isupport -Isys net/netsslcredentials.cc

So, the include line isn't getting in there for some reason.

I am building it by hand, and not from poudriere in this instance, although normal builds for us are via poudriere.

Are you building the 2016.1 perforce, which is what is currently checked into the ports tree?  Or are you building a newer version?

It looks like you're building on head, and I'm building on stable/12.

There is a -I with no argument where you have -I/usr/local/include.  Hmm...
Comment 18 Alan Somers freebsd_committer freebsd_triage 2018-11-07 16:32:49 UTC
I've been building 2016.1, and I've been building by hand on head, and with Poudriere for 12.0-BETA3 and 11.2-RELEASE.
Comment 19 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-07 19:03:34 UTC
Ok, obviously it's not picking up the include for some reason in my setup.

One issue in solving the problem this way (depending on an older version of OpenSSL in ports) is this:

======
Dependency error: This port wants the OpenSSL library from the FreeBSD
base system. You can't build against it, while a newer
version is installed by a port.
Please deinstall the port, remove DEFAULT_VERSIONS=ssl=base or undefine WITH_OPENSSL_BASE.
*** Error code 1

Stop.
make[1]: stopped in /usr/home/kenm/perforce_redline_portstest/ports/devel/staf
*** Error code 1
======

I also need to get devel/staf working with the new OpenSSL.  (It's broken.)  I got that after the p4 makefile caused the older version of OpenSSL to get installed.

The question here is which is incorrect?  The staf makefile, or having an OpenSSL port installed?  It seems like it should be the former, but my concern is are there other ports that do that?
Comment 20 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-07 19:40:27 UTC
I also get the same error in the p4 port, when I put things in the makefile in this order, and make with openssl already installed:

# p4 links libssl and libcrypto statically, so specify :build
USES=           ssl:build

.include <bsd.port.pre.mk>

.if ${OSVERSION} >= 1200000
OPENSSLBASE=    ${LOCALBASE}
BUILD_DEPENDS+= ${LOCALBASE}/lib/libssl.so.9:security/openssl
.endif


That yields:

{blackpearl:/usr/home/kenm/perforce_redline_portstest/ports/devel/p4:!:0} make
Dependency error: This port wants the OpenSSL library from the FreeBSD
base system. You can't build against it, while a newer
version is installed by a port.
Please deinstall the port, remove DEFAULT_VERSIONS=ssl=base or undefine WITH_OPENSSL_BASE.
*** Error code 1

Stop.
make: stopped in /usr/home/kenm/perforce_redline_portstest/ports/devel/p4
Comment 21 Alan Somers freebsd_committer freebsd_triage 2018-11-07 20:19:02 UTC
There certainly are other ports that are particular about whether they use ports openssl or the base openssl.  The answer is that you simply can't compile a port that wants base openssl if ports openssl is already installed.  You can still build it with Poudriere, though, and the dynamic linker will do the right thing.

The best thing for p4 is probably to use your patch, because that reduces p4's dependencies.  Have you test its functionality?  If it works for you, then I can conditionalize the patch.
Comment 22 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-07 20:34:54 UTC
I agree, it would be better to make the patch conditional and go that way instead of depending on a specific version of OpenSSL.

It does work for me, although I'm not sure whether our server is using SSL.

By the way, you can go to swarm.workshop.perforce.com and create an account, and use Perforce's public server.  I'm not sure how to tell whether they're using SSL on that server, though.
Comment 23 Alan Somers freebsd_committer freebsd_triage 2018-11-08 01:44:46 UTC
Created attachment 199071 [details]
Fix build by conditionally applying Ken's patch

Attached is a combination of Ken's patch and mine.  It conditionally applies Ken's patch on 12.0 and later versions.  Ken, could you please test it?
Comment 24 Nathan 2018-11-08 02:00:21 UTC
Comment on attachment 199071 [details]
Fix build by conditionally applying Ken's patch

+.if ${OSVERSION} >= 1200085

Didn't mean to do it that way. That only works if someone is on 12+, what if someone on 11.x installs it? Then the patch doesn't get used. Need to actually use if conditions in the patch/src itself
Comment 25 Alan Somers freebsd_committer freebsd_triage 2018-11-08 02:04:26 UTC
(In reply to Nathan from comment #24)

That's exactly the idea, Nathan.  On 11.2 p4 builds find with the OpenSSL in base.  The only case I know of when p4 won't build fine with this patch is on a system running < 12.0 with security/openssl111 installed and not using Poudriere.  However, that's a recipe for failure with dozens of ports.  People who mix and match base openssl with ports openssl simply have to use Poudriere.
Comment 26 Nathan 2018-11-08 02:07:38 UTC
(In reply to Alan Somers from comment #25)
There will be people who chooses to do so. 

simply do:
#if OPENSSL_VERSION_NUMBER < 0X1010000fL

OLDCODE

#else
NEWCODE
#endif

in the source file needed
Comment 27 Nathan 2018-11-08 02:08:43 UTC
(In reply to Nathan from comment #26)
Of course, will need to add 
#ifdef HAVE_OPENSSL
#include <openssl/opensslv.h>
#endif

the ifdef is optional
Comment 28 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-08 18:23:51 UTC
Created attachment 199084 [details]
New version of patch that works on old and new OpenSSL versions

Here is a patch that will work on both old and new versions of OpenSSL.

It uses a condition similar to what Nathan proposed, except that I used this test:

#if OPENSSL_VERSION_NUMBER < 0x10100000L

Nathan suggested 0x1010000fL, which doesn't seem to agree with what the OpenSSL folks are using here:

https://wiki.openssl.org/index.php/OpenSSL_1.1.0_Changes
Comment 29 Nathan 2018-11-08 18:27:16 UTC
(In reply to Kenneth D. Merry from comment #28)
Looks good, I've seen ...0fL and ...00L as well. 
New patch is the way it should be. Expecting all users to use poudriere to get it to work should not be the correct way, as some don't know how to use it; others just want to simply "make install" in port dir
Comment 30 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-08 18:39:19 UTC
I should have mentioned that I have tested this on stable/12 and stable/11.  It builds, and the resulting p4 binary runs, although I don't think my server is running SSL.  (Unless that is turned on by default somehow.)
Comment 31 Alan Somers freebsd_committer freebsd_triage 2018-11-13 17:04:30 UTC
(In reply to Nathan from comment #29)

I don't expect ordinary users to use Poudriere; I expect them to use pkg(8) ;)

That said, Ken's patch works.  I can't test SSL though; even swarm.workshop.perforce.com doesn't use SSL.  Do any of you want to do any more testing before I commit?
Comment 32 Kenneth D. Merry freebsd_committer freebsd_triage 2018-11-13 17:06:26 UTC
I think it's probably fine to commit as-is.  The fix is relatively straightforward in this case.
Comment 33 commit-hook freebsd_committer freebsd_triage 2018-11-13 17:16:04 UTC
A commit references this bug:

Author: asomers
Date: Tue Nov 13 17:15:09 UTC 2018
New revision: 484879
URL: https://svnweb.freebsd.org/changeset/ports/484879

Log:
  devel/p4: fix build with OpenSSL 1.1

  PR:		229021
  Reported by:	brnrd
  Submitted by:	ken
  MFH:		2018Q4

Changes:
  head/devel/p4/Makefile
  head/devel/p4/files/patch-net_netsslcredentials.cc
Comment 34 commit-hook freebsd_committer freebsd_triage 2018-11-18 16:06:56 UTC
A commit references this bug:

Author: asomers
Date: Sun Nov 18 16:06:30 UTC 2018
New revision: 485231
URL: https://svnweb.freebsd.org/changeset/ports/485231

Log:
  MFH r484879

  devel/p4: fix build with OpenSSL 1.1

  PR:             229021
  Reported by:    brnrd
  Submitted by:   ken
  MFH:            2018Q4

  Approved by:	ports-secteam (miwi)

Changes:
_U  branches/2018Q4/
  branches/2018Q4/devel/p4/Makefile
  branches/2018Q4/devel/p4/files/patch-net_netsslcredentials.cc
  branches/2018Q4/devel/subversion/files/extra-patch-fbsd-template