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

Add server module #1067

Merged
merged 8 commits into from Dec 11, 2017

Conversation

Projects
None yet
2 participants
@devneal
Contributor

devneal commented Nov 13, 2017

server

Adds a class pwnlib.tubes.server.server, similar to pwnlib.tubes.listen.listen, but able to accept multiple connections.

A few things worth mentioning that I'm unsure how to fix:

  • There is no way to kill the server other than using pkill
  • python -m doctest server.py hangs upon creation of the server
  • I'd like to add functionality similar to spawn_process for pwnlib.tubes.listen.listen, but I'm not sure how to best go about implementing this

devneal added some commits Nov 13, 2017

Show outdated Hide outdated pwnlib/tubes/server.py Outdated
Show outdated Hide outdated pwnlib/tubes/server.py Outdated
Show outdated Hide outdated pwnlib/tubes/server.py Outdated
@zachriggle

This comment has been minimized.

Show comment
Hide comment
@zachriggle

zachriggle Nov 13, 2017

Contributor

Thanks for this! Overall it looks good, with only some minor tweaks necessary.

Because this is ~50% copy-pasted from listen.py, it might make sense to add this functionality to the listen class rather than a whole new one.

Contributor

zachriggle commented Nov 13, 2017

Thanks for this! Overall it looks good, with only some minor tweaks necessary.

Because this is ~50% copy-pasted from listen.py, it might make sense to add this functionality to the listen class rather than a whole new one.

continue
h.status("Trying %s" % self.sockaddr[0])
listen_sock = socket.socket(self.family, self.type, self.proto)

This comment has been minimized.

@zachriggle

zachriggle Nov 13, 2017

Contributor

We never set self.sock, which makes inheritance from sock behave oddly. Consider setting self.sock = listen_sock once we have the listener.

@zachriggle

zachriggle Nov 13, 2017

Contributor

We never set self.sock, which makes inheritance from sock behave oddly. Consider setting self.sock = listen_sock once we have the listener.

This comment has been minimized.

@devneal

devneal Nov 14, 2017

Contributor

I also ended up setting self.rhost and self.rport on each new connection so that closing wouldn't crash.

@devneal

devneal Nov 14, 2017

Contributor

I also ended up setting self.rhost and self.rport on each new connection so that closing wouldn't crash.

Show outdated Hide outdated pwnlib/tubes/server.py Outdated
Show outdated Hide outdated pwnlib/tubes/server.py Outdated

@zachriggle zachriggle self-assigned this Nov 13, 2017

@zachriggle zachriggle added the feature label Nov 13, 2017

@devneal

This comment has been minimized.

Show comment
Hide comment
@devneal

devneal Nov 14, 2017

Contributor

I'd like to keep the two modules separate until the server is ready, then see if combining the two is still feasible.

Contributor

devneal commented Nov 14, 2017

I'd like to keep the two modules separate until the server is ready, then see if combining the two is still feasible.

@devneal

This comment has been minimized.

Show comment
Hide comment
@devneal

devneal Nov 21, 2017

Contributor

Any update on the review?

Contributor

devneal commented Nov 21, 2017

Any update on the review?

@zachriggle

This comment has been minimized.

Show comment
Hide comment
@zachriggle

zachriggle Nov 23, 2017

Contributor
Contributor

zachriggle commented Nov 23, 2017

@devneal

This comment has been minimized.

Show comment
Hide comment
@devneal

devneal Dec 4, 2017

Contributor

Any update on this?

Contributor

devneal commented Dec 4, 2017

Any update on this?

@zachriggle

This comment has been minimized.

Show comment
Hide comment
@zachriggle

zachriggle Dec 4, 2017

Contributor

You need to add a server.rst file like listen.rst in docs/source so that the doctests are actually executed and run. Then, the doctests need to pass :)

Contributor

zachriggle commented Dec 4, 2017

You need to add a server.rst file like listen.rst in docs/source so that the doctests are actually executed and run. Then, the doctests need to pass :)

Show outdated Hide outdated pwnlib/tubes/server.py Outdated

devneal added some commits Dec 8, 2017

@devneal

This comment has been minimized.

Show comment
Hide comment
@devneal

devneal Dec 9, 2017

Contributor

I've added a server.rst file, but there is no listen.rst to go off of.

Contributor

devneal commented Dec 9, 2017

I've added a server.rst file, but there is no listen.rst to go off of.

@zachriggle

This comment has been minimized.

Show comment
Hide comment
@zachriggle

zachriggle Dec 11, 2017

Contributor

It looks like we actually just put them all in sockets.rst, so I was wrong about needing server.rst. Just mention the module in sockets.rst.

https://github.com/Gallopsled/pwntools/blob/dev/docs/source/tubes/sockets.rst

Thanks!

Contributor

zachriggle commented Dec 11, 2017

It looks like we actually just put them all in sockets.rst, so I was wrong about needing server.rst. Just mention the module in sockets.rst.

https://github.com/Gallopsled/pwntools/blob/dev/docs/source/tubes/sockets.rst

Thanks!

devneal added some commits Dec 11, 2017

@zachriggle zachriggle merged commit ab8879a into Gallopsled:dev Dec 11, 2017

3 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 58.071%
Details

@zachriggle zachriggle added this to the 3.12.0 milestone Jan 2, 2018

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