i-am-shodan / USBArmyKnife

USB Army Knife – the ultimate close access tool for penetration testers and red teamers.
MIT License
462 stars 40 forks source link

WebServer times out when loading NoVNC #8

Closed i-am-shodan closed 4 weeks ago

i-am-shodan commented 1 month ago

USBArmyKnife uses a fork of the ESPAsyncWebServer. Whilst this has got a lot better under its new maintainer I've noticed a few issues where the webserver recieves a high volume of requests. This is especially clear when Chrome/Edge tries to load a ton of the files needed for NoVNC.

https://github.com/mathieucarbou/ESPAsyncWebServer/issues/70 maybe related

mathieucarbou commented 1 month ago

FYI:

I've just fixed a bug regarding string usages for ESP8266 (long time bug):

https://github.com/mathieucarbou/ESPAsyncWebServer/releases/tag/v3.3.16

If possible, please let me know if it solves the issue....

Regarding the slowdown, there were an issue in AsyncTCP lib for ESP32 which has been fixed also. For ESP32, you can use either AsyncTCP or AsyncTCPSock:

i-am-shodan commented 1 month ago

Interesting, have you got any more details on why constexp was bad. I'm using constexp in places.

mathieucarbou commented 1 month ago

Interesting, have you got any more details on why constexp was bad. I'm using constexp in places.

its not bad. It's just that for ESP8266, strings have to be put in flash with PROGMEM to not count against ram memory.

And the define was wrong (ESP8622 instead of ESP8266), which lead all the strings to be kept in ram instead of flash

https://github.com/mathieucarbou/ESPAsyncWebServer/commit/65906f7419442b7bd4764e9703f7401fd303e92d#diff-eff42aa28535f8fe20945bef89f9d8640da70cf6fe881c51b9af0b951b4b36c2R185

mathieucarbou commented 1 month ago

Oh boy! I didn't know your projects! That's cool: I didn't know the esp-idf api had a promiscuous mode!

i-am-shodan commented 1 month ago

@mathieucarbou I tried your changes and what I can say is that every release things seems to get a lot better. Sadly not well enough to resolve this issue for me.

In my case I'm using AsyncTCP and ESPAsyncWebServer straight from your git repo (thanks for all the work maintaining all of this by the way).

I'm still getting issues trying to load lots of files with varying sizes. Hitting F5 in the browser to try and get these to be returned often results in the ESPAsyncWebServer deadlocking - the device itself doesn't reset. Here is an example of what Edge sees:

image

Everything is going good and then one file fails to load :(

The files i'm trying to load are here: https://github.com/i-am-shodan/USBArmyKnife/tree/master/ui/vnc These are converted by a build script into gzipped compressed versions to save on RAM and transfer. Each page is defined as: const uint8_t PROGMEM bootstrapMinCssGz as an example

I then autogenerate a really big table of URL -> PROGMEM data pointer. When a request comes through this is looked up and sent across with:

https://github.com/i-am-shodan/USBArmyKnife/blob/cfc9637cc25eff88a2d941e8337008235e5194ed/src/Comms/Web/WebServer.cpp#L286

I wondering if you've got any ideas. Also no idea what AsyncTCPSock is and whether it could help here. Which is better?

mathieucarbou commented 1 month ago

Don't use _P methods on Esp32: they are deprecated

Also, have a look at the the ESPAsyncWebServer project page (my fork) there are some perf test and also some explications to swap AsyncTCP by AsyncTCPSock which is a better implementation

AsyncTCPSock was written by the same guy who also maintained ESPAsyncWebServer fork in the past and fixed many issues. My ESPAsyncWebServer fork is based on his fork and his AsyncTCPSock lib is based on bsd sockets and fixes the limitations over concurrent requests handling (16 max) for AsyncTCP and fixes it's design based on a slower queue system.

i-am-shodan commented 1 month ago

So I had a go migrating to AsyncTCPSock. Firstly it looks like it doesn't compile under the latest Arduino library. In AsyncTcp.h I had to change the code to this to get it to compile.

int res = getsockopt(_socket, IPPROTO_TCP, TCP_NODELAY, (char *)&flag, (socklen_t*) &size);

I ran the code and anecdotally it seemed worse that AsyncTCP. Not sure why exactly, just more failed requests.

mathieucarbou commented 1 month ago

Look here: https://github.com/mathieucarbou/ESPAsyncWebServer?tab=readme-ov-file#dependencies

It compiles, its tested and perf tested on Arduino 3 and latest rc1 also.

i-am-shodan commented 1 month ago

Using the code from the dev branch worked a treat. Default branch seemed to be the problem for me. However my testing sadly still stands and I don't see any improvement with AsyncTCPSock.

mathieucarbou commented 1 month ago

Do you have a waterfall view of your requests ? I know AsyncTCP has a concurrency limit (lwip layer) of 16 slots and cannot handle more than 16 clients at a time. AsyncTCPSock does better.

But what could happen is that your browser is querying too many resources at once, forcing the ESP to use more memory to serve them.

If you are forced to split into several files and cannot regroup in 1 minified bigger file, like all other ESP32 projects are doing usually, then, an option could be to use a JS library that will load these scripts one by one : did you try that ?

In any case, if you want the cause of this issue to be found, you need more data, like maybe print the free heap before serving fa file, print the pointers and size to be sure and their content in the console, etc.

There are too many hypothesis right now to troubleshoot anything.

i-am-shodan commented 1 month ago

I think one large file would probably solve the problem here. Because NoVNC is a library from elsewhere merging everything together isn't going to be trivial. I didn't consider a library to load them one by one, I'll check that out as it might be a good compromise.

mathieucarbou commented 1 month ago

I remember we were doing that with https://requirejs.org 10 years ago. But considering the recent evolution of JS, I bet it can be done in a few lines of code now...

i-am-shodan commented 4 weeks ago

I think i've fixed this by preloading all the resources one by one