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.

Sign up for GitHub

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

Jump to bottom

Add --protectinterface argument. #939

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add --protectinterface argument. #939

wants to merge 1 commit into from

Conversation

sophron
Copy link
Member

@sophron sophron commented Jun 5, 2018

No description provided.

@sophron sophron mentioned this pull request Jun 12, 2018
Closed
@sophron sophron force-pushed the add_protectinterface_argument branch 2 times, most recently from 8542681 to 12a19ef Compare June 16, 2018 15:22
@sophron
Copy link
Member Author

sophron commented Jun 16, 2018

@blackHatMonkey @anakin1028 Can you review & merge?

blackHatMonkey
blackHatMonkey requested changes Jun 16, 2018
View reviewed changes
wifiphisher/common/interfaces.py Outdated
if not is_managed_by_network_manager(interface):
raise InterfaceManagedByNetworkManagerError(interface)

return True
Copy link
Member

Choose a reason for hiding this comment

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

Why return True here? as far as I can see this method is just to raise an error.

wifiphisher/pywifiphisher.py Outdated
@@ -401,7 +407,11 @@ def start(self):
time.sleep(1)
self.stop()

if not args.internetinterface:
if args.protectinterface:
for i in args.protectinterface:
Copy link
Member

Choose a reason for hiding this comment

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

Can you also change i to a more descriptive name such as interface?

wifiphisher/common/interfaces.py Outdated
:rtype bool
"""

out = check_output(["nmcli", "dev", interface, "managed", "no"])
Copy link
Member

Choose a reason for hiding this comment

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

check_output needs to be in a try except in case non-zero return value. Also stderr needs to be handled as well.

@sophron sophron force-pushed the add_protectinterface_argument branch from 12a19ef to 09ad3b7 Compare June 22, 2018 17:51
@sophron
Copy link
Member Author

sophron commented Jun 22, 2018

Updated.

@sophron sophron force-pushed the add_protectinterface_argument branch from 09ad3b7 to 3196c03 Compare July 7, 2018 18:02
@sophron
Copy link
Member Author

sophron commented Jul 7, 2018

Can you guys merge this?

blackHatMonkey
blackHatMonkey requested changes Jul 7, 2018
View reviewed changes
Copy link
Member

@blackHatMonkey blackHatMonkey left a comment

Choose a reason for hiding this comment

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

I'm a bit confused. Based on this code if I give it a interface it will call the nm_unmanage which will disconnect it from the internet. Isn't this the exact opposite of what we want.

wifiphisher/common/interfaces.py
"""

try:
out = check_output(["nmcli", "dev", interface, "managed", "no"])
Copy link
Member

Choose a reason for hiding this comment

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

I think we should Popen here and catch stderr. We can log that so the user knows that command failed.

wifiphisher/common/interfaces.py
try:
out = check_output(["nmcli", "dev", interface, "managed", "no"])
# (CalledProcessError)
except:
Copy link
Member

Choose a reason for hiding this comment

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

we should not use a bare except as it hides errors.

wifiphisher/pywifiphisher.py
for iface in args.protectinterface:
self.network_manager.nm_unmanage(iface)

if not args.internetinterface and not args.protectinterface:
Copy link
Member

Choose a reason for hiding this comment

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

I think this and should change to a or.

wifiphisher/pywifiphisher.py
"-pI",
"--protectinterface",
nargs='+',
help=("Choose an interface that you want to protect and persist its connection. " +
Copy link
Member

Choose a reason for hiding this comment

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

The wording needs to be changed a bit to show you can have more than one interface.

wifiphisher/pywifiphisher.py
@@ -401,7 +407,11 @@ def start(self):
time.sleep(1)
self.stop()

if not args.internetinterface:
if args.protectinterface:
Copy link
Member

Choose a reason for hiding this comment

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

This if condition is unnecessary.

@Tsadoq
Copy link

Tsadoq commented Aug 8, 2018

is there any news about the merge? When will we be able to use the --protect-interface option?

@blackHatMonkey
Copy link
Member

@sophron Are we still continuing with this PR. If so I can help fix those errors.

@sophron
Copy link
Member Author

sophron commented Sep 28, 2018

Oh I've left that behind. Can you continue it @blackHatMonkey?

@Tsadoq
Copy link

Tsadoq commented Oct 23, 2018

sorry guys, any possibility to see that fixed?

@sophron
Copy link
Member Author

sophron commented Oct 23, 2018

Hi @Tsadoq,

Yes, I'll make this a priority.

@sophron
Copy link
Member Author

sophron commented Nov 15, 2018

An update on this.

You can currently keep network manager alive using the -kN option. However, in order for the network manager to not mess with your interfaces, you need to manually to manually instruct it. For example, "nmcli dev set wlan0 manage no". Then if you run "wifiphisher --interface wlan0 -kN" you should have a working Wifiphisher with a wireless internet connection to another interface (e.g. wlan1). If you want to bridge this connection to the victims you can do "wifiphisher --interface wlan0 --internetinterface wlan1 -kN". For this last scenario, you need to make sure that wlan0 is "unmanaged" by networkmanager but make sure that wlan1 which provides the Internet remains managed.

I'll work on automating the process the next days.

@sophron sophron mentioned this pull request Dec 7, 2018
Closed
@sophron sophron mentioned this pull request Dec 21, 2018
Open
@manosf manosf mentioned this pull request Apr 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blackHatMonkey blackHatMonkey blackHatMonkey requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sophron @Tsadoq @blackHatMonkey