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

Scapy3k for Windows7 #96

Closed
wants to merge 3 commits into from
Closed

Scapy3k for Windows7 #96

wants to merge 3 commits into from

Conversation

Bioh4z4rd
Copy link
Contributor

Tested my implementation. Sending / send+receive and arping works correctly on Windows 7 with winpcap installed in the newest version.

Changes performed:

  • Adapted the latest windows/init.py from the original scapy project
  • Fixed a problem causing the MAC Address 00:00:00:00:00:00 to be used in all cases --> Scapy could not perform ARP for layer 3 transmission

The get_if_raw_hwaddr function always crashed. This causes scapy to use the 00:00:00:00:00:00 to be the mac address used (windows specific code only).

Fixed the function to not crash as well as to return the correct / expected values
…ests)

Removed old prints
Changed "ifaces" variable in get_working_if to "IFACES". This was a changed introduced on original scapy project on the master. Wanted to stick with their changes for compliancy reasons
Added a print for the Exception in case the get_working_if function crashes.
@Bioh4z4rd Bioh4z4rd mentioned this pull request Jun 22, 2016
@phaethon
Copy link
Owner

Can you clarify which code parts are due to Windows 7 compatibility and which are general improvements? The code as is worked ok for Windows 8+ users, so I am not fond of major rewrite to make it compatible with older Windows version if the reasons are not traceable to specific parts of code.

@Bioh4z4rd
Copy link
Contributor Author

I actually copied the latest windows/init.py from the secdev python master.
Nevertheless i would group the changes the following:

  1. Adaption of the functions to get the interfaces and routes. The new code determines whether it can use powershell for the commands otherwise uses VBS scripts for it. It performs then platform specific powershell / vbs commands to obtain network information and so on.
    What needs to be tested here: Starting it on different flavors of Windows and check if Interfaces + Routes are correctly detected
    Risk something could be broken: --> Very Low (as it is easy to check)
  2. The Hardware guid is also safed in a Interface object, and the Representation of the Interface Object is changed to consist of the Class, Interface Name and Guid. Old representation was: Class, Name, Mac, IP (all attributes basically).
    This is the more object oriented / nicer way in my opinion of grouping the Attributes. The only risk here is that if someone parsed the "representation" of an Interface in a scapy script to get some Information this script will definetly be broken now!. Nevertheless a user should directly access the attributes of a interface instead of parsing it's representation.
    Risk: --> Low as bad programming style would only trigger problems here
  3. All the old methods that can be used to get interface device names are no longer there. Renamed from devname_from_xy to dev_from_xy. Formerly the devname was returned as string, now a Interface object is returned.
    Note: It is possible to pass a Interface Object or a Interface Name as "iface" to scapy methods (e.g. send(packet, iface=XY))
    What could be broken here is that users that relied on the old devname_from_XY functions will need to adapt their scripts.
    Risk: --> Medium as functions are removed
  4. The global that is used to hold the Interface dictionary is changed from lowercase to uppercase.
    If this global is used in a scapy script the script will now fail.
    Benefit of the fix is just sticking with styling guide to make globals all uppercase.
    Risk: --> Medium

I would suggest from my side to handle perform the following:
Separate commit for point 1 as it is the most important one. Sending packets with scapy and looking at the routes is enough to check if everything works.
Point 2 is a neat code optimization with low risk of breaking existing python scripts.
I would also implement Point 3 BUT keep the existing devname_from_XY functions to not break existing python scripts so we would reduce the risk to low ;)
Implementing Point 4 would mean staying compliant with python2 scapy to keep scripts "more" portable between both versions. Nevertheless i think we could easily leave this change out.

I can check if i have some free time to implement points 1-3 as described each separatly and update the Pull Request.

@phaethon
Copy link
Owner

You see the history is not simple :) Windows support without libdnet first appeared in this project (scapy3k) not in scapy v2.0 project. Also, scapy3k targets primarily newer Windows platforms (e.g. 10) while scapy v2.0 is naturally more frequently used on older ones. Both projects would like to see full compatibility, but this is about priorities, which, also, affect the code. So, pure copying from the other project is not a good option.
If you split the changes into parts as you suggested, and explicitly differentiate, which part of the code is for Windows 7 compatibility (so that in the future this part could be treated differently), and which is general improvement, I will be more than happy to integrate the code. Also, additional test case (or several) for Windows 7 would be nice.

@sebasmonia
Copy link

For what it's worth...I used this version of the file to get Scapy working with Win 7.

And then with Win 10 sniffing worked if I manually fixed the value in conf.iface (sometimes it would pick up the BT or wired -and inactive- Ethernet port) but sending always failed until I tried this same file.

Seems like a winner to me :)
I do understand why it's risky to include all changes in one go though.

flokli pushed a commit to flokli/scapy that referenced this pull request Nov 21, 2016
@gpotter2
Copy link

As a side note, it's wrong that the original scapy does not support the latest versions. It actually does, and supports as well older ones.

@Bioh4z4rd
Copy link
Contributor Author

Closing as there is a newer Pull Request that is less intrusive

@Bioh4z4rd Bioh4z4rd closed this Jun 30, 2017
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.

4 participants