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

wisp_network adapter #1097

Merged
merged 4 commits into from
Aug 27, 2024
Merged

wisp_network adapter #1097

merged 4 commits into from
Aug 27, 2024

Conversation

ProgrammerIn-wonderland
Copy link
Contributor

@ProgrammerIn-wonderland ProgrammerIn-wonderland commented Jul 30, 2024

WISP is a proxy protocol similar to SOCKS5 made by Mercury Workshop. It's documentation and a list of server implementation can be found at https://github.com/MercuryWorkshop/wisp-protocol and a demo server at wisps://wisp.mercurywork.shop/

This PR adds the ability to specify a wisp URL to use as a network proxy in the place of jor1k relay. The main benefit of this is that wisp is a lot less chatty since its not sending direct ethernet frames but rather just TCP and UDP payloads as needed, usually in 65k frame sizes instead of 1.5k ethernet frames. This results in a pretty good speed increase.

Here is some real world performance I get:
image

This PR only adds the TCP support and not UDP yet though UDP can be added in the future. Additionally it has a rudimentary DNS over HTTPS client which sends requests to google's DOH server to find the IP. It may not be a bad idea to make the DNS over HTTPS server a config option in the future?

Special Thanks:
@basicer For making fetch_network.js. This PR reuses a lot of what was introduced in that PR such as the TCP parser
@ading2210 For creating the wisp protocol and hosting wisp.mercurywork.shop
the rest of Mercury Workshop for helping me as I needed help
and ofc copy and the other v86 contributors for making this amazing project I've looked up to since I was a child

Copy link
Owner

@copy copy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks very useful.

Could you do one pass to make the style more like in the rest of the codebase? In particular:

  • {} after if and newline betweem ) and {
  • snake_case for identifiers

There should be at least one test case, see tests/devices/fetch_network.js for a reference. I'd suggest not using mocks and testing against the real wisp server, if that's okay. It's not necessary to run the test automatically on ci.

Also, it would be nice if you could add a short section at the bottom of docs/networking.md

src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Show resolved Hide resolved
@ProgrammerIn-wonderland
Copy link
Contributor Author

Yeah I can work on that

@ProgrammerIn-wonderland
Copy link
Contributor Author

further testing reveals that not all DNS over HTTPS servers are so kind to cross origin requests, dns.google worked but 1.1.1.1 gives a 400 error if its not on the same origin. Quad9, adguard, and the others I tried just fails with a cors error. I'm not as sure if it should be a configurable option since dns.google seems to be the only one friendly to cross origin

@ProgrammerIn-wonderland
Copy link
Contributor Author

I'm also running into this weird bug where

    const req = await fetch(`https://dns.google/resolve?name=${"google.com."}&type=${1}`,  {headers: [["accept", "application/dns-json"]]});
const res = await req.json()

will give res.Answer most of the time but suddenly it will disappear and not come back until you clear cookies, trying to figure that out as well

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 30, 2024

I'm wondering if using corsproxy.io to fix this issue would be appropriate, though it would get rid of the main point of DNS over HTTPS
edit: nope, corsproxy.io flags for automated use apparently, going to mark as draft until i figure out the whole DNS thing unfortunately

@ProgrammerIn-wonderland ProgrammerIn-wonderland marked this pull request as draft July 30, 2024 22:23
@SuperMaxusa
Copy link
Contributor

SuperMaxusa commented Jul 30, 2024

1.1.1.1 gives a 400 error if its not on the same origin

From https://developers.cloudflare.com/1.1.1.1/encryption/dns-over-https/make-api-requests/#return-codes:

400 DNS query not specified or too small.

Cloudflare uses a RFC 8484-compatible API which different then Google DNS JSON API, but Google responds with application/dns-message on /dns-query endpoint.
Anyways, they are same of functionallity (and works with CORS too).

> google = await fetch("https://dns.google/resolve?name=example.com", { headers: {"Accept": "application/dns-json"}});
> await google.json()
{
  Status: 0,
  TC: false,
  RD: true,
  ...
}

> cloudflare = await fetch("https://1.1.1.1/dns-query?name=example.com", { headers: {"Accept": "application/dns-json"}});
> await cloudflare.json()
{
  Status: 0,
  TC: false,
  RD: true,
  ...
}

> quad9 = await fetch("https://dns.quad9.net:5053/dns-query?name=example.com", { headers: {"Accept": "application/dns-json"}});
> await quad9.json()
{
  Status: 0,
  TC: false,
  RD: true,
  ...
}

EDIT: Quad9 works too but it uses port 5053, see https://www.quad9.net/news/blog/doh-with-quad9-dns-servers/:

This is now running on a custom port since this implementation is not inline with the most recent standards based DoH implementation. (see RFC 8484)

@ProgrammerIn-wonderland
Copy link
Contributor Author

I tried that and was complaining to one of my friends about how it feels like it should work but it doesn't. He advised me to check my network tab and pointed out the serviceworkers could intercept cross origin requests. As it turned out, that was my issue. None of the issues I described with Answer not happening existed, that was a testing error on my part, apologies

@copy
Copy link
Owner

copy commented Jul 30, 2024

@ProgrammerIn-wonderland Now that I think about it, couldn't dns requests be handled like regular wisp traffic? (that is, delete the special dns handling in this code and let it pass through as regular udp traffic)?

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 30, 2024

its possible, its just also common practice to disable UDP on wisp servers to disincentivize torrenting, I believe the public wisp.mercurywork.shop is one of these

edit: I'm mistaken, UDP is allowed on wisp.mercurywork.shop, I'll add support for it

edit 2: A concern I still have for UDP is DHCP, the server probably wouldn't be too happy with handling that, should there be certain ports handled internally and others sent?

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 30, 2024

I think other than the tests, the TCP + DOH is ready to go now
I'll start writing the tests, though I need to figure out how to run them

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 30, 2024

It seems to test Wisp network from nodejs I'd need the https://www.npmjs.com/package/ws npm package
there is extremely recent nodejs WebSocket support but it would require you to have a very recent (made after 2023, so node 20.10.xx) nodejs version to run the test, I'm not sure if thats appropriate or acceptable for this project nodejs/node#49830

@ProgrammerIn-wonderland
Copy link
Contributor Author

committed the test though, it does pass

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 31, 2024

I asked the hoster of wisp.mercurywork.shop, he is somewhat concerned about bandwidth usage since its hosted on a server with limited bandwidth monthly. Is it possible for you to be able to host one? Hosting a wisp server is a lot easier than hosting a jor1k relay since it doesn't need docker or dnsmasq, just a up to date version of nodejs or python (or rust) depending on which server you choose

@basicer
Copy link
Contributor

basicer commented Jul 31, 2024

This is super cool.

I wonder if a wisp server could be built out of a cloudflare worker.

For DNS, if google can give a message already encoded in the DNS wire format, why not just send that message right back down.

It seems like we should refactor out the TCP/Ethernet/UDP/etc stack from fetch instead of copying a bunch of the code. I'm pretty sure there's bound to be bugs / things to improve, especially in TCP and it would be a pain to have to do that in two places.

@copy
Copy link
Owner

copy commented Jul 31, 2024

edit 2: A concern I still have for UDP is DHCP, the server probably wouldn't be too happy with handling that, should there be certain ports handled internally and others sent?

Yes, dhcp (and arp) should be handled internally and dns should be sent off to whisp.

there is extremely recent nodejs WebSocket support but it would require you to have a very recent (made after 2023, so node 20.10.xx) nodejs version to run the test, I'm not sure if thats appropriate or acceptable for this project nodejs/node#49830

Yes, depending on node 20.x is fine (it's the current LTS).

I asked the hoster of wisp.mercurywork.shop, he is somewhat concerned about bandwidth usage since its hosted on a server with limited bandwidth monthly. Is it possible for you to be able to host one? Hosting a wisp server is a lot easier than hosting a jor1k relay since it doesn't need docker or dnsmasq, just a up to date version of nodejs or python (or rust) depending on which server you choose

If that's a concern, we could avoid mentioning wisp.mercurywork.shop, so most users wouldn't use it. I don't have any plans of hosting a proxy service myself, but if anybody else does I would add those to a dropdown on the website.

They are also free to apply stricter throttling to v86 users.

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 31, 2024

This is super cool.

I wonder if a wisp server could be built out of a cloudflare worker.

For DNS, if google can give a message already encoded in the DNS wire format, why not just send that message right back down.

It seems like we should refactor out the TCP/Ethernet/UDP/etc stack from fetch instead of copying a bunch of the code. I'm pretty sure there's bound to be bugs / things to improve, especially in TCP and it would be a pain to have to do that in two places.

We actually do have an impl that works on cloudflare workers, the main thing is the usage limits can make it hard to work with https://github.com/MercuryWorkshop/wisp-server-workers, we ran into limits like cf not being able to handle more than 6 concurrent connections iirc

Also good point on the DNS wire format, I may try to feed that directly in.

Also mainly, the code that wisp_network uses is defined in fetch_network.js, it doesn't redefine TCP connection or any of the bigger things such as the packet makers/handlers. I think it would be nice to have the code not specific just to fetch to be in a seperate file like network_lib.js or something which future high level networking implementations could use (or just these two, but it would maybe make sense for a split either way)

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Jul 31, 2024

undrafting since I think its fairly ready now
also the reason the CI test seems to be failing is having a nodejs version too old for WebSocket, is there a way to update the nodejs?

also it seems like websockets were introduced to be enabled by default in node v22, in v20.10 they required --experimental-websocket

@ProgrammerIn-wonderland ProgrammerIn-wonderland marked this pull request as ready for review July 31, 2024 18:03
@SuperMaxusa
Copy link
Contributor

is there a way to update the nodejs?

GitHub CI has a v20.15.1 in cached dependencies: https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#nodejs

And I think we could do a check version or WebSocket object at the beginning of the script like that:

// checking the major version
if (parseInt(process.versions.node.slice(0, 2)) < 20) {
	console.error("This version of Node.JS doesn't support running this test.");
	console.error("Minimal version: v22.x.x (or v20.10-21.x with --experimental-websocket flag)");
	process.exit(1);
}

// or websocket availability
if (typeof WebSocket === "undefined") {
	console.error("Your Node.JS doesn't support WebSocket.");
	console.error("If Node.JS version is v20.10-21.x, make sure the script was run with --experimental-websocket flag.");
	process.exit(1);
}

@ProgrammerIn-wonderland
Copy link
Contributor Author

I added the --expirimental-websocket flag I'm hoping that exposes the object

@ProgrammerIn-wonderland
Copy link
Contributor Author

also a slight update on wisp-server-workers, I gave some misleading information but its not 6 total streams, its 6 streams per wisp connection (each v86 Emulator connected to wisp-server-worker would get 6 concurrent TCP streams)

@ading2210
Copy link

If that's a concern, we could avoid mentioning wisp.mercurywork.shop, so most users wouldn't use it. I don't have any plans of hosting a proxy service myself, but if anybody else does I would add those to a dropdown on the website.

I'm honestly fine with wisp.mercurywork.shop being the default, but I would have to decrease the rate limits so I don't hit the 10TB free bandwidth limit that Oracle Cloud gives me. Perhaps v86 users could use an alternate endpoint (wss://wisp.mercurywork.shop/v86/) with low rate limits (something like 300kb/s?) so that I don't have to throttle everyone else.

@ProgrammerIn-wonderland
Copy link
Contributor Author

I'd also probably encourage you to keep a global counter that shuts down all endpoints at 9.5TB usage to avoid the oracle overage bill

@SuperMaxusa
Copy link
Contributor

I submitted a PR for updating Node.js: #1100

With Node.js v20.15.1 on local machine, Test #5: Curl example.org fails:

Preparing test #5: Curl example.org
Starting test #5: Curl example.org
Capturing...
    Captured: wget -T 10 -O - example.org
    Captured: echo -e done\\texample.org
    Captured: ~% wget -T 10 -O - example.org
    Captured: Connecting to example.org (93.184.215.14:80)
node:internal/event_target:1094
  process.nextTick(() => { throw err; });
                           ^

RangeError: Offset is outside the bounds of the DataView
    at DataView.getUint32 (<anonymous>)
    at $WispNetworkAdapter$$.process_incoming_wisp_frame (.../v86-wisp-net/build/libv86-debug.js:10241:104)
    at wispws.onmessage (.../v86-wisp-net/build/libv86-debug.js:10309:12)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:820:20)
    at WebSocket.dispatchEvent (node:internal/event_target:755:26)
    at fireEvent (node:internal/deps/undici/undici:10966:14)
    at websocketMessageReceived (node:internal/deps/undici/undici:10988:7)
    at ByteParser.run (node:internal/deps/undici/undici:11439:17)
    at ByteParser._write (node:internal/deps/undici/undici:11315:14)
    at writeOrBuffer (node:internal/streams/writable:564:12)

Node.js v20.15.1

On v20.16.x and v22.5.1 the test passes successfully.

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Aug 2, 2024

Is devicestest frozen in the CI? All the tests seemed to have passed but it's still counting minutes.
I may just add a process.exit(0) that runs after the all tests passed message, but I wonder what's keeping the test running

@copy
Copy link
Owner

copy commented Aug 2, 2024

Is devicestest frozen in the CI? All the tests seemed to have passed but it's still counting minutes.
I may just add a process.exit(0) that runs after the all tests passed message, but I wonder what's keeping the test running

It means there are still resources that can't be garbage-collected. Try setting the websocket to null or removing the event handlers from the websocket in destruct.

@ProgrammerIn-wonderland
Copy link
Contributor Author

Screenshot_20240803_162216-6
works as well

@ProgrammerIn-wonderland
Copy link
Contributor Author

this was an issue since the beginning but now that windows98 networking works perfectly I wonder why windows 2000 dhcp is still stalled
Screenshot_20240803_162216-7

@ProgrammerIn-wonderland
Copy link
Contributor Author

Screenshot_20240803_162216-8
the issue is just with dhcp too since if you specify the IP manually it doesn't have an issue

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Aug 3, 2024

dsl, haiku, reactos, win98, archlinux, linux4, buildroot all seem to work now, its really just windows 2000 remaining

@ProgrammerIn-wonderland
Copy link
Contributor Author

okay this is bizzare, it works fine with https://copy.sh/v86/?profile=windows2000-boot, just not snapshot windows 2k? I am not very sure why

@ProgrammerIn-wonderland
Copy link
Contributor Author

image
this is dhcp, no static trickery going on here

@ProgrammerIn-wonderland
Copy link
Contributor Author

issue seems to be with the snapshot itself, the current snapshot works fine with widgetry relay but not with fetch or wisp_networking, if I take a new snapshot it works with both

@ProgrammerIn-wonderland
Copy link
Contributor Author

ProgrammerIn-wonderland commented Aug 4, 2024

win2k-networking.bin.bz.txt
here is a new state that works for all (relay and wisp and fetch) testing (I called it .txt to upload to github but remove that, its a xz compressed file, I just accidentally called it .bz)

didn't do anything special to create it, I just loaded windows2000-boot for a bit and it just works, not sure why the old state didn't work for anything but relay

src/browser/wisp_network.js Outdated Show resolved Hide resolved
src/browser/wisp_network.js Outdated Show resolved Hide resolved
@copy
Copy link
Owner

copy commented Aug 24, 2024

This needs a rebase after #1113 and addressing the last two suggestions, then it should be good to merge.

@ProgrammerIn-wonderland
Copy link
Contributor Author

👍, I was busy this week due to standardized testing so I didn't have time to rebase, but I should be able to handle it soon

@ProgrammerIn-wonderland
Copy link
Contributor Author

accident

@ProgrammerIn-wonderland
Copy link
Contributor Author

It seems there some parts i cant reuse from fake_network this handle_fake_networking uses the fake DNS server, but other than that function I believe I can make it mostly work

@ProgrammerIn-wonderland
Copy link
Contributor Author

should be good now I think

@ProgrammerIn-wonderland ProgrammerIn-wonderland marked this pull request as ready for review August 24, 2024 22:54
copy.sh/v86/index.html Outdated Show resolved Hide resolved
}

if(packet.dns) {
// TODO: remove when this wisp client supports udp
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I would like to avoid DOH (behind a switch that is disabled by default), in order to avoid leaking hostnames to cloudflare. However, since this wisp implementation doesn't have UDP yet, I'll leave it with this comment for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable, I'm a bit busy this week but I'll see if I can make out time in the near future to add in UDP

handle_fake_ping(packet, this);
return;
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the fake ping implementation here, it's not very useful (and possibly confusing) when combined with a real networking proxy.

@copy
Copy link
Owner

copy commented Aug 27, 2024

If that's a concern, we could avoid mentioning wisp.mercurywork.shop, so most users wouldn't use it. I don't have any plans of hosting a proxy service myself, but if anybody else does I would add those to a dropdown on the website.

I'm honestly fine with wisp.mercurywork.shop being the default, but I would have to decrease the rate limits so I don't hit the 10TB free bandwidth limit that Oracle Cloud gives me. Perhaps v86 users could use an alternate endpoint (wss://wisp.mercurywork.shop/v86/) with low rate limits (something like 300kb/s?) so that I don't have to throttle everyone else.

Sure. We could also add a custom header (then other wisp relays could use apply throttling too). In any case, this relay is currently only used for tests, which causes very little traffic.

@copy copy merged commit 67bfed3 into copy:master Aug 27, 2024
3 checks passed
@copy
Copy link
Owner

copy commented Aug 27, 2024

Thanks everyone!

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.

5 participants