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

Server does not validate that offer_id is 20 bytes #525

Open
minwidth0px opened this issue Aug 6, 2024 · 6 comments
Open

Server does not validate that offer_id is 20 bytes #525

minwidth0px opened this issue Aug 6, 2024 · 6 comments
Labels

Comments

@minwidth0px
Copy link

minwidth0px commented Aug 6, 2024

What version of this package are you using?

latest (11.1.1 according to package.json)

What operating system, Node.js, and npm version?

windows, v22.4.0, 10.8.1

What happened?

I ran the following client code on a local server which worked, but did not work on wss://tracker.webtorrent.dev. The code tries to send an offer from peer 1 to peer 2 and peer 2 to peer 1 using websockets.

<!DOCTYPE html>
<html lang="en">
<head>
  <title>WebTorrent Signaling</title>
</head>
<body>
  <script>

//try different trackers
//const url = 'wss://tracker.openwebtorrent.com'; //<-- works
//const url = 'ws://localhost:8000';//<--works
const url = 'wss://tracker.webtorrent.dev';//<--fails
//const url = 'wss://tracker.files.fm:7073'//<--works

const socket1 = new WebSocket(url);
const socket2 = new WebSocket(url);

const infoHash = '01234567891234567890';

const sleep = ms => new Promise(r => setTimeout(r, ms));

const socket1PeerId = 'abcdefghijklmnopqrst';
const socket2PeerId = '1bcdefghijklmnopqrst';

const socket1Offer = [
    {
        offer: {
            type: 'offer',
            sdp: 'mocked offer 1'
        },
        //edit: set to length 20 to fix the bug
        offer_id: '123456'
    },
]

const socket2Offer = [
    {
        offer: {
            type: 'offer',
            sdp: 'mocked offer 2'
        },
        //edit: set to length 20 to fix the bug
        offer_id: '654321'
    },
]

socket1.addEventListener('open', function (event) {
    console.log('Socket 1 connected');
    socket1.send(
        JSON.stringify({
            event: 'started',
            action: "announce",
            info_hash: infoHash,
            peer_id: socket1PeerId,
            numwant: 5,
        })
    );
});

socket1.addEventListener('message', async function (event) {
    console.log('Socket 1 received:', event.data);
    const message = JSON.parse(event.data);
    if (message.offer){
        await sleep(1000);
        socket1.send(
            JSON.stringify({
                event: 'completed',
                action: "announce",
                info_hash: infoHash,
                peer_id: socket1PeerId,
                numwant: 5,
                offers: socket1Offer,
            })
        );
    }
});

socket2.addEventListener('open', async function (event) {
    console.log('Socket 2 connected');
    await sleep(1000);
    socket2.send(
        JSON.stringify({
            event: 'started',
            action: "announce",
            info_hash: infoHash,
            peer_id: socket2PeerId,
            numwant: socket2Offer.length,
            offers: socket2Offer,
        })
    );
});

socket2.addEventListener('message', function (event) {
    console.log('Socket 2 received:', event.data);
});
  </script>
</body>
</html>

What did you expect to happen?

I expected the code to either work in the local server and the public tracker or in none, Since it fails on both wss://tracker.openwebtorrent.com and wss://tracker.webtorrent.dev that implies that it is a bug in the server implementation.

Are you willing to submit a pull request to fix this bug?

Yes

I installed the server via npm install -g bittorrent-tracker and ran bittorrent-tracker in console. I receive the following logs when running:

wss://tracker.webtorrent.dev:


Socket 1 connected
Socket 1 received: {"failure reason":"Invalid request"}
Socket 2 connected
Socket 2 received: {"failure reason":"Invalid request"}

local server, wss://tracker.openwebtorrent.com, wss://tracker.files.fm:7073


Socket 1 connected
Socket 1 received: {"action":"announce","interval":120,"info_hash":"01234567891234567890","complete":0,"incomplete":1}
Socket 2 connected
Socket 2 received: {"action":"announce","interval":120,"info_hash":"01234567891234567890","complete":0,"incomplete":2}
Socket 1 received: {"action":"announce","info_hash":"01234567891234567890","offer_id":"654321","peer_id":"1bcdefghijklmnopqrst","offer":{"type":"offer","sdp":"mocked offer 2"}}
Socket 2 received: {"action":"announce","info_hash":"01234567891234567890","offer_id":"123456","peer_id":"abcdefghijklmnopqrst","offer":{"type":"offer","sdp":"mocked offer 1"}}
Socket 1 received: {"action":"announce","interval":120,"info_hash":"01234567891234567890","complete":1,"incomplete":1}

wss://tracker.openwebtorrent.com sometimes does not send peer 1's offer to peer 2.

I think there should be better error messages for this case in the bittorrent-tracker server if it is not a bug and is intended.

Also, I would greatly appreciate if someone could help me write some code that actually works for tracker.webtorrent.dev (without using any dependencies on the client side). If you could give me some pointers on how the basic signaling works that would really help me. Right now I'm just trying to get the signaling to work. I already know how WebRTC in general works and have used it before (generate SDP offer and ICE candidates on peer A, send them to peer B, generate SDP answer and ICE candidates from peer B, send them to A). I'm just not sure how I can send these messages within the webtorrent protocol. Since there is no BEP it's hard to understand if this is a bug or not, or if this is just something tracker.webtorrent.dev is doing but is not part of the specification. The reference implementation is a bit hard to follow and I ended up having to run the server and client and print the messages to console to understand what is going on, and even then my implementation seems to not be fully correct, as it's not working for tracker.webtorrent.dev. In an earlier version of this post I still did not have wss://tracker.openwebtorrent.com working.

@minwidth0px minwidth0px changed the title different behaviour between local server and wss://tracker.webtorrent.dev Clarify some aspects of the signaling protocol (different behaviour between wss://tracker.webtorrent.dev and local server/other trackers) Aug 6, 2024
@SilentBot1
Copy link
Member

SilentBot1 commented Aug 17, 2024

Hi @minwidth0px,

From what I understand, Aquatic (which powers https://tracker.webtorrent.dev) is a lot more strict with parsing messages.

The error you are getting is due to the incoming message failing to be parsed as either an announcement message or a scrape request as they are parsed here.

In your example, your offer has an offer_id of 123456, wherein the Aquatic implementation, it must be 20 bytes. Updating your offer_id to a valid one causes your example to work and the 2nd socket to get the answer.

The bittorrent-tracker's implementation does not validate the length of the offer's offer_id, and this is transparently passed through with no validation, as seen here.

I hope this helps explain what you're seeing.

@minwidth0px
Copy link
Author

@SilentBot1 Thanks! Confirmed that setting the offer_id length to 20 allows it to work. Honestly I should have tried that earlier being that everything else needs to have length 20. I guess the next question is, is this a bug in Aquatic or bittorrent-tracker? It's somewhat of a meaningless question since there is no spec, but ideally these implementations should be the same, right? There isn't really an analog to offer_id in the bittorent protocol.

@ThaUnknown
Copy link
Member

its not a bug, since there's no specification to follow

@SilentBot1
Copy link
Member

The bittorrent-tracker client will always generate a 20 byte offer_id, as seen here.

The other trackers assume this is always the case, where as bittorrent-tracker doesn't, solely as there is no standard as to what the size of the field should be outside of the reference implementation here.

@minwidth0px
Copy link
Author

minwidth0px commented Aug 18, 2024

If bittorrent-tracker generates a 20 byte offer, we should probably add a check for that, right? Thinking about it logically, we should want the offer_id to be guaranteed to have high entropy as they should be randomly generated and we don't want collisions. So while not technically a bug. we should probably add a check for this in bittorrent-tracker server.

@minwidth0px minwidth0px changed the title Clarify some aspects of the signaling protocol (different behaviour between wss://tracker.webtorrent.dev and local server/other trackers) Server does not validate that offer_id is 20 bytes Aug 18, 2024
Copy link

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@github-actions github-actions bot added the stale label Oct 18, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 25, 2024
@SilentBot1 SilentBot1 added accepted and removed stale labels Nov 14, 2024
@SilentBot1 SilentBot1 reopened this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants