feross / SpoofMAC

:briefcase: Change your MAC address for debugging
https://feross.org/spoofmac/
3.03k stars 269 forks source link

Added support for multiple OS implementation. #14

Closed cjbarker closed 11 years ago

cjbarker commented 11 years ago

TODO - implement Windoze MAC spoof (coming soon)

feross commented 11 years ago

@cjbarker – Thanks adding Linux support. However, the codebase has changed a lot since you forked it, so it's basically impossible to integrate this in as-is. It'll require some changes.

Also, Linux already has an excellent tool for changing MAC addresses, MAC Changer, so I'm not sure that this tool needs to support the Linux use case, but maybe I'm wrong. Curious to hear your thoughts on this.

cjbarker commented 11 years ago

I didn't check the recent activity on the codebase. That's my mistake for not doing a sync from your repo first before merging in my changes.

I like the idea of a single script/tool for MAC spoofing for all 3 major platforms: *NIX, Mac, & Windows. I saw a link to your tool on HNews and it was easily extendable for linux. I also need to utilize it for Windows too (coming soon).

I see value-add in having a single MAC Spoofer tool for all platforms. It's one stop shop tool that can easily be ported to all major platforms. Additionally, it's incredibly easy for changing MAC on Linux (as you can see). I don't see the value in having multiple, separate tools per OS. Windows is where it gets messy (done via registry); however, it can be easily integrated into your SpoofMAC tool.

Ultimately, it's up to you. Do you want to extend and expand the SpoofMAC to support multiple platforms or leave this as a tool exclusive to Macs? Regardless, your initial code base was a nice starting point for me to extend this onto additional platforms. If the answer is yes to expansion than I'd be happy to help.

Let me know how you'd like to proceed. I'd be happy to update my fork and merge in my changes w/ your latest and greatest code base. Additionally, I could add Windows support soon.

Cheers.

feross commented 11 years ago

Okay. If adding Linux support is easy, then let's do it. Windows too (if it's not going to muck up the codebase too much).

TkTech commented 11 years ago

My refactor moves most of the work into a module (spoofmac) for reusability. It would be nice to have a portable python package that other tools can use and build on.

cjbarker commented 11 years ago

Hi Folks,

Just following up here. I have merged in the last major refactor that support egg & python module to my fork. I have support for Windows in addition to Linux into the module.

I need to wrap up some testing before I commit and submit new pull requests. I've got a few personal issues to attend, but I should have this completed in a few days, and will submit pull when ready.

Cheers.

feross commented 11 years ago

Hey @cjbarker, I don't see any windows support in your latest code. Where is it?

cjbarker commented 11 years ago

@feross see commit above with Windows spoofing addition. This does NOT include Linux. I'll integrate that in this weekend or or next weekend.

Hopefully, the refactoring is self explanatory in the code.

feross commented 11 years ago

@TkTech, what are your thoughts on @cjbarker's changes?

TkTech commented 11 years ago

I've given some comments on it. At this point I'd say it's an okay starting point but definitely would not advise merging it. It adds needless abstraction, introduces singletons (never a good idea in python), has some odd styling, overly complex regexes, and a couple of other bad practices.

cjbarker commented 11 years ago

@TkTech, I never heard back from you regarding changes based on your recommendations.

I've also added Linux support w/ the latest commit here.

@feross you now have full OS support for Mac, Windows & Linux. If you want to provide a more robust, platform independent library for MAC spoofing then I recommend you accept this pull request.

feross commented 11 years ago

Looks reasonable to me. If @TkTech likes the style and has no more feedback, I will merge it in.

What OSs have you tested this on?

cjbarker commented 11 years ago

@feross tested on Linux Mint 13 and Ubuntu 12.04 LTS. It's utilizing ifconfig for getting and setting the MAC, which should be standard across Linux platforms.

cjbarker commented 11 years ago

@TkTech , @feross is waiting for your response. Any more feedback?

TkTech commented 11 years ago

I have been busy, apologies. I can give this a check tomorrow afternoon.

TkTech commented 11 years ago

I'm okay with this. I'd like to make some changes to it when I can find the time, but the interface won't change so I'd say merge. @feross ?

feross commented 11 years ago

Merged. Will test and if everything looks good, I'll update the version in PyPI.

feross commented 11 years ago

There are several syntax errors in the code now. @cjbarker - you sure you tested this?

feross commented 11 years ago

@cjbarker Could you ping me when these issues (and possibly others) are fixed?

feross commented 11 years ago

I fixed the obvious errors, but there may still be others.

cjbarker commented 11 years ago

@feross , as previously mentioned, the Linux and Windows additions have been tested. I do not own a Mac and have NOT tested the Mac OS portion. That class was strictly a copy and paste with very minor modifications. I'll see if I can get a hold of a Mac to test on.

feross commented 11 years ago

I can test on Mac if you test on Windows and Linux.

cjbarker commented 11 years ago

Sure, I just sent you another pull request regarding the custom expection removal. Was in local version, but missed in diff/commit review. Didn't catch it since I was testing directly against module library on last test rather than running as service/cli program.

feross commented 11 years ago

Tested on Mac and it looks good. Can you just confirm that it looks good on Windows/Linux? Then I'll update in PyPI.

feross commented 11 years ago

Just saw your previous message. Okay, updating in PyPI now...