AdrianVollmer/PowerHub

Use nginx as a reverse proxy

AdrianVollmer opened this issue · 15 comments

I'm reluctant to include nginx as a dependency, but twisted is simply not powerful enough. It can't set the X-Forwaded-Host header, so when Flask redirects to a different URL, it doesn't know the host or protocol which the client used to initiate the request.

I'll try to not make it a mandatory dependency, such that everything runs without. However, SSL likely won't work. This must be why everyone discourages the use of SSL with Flask directly. Not a production server, and so on...

If we decide to use nginx, PowerHub needs to run as root.... https://trac.nginx.org/nginx/ticket/147

Even with nginx, if the reverse proxy uses an non standard port, it doesn't work (yet)...

Can you elaborate on the need for the X-Forwaded-Host header? Is this just to assemble absolute URLs? And would this always be the same value passed in as the URI_HOST cli parameter? If yes, maybe it can be taken from there instead.

This might be a bit hacky, but introducing a system dependency like nginx (plus requiring root privileges) sounds a bit inordinate. I really like that powerhub is a pretty much self contained application that can solely live in its virtualenv.

Besides: While I have not used twisted by myself, yet, have you checked if you cannot simply add your own HTTP headers in the forwarded requests?

Is this just to assemble absolute URLs?

yes

And would this always be the same value passed in as the URI_HOST cli parameter? If yes, maybe it can be taken from there instead.

Yes. I wanted to avoid hacky solutions but there may not be another way.

I really like that powerhub is a pretty much self contained application that can solely live in its virtualenv.

I agree, but without a reverse proxy it's pretty much impossible to use both SSL and socketio for websockets. At this point, websockets are only used for neat UI elements like push notifications in the webapp, but I have plans to use websockets as a transport method for the reverse shell.

Besides: While I have not used twisted by myself, yet, have you checked if you cannot simply add your own HTTP headers in the forwarded requests?

I have checked and have not found a way. I used this idea for the reverse proxy: https://gist.github.com/j4mie/921672

Here someone said twisted was not designed to be this flexible

Too bad... On the other hand, the answer lists a possible solution by subclassing the respective functionality. Doesn't look that hacky to me. I would expect that functionality intended for such tasks look very similar. Don't you like it?

I couldn't get it to work ;) the answer is modifying ProxyRequest, and the example I linked above isn't even using that class. I read some twisted source code, tried various things I can't recall now and eventually gave up.

On the other hand, it might actually make sense to hard code the URI in the Flask redirects. Similar to meterpreter, there could be some whacky setup where traffic is redirected in weird ways we can't predict. That's why I describe the option with "the URI path where the target can reach the server". So I guess we already have that information by definition and don't have to extract any headers.

the answer is modifying ProxyRequest, and the example I linked above isn't even using that class.

Yeah, but the subclass of ProxyRequest (or in your case ReverseProxyRequest) is passed to the custom subclass of Site. You are also using Site :P

On the other hand, it might actually make sense to hard code the URI in the Flask redirects. Similar to meterpreter, there could be some whacky setup where traffic is redirected in weird ways we can't predict. That's why I describe the option with "the URI path where the target can reach the server". So I guess we already have that information by definition and don't have to extract any headers.

I agree, that makes sense and would save us work. (Otherwise, if you change your mind and would like to do it with headers, I could give that ReverseProxyRequest subclassing idea a try)

I did it so at least the webapp is usable again. I'll leave this open until we have a decision

I would still like to see these headers in flask. Then we can initiate a redirection to SSL for things that require authentication and not for other things. It wouldn't make sense to make the decision to redirect at the proxy level. Also then we could display the clients IP address in the log, because right now it's just 127.0.0.1 which is not very useful.

Unrelated: twisted is printing lots of pointless stack traces which I don't know how to catch.

I haven't looked at the recent features you introduced, but I'm wondering why you would want to only redirect to HTTPS when authentication is involved. Why not always serving everything over HTTPS?

However, I will see if I can take a look on the header passing thing during the weekend. Though, don't know if it will work.

Because you almost always (if you use self-signed certs) have to disable certificate validation in powershell when retrieving the download cradle anyway. And PowerShell disables TLS1.2 by default, while current Kali systems only offer TLS1.2 by default. So the download cradle is huge. I may leave it in as an option, but I see little benefit in encrypting the download cradle.

The issue I'm having with subclassing the above twisted class when doing it like the poster on stackexchange says is this:

  File "/usr/lib/python3/dist-packages/twisted/web/http.py", line 917, in requestReceived
    self.process()
  File "/home/avollmer/git/PowerHub/powerhub/reverseproxy.py", line 27, in process
    ProxyRequest.process(self)
  File "/usr/lib/python3/dist-packages/twisted/web/proxy.py", line 146, in process
    port = self.ports[protocol]
builtins.KeyError: b''

That's definitely nothing more than a first guess, but maybe the example on stackexchange was written for py2 and now you need to omit the b prefix when adding the raw header.

But maybe I'm just wrong and it is indeed necessary. I will not start investigating this today, but will probably have a look tomorrow.

I see you already fixed that. Congratulations :D And even subclassing was not necessary. Looking at it, the solution is totally clear. Twisted definitely needs better documentation...

Yeah so the stackexchange post was about proxies, not reverse proxies. It was much easier to add headers in a reverse proxy.

Also, what a disgrace it is to only have SSLv3 and TLS1.0 available by default in 2019... on a client:

PS C:\Users\avollmer> [Net.ServicePointManager]::SecurityProtocol
Ssl3, Tls
PS C:\Users\avollmer> [Net.ServicePointManager]::SecurityProtocol = `
>>     [Net.SecurityProtocolType]::Tls12,
>>     [Net.SecurityProtocolType]::Tls11 ;
PS C:\Users\avollmer> [Net.ServicePointManager]::SecurityProtocol
Tls11, Tls12
PS C:\Users\avollmer> get-date

Samstag, 15. Juni 2019 08:27:17