Hey,

grouping changes in this PR to discuss about them one by one...

pyreadline3

Yep, this is a needed workaround for these examples to run on Windows. At least until pyreadline3 fixes support to py3.13... checking if can be done more globally, instead than in every example; but yes, should be merged.

pydivert

I see the need of the redirection when runnning smb relay server in windows, but I wouldn't include redirection functionality out of the box in Impacket.
Operator can set the smb_port to use in Impacket parameters and in a different process configure needed redirections if any

ps1 installer & npcap

As talked in #1871 I agree with @anadrianmanrique that the building examples infrastructure is a bit out of scope of Impacket. But, after merging pyreadline3 workaround, I guess the project will be in a status in which it will help running that infrastructure as a different project for everyone needing it.

Thoughts? Comments?

@gabrielg5

I can take a look at the pyreadline3 issue on a more global sense if you'd prefer. To my understanding, the property is accessed by the Cmd class that is wrapped by all the other shells, patching the Cmd class is not possible since it is not part of Impacket. You have to import pyreadline3 before Cmd does, and then add the missing property, Since the examples are separate you have to do this for each one. It would however be possible to deduplicate the code a bit by adding another intermediate shell class that wraps Cmd and which would be wrapped by all the other shells which simply patches the property and then does everything else normally. This would still have to be swapped out for each shell though, as far as I know there is no simple "set it once" approach possible here.

Regarding pydivert, there's not a lot of viable options to have SMB redirection when running on windows. Even when it comes to 3rd party support options. Since port 445 is bound by the kernel, you either have to redirect the packets, or use a driver. There are environments where that might not be easy or possible at all. If you are willing to consider having support for windows across the Impacket examples (which I feel is advantageous to a lot of people out there) then I think this is a key component.

The installer can be ignored, I'm happy to leave that as a script in my fork that can pull from this one, and I can see why you might not want to include Npcap as well, perhaps just a check and warning line stating that it's required for functionality on Windows would be a solution?

The goal here is to allow a consultant the ability to do testing and research from a Windows device. Sometimes there are no alternatives on engagements. Having tested my builds in engagements recently, I can confirm they are working as intended. Merging as many of the changes as possible back upstream and maintaining some kind of build script separately would be the ideal outcome for me.

Hi @p0rtL6,

The issue with the pydivert addition we see (besides adding a new dependency to the lib, which could be minor), is that we are giving too much responsibility to Impacket in deciding how to handle this scenario.
We think that decision should be in the operator's plate; also deciding which technique to use to circumvent the fact that the OS is bound to the 445 (ICS Port Forwarding, Firewall Redirection, even shutting down smb server, ...)

So in order to move forward with this one.. win-building script and npcap should be excluded as well as pydivert processing

Does it make sense?

@gabrielg5

I understand what you are saying about pydivert. If the issue is the lack of user control, it would be possible to add something like a --divert option that would trigger the port redirection. This would allow for the functionality if requested by the user, but would not change behavior by default.

If this is still not wanted, I can strip down the changes to just the pyreadline patch, and then open a new pull request.

In addition, the pcapy module seems to be broken on newer versions of Python, and is worth looking into replacement. In my fork, it has been replaced with pcapy-ng which seems to work fine, but there might be other forks of the project that are worth looking into.

Hey,

yes, let's go that path... leave the pyreadline patches so we can move forward with this

Related to pcapy, yes! we were already talking internally on that issue. If you want to upload a PR with those changes we can work from there too

Thanks!

Ok, I have opened a pull request with just the patches for pyreadline3 so they can be merged separately.

As for the pcapy changes, as far as I can tell pcapy-ng is a drop-in replacement for the library, so you simply just need to change the package in the requirements file, the import name is still the same. That seems like something trivial enough that making a pull request would be redundant unless you would like that thread to serve as a place to discuss the changing of the library, in which case I am more than happy to make that pr.

Edit: I am unsure based on your last comment what you want to be done with the pydivert changes, are they confirmed to be closed out, or do you want me to make the changes I suggested to the functionality? If they are going to be closed out, I should just be able to close this entire PR after dealing with the readline and pcapy issues.