> ssl.c:91:9: error: variable has incomplete type 'BIGNUM' (aka 'struct bignum_st') 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. The intent to update OpenSSL to 1.1 in 12 has now officially been documented in Bug #228912 Poudriere log: https://keg.brnrd.eu/data/111amd64-default-openssl110/2018-06-11_10h42m37s/logs/errors/rdesktop-1.8.3.log
Created attachment 194639 [details] openssl.patch
Created attachment 194640 [details] openssl2.patch
I found Archlinux was applying both of these patches to fix the OpenSSL problem. I tried tossing them in the files directory but they're not applying right and I"m not sure why? If someone can help me fix this issue?
Hi Greg, These are git diffs. You should be able to apply them manually from WRKSRC using `patch -p1 < openssl.patch` etc. I tend to convert these using a sed one-liner, see https://wiki.freebsd.org/BernardSpil/PortingNotes 'Convert git diff' Once applied to the source, please use `make makepatch` to generate the patches and use `svn diff` to generate a diff to upload here as a patch.
Created attachment 196387 [details] rdesktop.diff
Reporter is committer, assign accordingly.
Everyone busy or am I supposed to be doing something here, I see it needs maintainer feedback, but I'm not sure on what.
I need this port :-). I'm happy to test and/or commit, but I need approval from someone with a ports bit.
(In reply to Greg Fitzgerald from comment #5) Upstream has both of these patches in git; the first since September 2016, for what it's worth: https://github.com/rdesktop/rdesktop/commit/50b39d114aa9d528c43e7aeefce37b3542f44ba5 Why don't we just update rdesktop to a newer git version instead of adding more patch files? Upstream is active but hasn't issued a new "release" since 2015, which is just silly. They're essentially a rolling release git project now.
(In reply to Conrad Meyer from comment #9) I would be in favor of changing this to a git based port if everyone is OK with that. I just don't have the time to do it for a bit yet. If you are in a hury Conrad to get that done I'll approve anything you get working. If not I'll work on it soon as I can.
No rush for me anymore — vangyzen@ pointed me at Remmina as a suitable replacement.
Could someone please review this and get it pushed so rdesktop will work in 12? I'll get on uinsg a git snapshot soon as I have some extra time, but for now this should fix the build errors.
Reviewing 2018-08-20 rdesktop.diff: This patch raises some questions of its own, like, "why is rdesktop reimplementing RSA encryption instead of just using openssl?". This would probably be easier (for me) in phabricator, FWIW. ----- patch-openssl mostly looks like a straightforward translation of the existing code. The line: algor = X509_PUBKEY_get0_param(NULL, NULL, 0, &algor, key); Is slightly off — the 3rd parameter should just be NULL as well. Fortunately, zero has the same value as null and is implicitly converted to 'int *'. (Oh, and as the second patch points out, the return value is an int, not a pointer -- this code simply doesn't work.) I have no idea if the part at 'X509_PUBKEY_set0_param' accomplishes the same goals as the prior code, but it looks reasonable. The hunk at 201,14 +209,24 is bogus: ++ BIGNUM *e = NULL; ++ BIGNUM *n = NULL; ... ++ RSA_get0_key(rkey, &e, &n, NULL); RSA_get0_key has the signature (r, *n, *e, *d). We're grabbing the modulus as 'e' and the public exponent as 'n'. Both are components of the public key, so it 's not like this is a private key leak, but the variables are now misnamed. It then proceeds to copy those incorrect values to its own modulus/exponent out-parameters. **This patch is totally broken.** ----- patch-openssl2 corrects some deficiencies in the first one and looks fine. ----- Port mechanics: looks fine to me (keeping in mind I don't have a ports bit). Usual convention is to have a single patch against a given file (e.g., ssl.c) but I don't especially like that convention and it doesn't really make sense when incorporating a series of upstream patches, like here. ----- While this would fix the build on openssl 1.1.0+, it introduces a regression in rdssl_rkey_get_exp_mod(). I don't think I would approve this as-is.
(In reply to Conrad Meyer from comment #13) Conrad, Thanks for the detailed review. I'm not qualified to handle this patch I fear. I don't know much of any c++ and I never worked with openssl outside of go and ruby. This patch is up streamed now so I'm not sure if we should just abandon rdesktop in ports because the code isn't safe? Either way I'm going to need some help on how to proceed from here.
(In reply to Greg Fitzgerald from comment #14) Unfortunately, the n/e swap was fixed in this upstream commit whose primary scope was unrelated and whose commit message doesn't even mention the correction: https://github.com/rdesktop/rdesktop/commit/a3dfceefc2c729243b71270e3f503fa2dd57ec8d It looks like it might also be significant for rdesktop functionality on OpenSSL 1.1.0+ anyway, so maybe we just take the whole patch? Unfortunately again, it definitely won't apply cleanly because it modifies the same code as all of these intermediary patches that were committed after patch-openssl2 and before a3dfceef: * https://github.com/rdesktop/rdesktop/commit/71bf45a5d7ac6dcd33324e9adc3df75daff2c59d#diff-5931449ee480a798cabbfe5a0df96da8 * https://github.com/rdesktop/rdesktop/commit/87d8d123b81093ec8ce635e3c5009038e5a102f3#diff-5931449ee480a798cabbfe5a0df96da8 * https://github.com/rdesktop/rdesktop/commit/908ad64d846502e8eecb887bb8509071b1c3ae38#diff-5931449ee480a798cabbfe5a0df96da8 * https://github.com/rdesktop/rdesktop/commit/5b7b955487a7f6886500aec6edcdc684da49e9d4#diff-5931449ee480a798cabbfe5a0df96da8 So here's what I suggest: take the logical changes in a3dfceef and apply them manually. Re-generate a diff and call it patch-openssl3. If you're uncomfortable doing that, let me know and I'd be happy to provide the proposed patch-openssl3 for you.
(In reply to Greg Fitzgerald from comment #5) This fix me Problem with FreeBSD 12.0 http://joneumbox.org/build.html?mastername=120amd64-ports&build=2018-12-14_14h00m52s http://joneumbox.org/build.html?mastername=120i386-ports&build=2018-12-14_14h00m54s Can we committed this patch?
(In reply to Jochen Neumeister from comment #16) As stated below my comments, this patch is incorrect and shouldn't be merged in it's current state. I haven't had the spare time to work on this so it's sitting till either I have time or someone steps up and does the work. Sorry for the delays!
I'll throw something in phabricator today-ish.
I've posted a review up at https://reviews.freebsd.org/D18566 . It builds and basic functions seem fine. I did not test thoroughly.
Created attachment 201252 [details] Upgrade rdesktop to 1.8.4 rdesktop 1.8.4 compiles fine and does not have the openssl error. Upgrade port to rdesktop 1.8.4
Created attachment 201253 [details] Log of rdesktop 1.8.4 building with openssl 1.1.1a on FreeBSD 12-STABLE
Created attachment 201358 [details] rdesktop-1.8.4.diff This is my working patch for 1.8.4
1.8.4 release seems to have lots of security fixes in addition to openssl 1.1.1 build ones (https://github.com/rdesktop/rdesktop/releases/tag/v1.8.4), could we please go forward with this one?
*** Bug 235600 has been marked as a duplicate of this bug. ***
Which is the right patch? Please delete all others. That's just some chaos here
ping
The one made by me. I don't think I have permissions to obsolete others patches.
See also: bug #235885.
In attachment 201358 [details] if you change USES=autoreconf:build to just USES=autoreconf you shouldn't need the pre-configure stanza.
A commit references this bug: Author: pi Date: Fri Feb 22 06:34:05 UTC 2019 New revision: 493554 URL: https://svnweb.freebsd.org/changeset/ports/493554 Log: net/rdesktop: update 1.8.3 -> 1.8.4 - many more CVEs are fixed by this upgrade, see Relnotes PR: 235885, 229029 Submitted by: Greg Veldman <freebsd@gregv.net> Reviewed by: w.schwarzenfeld@utanet.at, brnd, cem, joneum Approved by: gregf@hugops.pw (maintainer timeout) Relnotes: https://github.com/rdesktop/rdesktop/releases/tag/v1.8.4 Security: CVE-2018-8794 MFH: 2019Q1 Differential Revision: https://reviews.freebsd.org/D18566 Changes: head/net/rdesktop/Makefile head/net/rdesktop/distinfo head/net/rdesktop/files/patch-configure head/net/rdesktop/pkg-plist
A commit references this bug: Author: pi Date: Fri Feb 22 08:40:58 UTC 2019 New revision: 493562 URL: https://svnweb.freebsd.org/changeset/ports/493562 Log: MFH: r493554 net/rdesktop: update 1.8.3 -> 1.8.4 - many more CVEs are fixed by this upgrade, see Relnotes PR: 235885, 229029 Submitted by: Greg Veldman <freebsd@gregv.net> Reviewed by: w.schwarzenfeld@utanet.at, brnd, cem, joneum Approved by: gregf@hugops.pw (maintainer timeout) Relnotes: https://github.com/rdesktop/rdesktop/releases/tag/v1.8.4 Security: CVE-2018-8794 Differential Revision: https://reviews.freebsd.org/D18566 Approved by: ports-secteam (joneum) Changes: _U branches/2019Q1/ branches/2019Q1/net/rdesktop/Makefile branches/2019Q1/net/rdesktop/distinfo branches/2019Q1/net/rdesktop/files/patch-configure branches/2019Q1/net/rdesktop/pkg-plist
A commit references this bug: Author: pi Date: Fri Feb 22 17:58:17 UTC 2019 New revision: 493578 URL: https://svnweb.freebsd.org/changeset/ports/493578 Log: security/vuxml: dokument rdesktop < 1.8.4 vulnerabilities PR: 235885, 229029 Changes: head/security/vuxml/vuln.xml
Committed, thanks to all!