Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating hydra support for freedrdp 3 #538

Merged
merged 10 commits into from
Jul 7, 2020

Conversation

animetauren
Copy link
Contributor

@animetauren animetauren commented Jun 8, 2020

The newest version of freerdp is v.3, updating Hydra to support this new module.

Have tested this on my Ubuntu 18.04 LTS using latest build from freerdp and thc-hydra at time of writing.

Added support for freerdp module 3, this is the newest module from freerdp
@vanhauser-thc
Copy link
Owner

so everyone who runs a Linux that does not have freerdp3 yet cant use rdp anymore?
do you think that is a good solution?

@animetauren
Copy link
Contributor Author

so everyone who runs a Linux that does not have freerdp3 yet cant use rdp anymore?

Not sure what you are getting at, just if you are running thc-hydra and pull the latest freerdp repo and build from source, you will get an issue. I did, found the issue and just posted a solution that meant adding support for the new version.

@vanhauser-thc
Copy link
Owner

a solution that breaks rdp for most users is not a solution but a bug :)
the correct fix is changing the configure script to test if librdp3 is present and if not if librdp2 is present. not breaking it for librdp2 users.

@animetauren
Copy link
Contributor Author

animetauren commented Jun 9, 2020

Let me go do that, and re-submit a proper PR, good call absolutely right

Adding logic to check for freerdp2 first and if not the rdp module will check for freerdp3 to support the rdp module
Did not do proper check for freerdp2 or freerdp3 modules
Fixed logic inside of configure to properly check for freedrdp2 if not found check for freerdp3, if found to skip freerdp3
hydra-rdp.c Outdated
@@ -10,7 +10,7 @@
#include "hydra-mod.h"

extern char *HYDRA_EXIT;
#ifndef LIBFREERDP2
#if !defined(LIBFREERDP2) || (LIBFREERDP3)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the library versions make no different is v2 or v3 I would just name it LIBFREERDP

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, just fixed this

if [ -n "$WINPR2_PATH" ]; then
XLIBS="$XLIBS -lwinpr2"
if [ -n "$WINPR3_PATH" ]; then
XLIBS="$XLIBS -lwinpr3"
Copy link
Owner

@vanhauser-thc vanhauser-thc Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here you have not reestablished the previous librdp2 for XLIBS, only librdp3. same for the include paths above and defines etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fixed the XLIBS and XDEFINES paths

Removing double entry of libfreerdp in hydra.c and hydra-rdp.c
Trying to fix defines for freerdp
@animetauren
Copy link
Contributor Author

@vanhauser-thc this should be it, tested this on my machine, compiles and runs as it should and fixed the ldconfig issue by adding the instructions to the makefile. Let me know your thoughts.

@@ -78,6 +78,7 @@ install: strip
-cp -f *.csv $(DESTDIR)$(PREFIX)$(DATADIR)
-mkdir -p $(DESTDIR)$(PREFIX)$(MANDIR)
-cp -f hydra.1 xhydra.1 pw-inspector.1 $(DESTDIR)$(PREFIX)$(MANDIR)
-ldconfig
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why that? that would only be needed if a new shared library would be installed (which hydra does not have).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed for "libfreerdp3.so.3" which is loaded as a shared library, not sure why this is the case for v3 and not for v2. This is the error I get when ldconfig is not done:

/usr/local/bin/hydra: error while loading shared libraries: libfreerdp3.so.3: cannot open shared object file: No such file or directory

configure Outdated
@@ -1518,9 +1601,12 @@ fi
if [ -n "$MONGODB_IPATH" ]; then
XIPATHS="$XIPATHS -I$MONGODB_IPATH -I$BSON_IPATH"
fi
if [ -n "$FREERDP2_IPATH" ]; then
if [ -n "$FREERDP3_IPATH" ]; then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is wrong, you moved the check to RDP3, but then copy RDP2_IPATH. as the real RDP3 one is below this has to be $FREERDP2_IPATH :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this bug is still present

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on this today and will update when finished

@vanhauser-thc
Copy link
Owner

besides two minor things I just commented on it looks good.
One issue that could result is that if librdp2 and 3 are both installed then both will tried to be linked and that would fail. and you always check for both being present.
you could also check for 3 first and then skip 2 if it was found, that would save the trouble.

@vanhauser-thc
Copy link
Owner

sorry for the long delay, I have so much to do ...

there is one bug left, just commented on it.
also if both libfreerdp2 and libfreerdp3 are present the script will try to link both. I would recommend to prefer rdp3 over rdp2.

Updating the logic here to check for freerdpv3 first and if found do not check for freerdpv2. Also fixed paths for freerdpv2 paths.
Updated bash "or" to use "||" and "and" to use "&&" conditionals with proper POSIX specifications.
@animetauren
Copy link
Contributor Author

@vanhauser-thc This should be it, fixed the bug you mentioned, added logic to handle version of freerdp properly and just updated bash conditionals.

@vanhauser-thc
Copy link
Owner

looks good, thanks!

@vanhauser-thc vanhauser-thc merged commit df475f1 into vanhauser-thc:master Jul 7, 2020
@vanhauser-thc
Copy link
Owner

I had to perform a few fixes:

  • my [ does not understand &&/|| and failed, reversed that change
  • the logic to test for freerdp2 was inverse hence it was only checked for if freerdp3 was found
  • the ldconfig was not indented which makes gnu make fail. also does belong to the library install not hydra

but now everything compiles cleanly, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants