Page MenuHomeFreeBSD

net/rdesktop: Fix build with OpenSSL 1.1.0
AbandonedPublic

Authored by cem on Dec 15 2018, 1:59 AM.

Details

Summary
net/rdesktop: Fix build with OpenSSL 1.1.0

Fix rdesktop build on new OpenSSL in 12.0 and CURRENT.  Fix at least one
glaring issue in Jani Hakala's initial patch that went unnoticed for 12
months upstream before being fixed without mention.

Note: Given the general security naivety upstream and lack of new releases
since 2015 despite probably hundreds of errata fixed in git, I don't
recommend anyone actually use this port.

Nevertheless, this change gets it compiling.

PR: 229029
Reviewed_by: koobs, ???
Approved by: ??? (ports)
Approved by: portmgr (blanket: build, run, package fix)
Differential_Revision: 18566
Test Plan
  • portlint: TBD
  • testport: TBD
  • maketest: TBD or NA

I did a basic functionality test by connecting to a terminal server; it
seemed fine.

Note that there's more trouble coming:

tcp.c:313:27: warning: 'TLSv1_client_method' is deprecated [-Wdeprecated-declarations]
                g_ssl_ctx = SSL_CTX_new(TLSv1_client_method());
                                        ^
/usr/lib/include/openssl/ssl.h:1854:1: note: 'TLSv1_client_method' has been explicitly marked deprecated here
DEPRECATEDIN_1_1_0(__owur const SSL_METHOD *TLSv1_client_method(void))
^
/usr/lib/include/openssl/opensslconf.h:147:34: note: expanded from macro 'DEPRECATEDIN_1_1_0'
# define DEPRECATEDIN_1_1_0(f)   DECLARE_DEPRECATED(f)
                                 ^
/usr/lib/include/openssl/opensslconf.h:110:55: note: expanded from macro 'DECLARE_DEPRECATED'
#   define DECLARE_DEPRECATED(f)    f __attribute__ ((deprecated));
                                                      ^
1 warning generated.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21801
Build 21070: arc lint + arc unit

Event Timeline

linimon retitled this revision from PR 229029: Fix rdesktop build on OpenSSL 1.1.0 to PR 229029: Fix net/rdesktop build on OpenSSL 1.1.0.Dec 15 2018, 2:23 AM
koobs retitled this revision from PR 229029: Fix net/rdesktop build on OpenSSL 1.1.0 to net/rdesktop: Fix build with OpenSSL 1.1.0.Jan 2 2019, 5:30 AM
koobs edited the summary of this revision. (Show Details)
koobs edited the test plan for this revision. (Show Details)

@Conrad

Change looks fine, thanks for commenting/referencing the patches/sources

Happy to accept after confirmation of QA, particularly as this change will be MFH'd.

net/rdesktop/Makefile
13–14

Add a LICENSE_FILE if distribution file contains one

16

Explanation/why for the xrandr addition? Related/unrelated (to openssl fix) change?

Note also this is:

Approved by: portmgr (blanket: build, run, package fix)

Pending QA

cem marked 2 inline comments as done.Jan 2 2019, 6:39 AM

Change looks fine, thanks for commenting/referencing the patches/sources

Happy to accept after confirmation of QA, particularly as this change will be MFH'd.

Thanks Kubilay. As noted in the PR, "It builds and basic functions seem fine." I don't know of any better QA tests than connecting to my rdesktop server, do you?

net/rdesktop/Makefile
13–14

ok

16

I got a warning about it being missing with DEVELOPER=yes port build:

Error: /usr/local/bin/rdesktop is linked to /usr/local/lib/libXrandr.so.2 from x11/libXrandr but it is not declared as a dependency
Warning: you need USE_XORG+=xrandr

Unrelated to openssl but might be needed for minimal chroot build.

cem marked 2 inline comments as done.

add license_file

In D18566#399201, @cem wrote:

Change looks fine, thanks for commenting/referencing the patches/sources

Happy to accept after confirmation of QA, particularly as this change will be MFH'd.

Thanks Kubilay. As noted in the PR, "It builds and basic functions seem fine." I don't know of any better QA tests than connecting to my rdesktop server, do you?

Runtime testing is great (i wish more ports had test targets that ran test suites), and while that does cover 'build' testing/success, success @ run time doesn't necessarily preclude packaging (or other issues).

poudriere run confirmation is needed to test the complete packaging pipeline

I've added QA items to this reviews TEST PLAN section, just edit and add "OK" after running them and they pass

Thank you for clarifying the addition of xrandr too. If you could include an explanation in the commit log message that would be great "Add missing dependency on xrandr" or similar is fine

Runtime testing is great (i wish more ports had test targets that ran test suites), and while that does cover 'build' testing/success, success @ run time doesn't necessarily preclude packaging (or other issues).

Sure, but the status quo is it hasn't built in months; IMO, it will be hard to regress the port and I identified and fixed a regression in the upstream patches (see PR history).

poudriere run confirmation is needed to test the complete packaging pipeline

I simply don't have the disk space to do a full poudriere run, sorry. I think this is a leaf port, so I'm not sure what else it could break.

Thank you for clarifying the addition of xrandr too. If you could include an explanation in the commit log message that would be great "Add missing dependency on xrandr" or similar is fine

No problem!

I would just like to point out that 1.8.4 was released today.

https://github.com/rdesktop/rdesktop/releases/tag/v1.8.4

How do you want to proceed? It looks like it fixes numerous CVEs, so is probably worth holding off for. I did a spot check and RSA_get0_key(rkey, &e, &n, NULL); is still broken in 1.8.4, so that needs to be reapplied: https://github.com/rdesktop/rdesktop/compare/v1.8.3..v1.8.4#diff-5931449ee480a798cabbfe5a0df96da8R232

I'm not interested enough in this port to do the 1.8.4 work myself. I am just a (now former) user who wanted to help get it building on 13-CURRENT but now I just use remmina instead.