Bug 233014 - net/freerdp1: Fix build with OpenSSL 1.1
Summary: net/freerdp1: Fix 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: Kyle Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-06 05:26 UTC by Kyle Evans
Modified: 2018-11-22 23:22 UTC (History)
1 user (show)

See Also:


Attachments
svn(1) diff against the ports tree (54.84 KB, patch)
2018-11-06 05:26 UTC, Kyle Evans
no flags Details | Diff
svn(1) diff to mark BROKEN (1.00 KB, patch)
2018-11-14 18:39 UTC, Kyle Evans
no flags Details | Diff
svn(1) diff against the ports tree (56.62 KB, patch)
2018-11-16 17:50 UTC, Kyle Evans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle Evans freebsd_committer freebsd_triage 2018-11-06 05:26:17 UTC
Created attachment 199001 [details]
svn(1) diff against the ports tree

Attached patch should resolve... testing required
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2018-11-06 05:28:45 UTC
Hi Richard,

Can you help facilitate testing of this patch? The only consumer left is guacamole-server for the RDP bits. I have no guacamole setup nor anything that net/freerdp1 can actually connect to anymore, and this required some refactoring of things that I couldn't quite pull from upstream. A basic smoke test of establishing a usable connection is all that I think's needed, both -with- OpenSSL 1.1.1 *and* without (e.g. 11.x releases, stable/11)
Comment 2 Richard Gallamore freebsd_committer freebsd_triage 2018-11-06 07:53:52 UTC
Hey Kyle,

This is quite the patch, you are quite determined. Added it to my daily build queue and will run some tests to verify functionality.
Comment 3 Kyle Evans freebsd_committer freebsd_triage 2018-11-06 12:37:12 UTC
(In reply to Richard Gallamore from comment #2)

Thanks! I was crossing my fingers hoping for either a guacamole update or a patch that could be backported for net/freerdp support so I could just drop freerdp1, but alas... I found little evidence that upstream is moving towards that goal without the final release of FreeRDP 2. =(
Comment 4 Richard Gallamore freebsd_committer freebsd_triage 2018-11-06 17:55:57 UTC
(In reply to Kyle Evans from comment #3)
The patch breaks rdp. I'll attempt to debug it when I have some more time unless you have an idea of what may be buggy.
Comment 5 Kyle Evans freebsd_committer freebsd_triage 2018-11-06 17:59:05 UTC
(In reply to Richard Gallamore from comment #4)

Any kind of errors you're seeing with this breakage, and is this against OpenSSL 1.0.x or 1.1.x?

My primary suspect if the former would be bits in files/patch-libfreerdp_core_tcp.c, as this is where I had to apply some manual refactoring because bio->num isn't exposed in the new world order.
Comment 6 Richard Gallamore freebsd_committer freebsd_triage 2018-11-06 18:18:01 UTC
(In reply to Kyle Evans from comment #5)
Only tested it on openssl 1.1.x so far. The logs are at [1] and [2]. Note: There was a revision bump before my daily builds so I had to bump a second time for the patch which is revision 14.



[1] https://poudriere.ultimasbox.com/build.html?mastername=112amd64-test&build=2018-11-05_23h57m32s
[2] https://poudriere.ultimasbox.com/build.html?mastername=112amd64-default&build=2018-11-06_08h53m03s
Comment 7 Richard Gallamore freebsd_committer freebsd_triage 2018-11-06 18:20:19 UTC
(In reply to Kyle Evans from comment #5)
Forgot to answer the question... No useful errors from what I could tell were being returned.
Comment 8 Kyle Evans freebsd_committer freebsd_triage 2018-11-13 16:16:13 UTC
(In reply to Richard Gallamore from comment #7)

Ugh. How upsetting would it be to have a net/freerdp1 marked BROKEN on FreeBSD12 and 13? I'm afraid with other things on my plate, I don't necessarily have the time or resources to dedicate to debugging this for some undetermined amount of time.

On the plus side, it really looks like FreeRDP should be close to a state where they can cut a 2.0 release for Guacamole to update to. =(
Comment 9 Richard Gallamore freebsd_committer freebsd_triage 2018-11-13 17:40:58 UTC
(In reply to Kyle Evans from comment #8)
Marking as broken for now is fine. Have my testing environment up but I haven't had the time to really dig into it yet. Keep the ticket open until we get it resolved or the 2.0 release + guacamole support occurs.
Comment 10 Kyle Evans freebsd_committer freebsd_triage 2018-11-14 18:39:10 UTC
Created attachment 199241 [details]
svn(1) diff to mark BROKEN

Hi,

Alright- may I have your approval to commit this BROKEN patch with the following commit message?

--
net/freerdp1: Mark BROKEN on 12.0+ and with openssl111

This port fails to build with OpenSSL 1.1. The upstream commits that
brought the relevant support do not apply cleanly, as some major
refactoring was done after this release.

General users of the port are advised to move to net/freerdp, which is
the effective replacement of this port. Users of net/guacamole-server,
which requires this version, will likely need to stay on 11.2 or switch
their OpenSSL preference to security/openssl.

Some effort will be made to unbreak this against OpenSSL 1.1 for Guacamole
users, but no timeline can be announced at this time.

PR: 233014
--

I'm not sure if I've communicated effectively everything that should be communicated here.
Comment 11 Richard Gallamore freebsd_committer freebsd_triage 2018-11-15 00:29:51 UTC
(In reply to Kyle Evans from comment #10)
Hey Kyle,

Comment and patch are both accurate and lgtm. I approved the commit assuming that it has been tested with portlint / poudriere and verified working with no obvious bugs.
Comment 12 Kyle Evans freebsd_committer freebsd_triage 2018-11-15 14:07:49 UTC
(In reply to Richard Gallamore from comment #11)

Excellent, thanks. I had a rare bout of free time last night and got Guacamole setup in an 11.2 jail so that I can test the different combinations and try to debug the breakage, so I'm going to hold off on committing this for a couple days to give myself a chance to fix it instead. I kind of expected Guacamole to be a lot more difficult and time-consuming to setup, but that's the easiest deployment I've had this year. =)
Comment 13 Kyle Evans freebsd_committer freebsd_triage 2018-11-16 17:50:44 UTC
Created attachment 199277 [details]
svn(1) diff against the ports tree

Hi,

After scraping through the patch with a fine-tooth comb or four and reconciling with upstream a little bit, I fixed it. Here's my new patch for your approval consideration, and proposed commit message:

--
net/freerdp1: Fix build with OpenSSL 1.1

Patch taken partially from upstream with some minor refactoring because
the patch from upstream was fairly far off from where this version of
FreeRDP is at.

Built with:     Poudriere (11.2 and 13.0-CURRENT)
Tested with:    OpenSSL 1.0.2 (11.2, base)
Tested with:    OpenSSL 1.1.1 (11.2, security/openssl111)
MFH:            2018Q4
--

Thanks,

Kyle Evans
Comment 14 Richard Gallamore freebsd_committer freebsd_triage 2018-11-17 20:32:23 UTC
(In reply to Kyle Evans from comment #12)
Hey Kyle,

That is great to hear. Being able to jump right in an application is the goal, can be challenging make it this way sometimes though.

(In reply to Kyle Evans from comment #13)
Awesome, I'm glad you had the time to work on it. I and the community definitely appreciates it!

I'll test it shortly and get back to you.
Comment 15 Richard Gallamore freebsd_committer freebsd_triage 2018-11-18 09:05:28 UTC
(In reply to Kyle Evans from comment #13)
There are a couple new warnings here that could be fixed, but it looks safe enough to not worry about.

lgtm, thanks for the great work Kyle!
Comment 16 Kyle Evans freebsd_committer freebsd_triage 2018-11-18 14:03:19 UTC
(In reply to Richard Gallamore from comment #15)

Excellent! I will proceed to commit with your approval... Once I wake up.
Comment 17 commit-hook freebsd_committer freebsd_triage 2018-11-18 14:53:55 UTC
A commit references this bug:

Author: kevans
Date: Sun Nov 18 14:53:02 UTC 2018
New revision: 485226
URL: https://svnweb.freebsd.org/changeset/ports/485226

Log:
  net/freerdp1: Fix build with OpenSSL 1.1

  Patch taken partially from upstream with some minor refactoring because
  the patch from upstream was fairly far off from where this version of
  FreeRDP is at.

  Built with:	Poudriere (11.2 and 13.0-CURRENT)
  Tested with:	OpenSSL 1.0.2 (11.2, base)
  Tested with:	OpenSSL 1.1.1 (11.2, security/openssl111)

  PR:		233014
  Approved by:	ultima (ports), myself (maintainer)
  MFH:		2018Q4 (OpenSSL build fix)

Changes:
  head/net/freerdp1/Makefile
  head/net/freerdp1/files/patch-git_1b5f5747
  head/net/freerdp1/files/patch-include_freerdp_crypto_crypto.h
  head/net/freerdp1/files/patch-libfreerdp_common_assistance.c
  head/net/freerdp1/files/patch-libfreerdp_core_certificate.c
  head/net/freerdp1/files/patch-libfreerdp_core_tcp.c
  head/net/freerdp1/files/patch-libfreerdp_core_transport.c
  head/net/freerdp1/files/patch-libfreerdp_crypto_CMakeLists.txt
  head/net/freerdp1/files/patch-libfreerdp_crypto_crypto.c
  head/net/freerdp1/files/patch-libfreerdp_crypto_opensslcompat.c
  head/net/freerdp1/files/patch-libfreerdp_crypto_opensslcompat.h
  head/net/freerdp1/files/patch-libfreerdp_crypto_tls.c
  head/net/freerdp1/files/patch-winpr_libwinpr_crypto_crypto.c
  head/net/freerdp1/files/patch-winpr_libwinpr_crypto_crypto.h
  head/net/freerdp1/files/patch-winpr_libwinpr_sspi_NTLM_ntlm.c
  head/net/freerdp1/files/patch-winpr_libwinpr_sspi_NTLM_ntlm__compute.c
  head/net/freerdp1/files/patch-winpr_tools_makecert_makecert.c
Comment 18 commit-hook freebsd_committer freebsd_triage 2018-11-18 15:07:09 UTC
A commit references this bug:

Author: kevans
Date: Sun Nov 18 15:06:43 UTC 2018
New revision: 485228
URL: https://svnweb.freebsd.org/changeset/ports/485228

Log:
  MFH: r485226

  net/freerdp1: Fix build with OpenSSL 1.1

  Patch taken partially from upstream with some minor refactoring because
  the patch from upstream was fairly far off from where this version of
  FreeRDP is at.

  Built with:	Poudriere (11.2 and 13.0-CURRENT)
  Tested with:	OpenSSL 1.0.2 (11.2, base)
  Tested with:	OpenSSL 1.1.1 (11.2, security/openssl111)

  PR:		233014
  Approved by:	ultima (ports), myself (maintainer)

  Approved by:	ports-secteam (blanket, build fix)

Changes:
_U  branches/2018Q4/
  branches/2018Q4/net/freerdp1/Makefile
  branches/2018Q4/net/freerdp1/files/patch-git_1b5f5747
  branches/2018Q4/net/freerdp1/files/patch-include_freerdp_crypto_crypto.h
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_common_assistance.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_core_certificate.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_core_tcp.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_core_transport.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_crypto_CMakeLists.txt
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_crypto_crypto.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_crypto_opensslcompat.c
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_crypto_opensslcompat.h
  branches/2018Q4/net/freerdp1/files/patch-libfreerdp_crypto_tls.c
  branches/2018Q4/net/freerdp1/files/patch-winpr_libwinpr_crypto_crypto.c
  branches/2018Q4/net/freerdp1/files/patch-winpr_libwinpr_crypto_crypto.h
  branches/2018Q4/net/freerdp1/files/patch-winpr_libwinpr_sspi_NTLM_ntlm.c
  branches/2018Q4/net/freerdp1/files/patch-winpr_libwinpr_sspi_NTLM_ntlm__compute.c
  branches/2018Q4/net/freerdp1/files/patch-winpr_tools_makecert_makecert.c
Comment 19 Kyle Evans freebsd_committer freebsd_triage 2018-11-22 23:22:38 UTC
Thanks!