Skip to content

Conversation

ksshen0000
Copy link
Contributor

@ksshen0000 ksshen0000 commented Feb 18, 2024

Add native support for websocket.

this file are derived from pwntools-tube-websocket by frankli0324 under the MIT License.
https://gist.github.com/frankli0324/795162a14be988a01e0efa0531f7ac5a

@peace-maker peace-maker linked an issue Feb 20, 2024 that may be closed by this pull request
Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

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

Thank you for bringing this in! There are several things we need to consider before merging which I've mentioned inline.

Hi @frankli0324

Examples:
>>> ws = wstube('wss://echo.websocket.events')
Copy link
Member

Choose a reason for hiding this comment

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

Can we host a basic echo server ourselves in ci instead of relying on an external service? Those tend to be flakey and disturb the testing flow from time to time.


## 4.13.0 (`dev`)

- [#2350][2350] add native websocket support
Copy link
Member

Choose a reason for hiding this comment

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

Please add new entries to the bottom of the list

.. testsetup:: *

from pwn import *
from websocket import WebSocket, ABNF, WebSocketException, WebSocketTimeoutException
Copy link
Member

Choose a reason for hiding this comment

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

This isn't necessary since it's not used in the doc tests themselves

Arguments:
url (str): The websocket server's URL to connect to.
headers (dict): The same headers as the websocket protocol.
Copy link
Member

Choose a reason for hiding this comment

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

Aren't those additional headers to pass around?
How could we test this?

self.closed = False
self.sock = WebSocket()
self.url = url
self.sock.connect(url, header=headers)
Copy link
Member

Choose a reason for hiding this comment

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

I think it is worth it to forward other useful arguments to the constructor like host, origin, cookie, and subprotocols. As well as the other ones.

Copy link
Member

Choose a reason for hiding this comment

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

Handling the proxy configured in context.proxy would be great too!

self.url = url
self.sock.connect(url, header=headers)


Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a magic __call__ handler to forward calls to the underlying web socket instance like ping()

Some __getattr__ forward to e.g. get the optional close reason of the remote after the server closed the connection would be cool too.

except:
return False

def close(self):
Copy link
Member

Choose a reason for hiding this comment

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

It should be possible to specify the web socket close reason

self.sock.connect(url, header=headers)


def recv_raw(self, numb):
Copy link
Member

Choose a reason for hiding this comment

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

can_recv_raw needs to be implemented too to handle timeouts in the tube model.

@minpeter
Copy link

Is anyone working on this? I hope it will be merged by the third 2024 Q3 at the latest.

If there are no contributors working on it, I will try it.

@peace-maker peace-maker marked this pull request as draft October 26, 2024 12:09
@peace-maker
Copy link
Member

@minpeter Please open a new pull request if you start working on this. Closing for now since this has gone stale, feel free to reopen if you continue yourself @ksshen0000!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Native websocket support in pwntools
3 participants