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

Add port, gdb_args, and gdbserver_args to gdb.debug() #2382

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

gfelber
Copy link

@gfelber gfelber commented Apr 3, 2024

added two arguments to gdb.debug()

  • port: specifies the port that should be used by the gdbserver, this is useful for alpine, because alpine doesn't support the default openssh port forwarding feature, due to a minimalistic version of openssh
  • gdb_args: allows forwarding arguments to the gdb binary spawned by gdb.attach() (just a passthrough)

also downgraded "GDB Python API is supported only for local processes" to a warning, because it seems to work.

These changes are made for compatibility with my pwntools "plugin" vagd which currently uses a workaround on Pwngd.debug() (basically the same behaviour as the patch)

+ port: specifies the port that should be used by the gdbserver (this is useful for alpine)
+ gdb_args: allows forwarding arguments to the gdb binary spawned by gdb.attach() (just a passthrough)

aslo downgraded "GDB Python API is supported only for local processes" to a warning, because it seems to work on some systems.
@gfelber
Copy link
Author

gfelber commented Apr 3, 2024

additionally added the optional argument gdbserver_args, which could be useful for some setups or troubleshooting

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.

Cool, thank you! Allowing more control over the way debugging is initiated seems useful for one-off problems.

Your "plugin" looks nice, do you have plans to upstream features? I'd rather have nice docker and qemu integration than patchwork changes to make the external tool work. :D

pwnlib/gdb.py Outdated Show resolved Hide resolved
pwnlib/gdb.py Outdated
@@ -612,7 +622,7 @@ def debug(args, gdbscript=None, exe=None, ssh=None, env=None, sysroot=None, api=
gdbscript = gdbscript or ''

if api and runner is not tubes.process.process:
raise ValueError('GDB Python API is supported only for local processes')
log.warn('GDB Python API is supported only for local processes')
Copy link
Member

Choose a reason for hiding this comment

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

I don't like changing this without tests that proove full support. Then we could just remove the warning altogether. Which runner did work for you other than a process?

Copy link
Author

Choose a reason for hiding this comment

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

ssh.process works for me.
This makes sense IMO, because gdb is still run on the host session therefore attaching the gdb api through a unix socket still works. Changing it into a warning for ssh.process and an error for everything else

Copy link
Author

Choose a reason for hiding this comment

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

7305502: GDB Python API is now a warning for ssh.process and an error for everything else

pwnlib/gdb.py Outdated Show resolved Hide resolved
pwnlib/gdb.py Outdated
if not port:
# Find what port we need to connect to
if ssh or context.native or (context.os == 'android'):
port = _gdbserver_port(gdbserver, ssh)
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 we should still parse the output even though the port is specified explicitly. There were problems with shells forking when gdbserver uses them causing stalls which were detected by a timeout on the message in _gdbserver_port recently. And this way we're sure gdbserver is up and running before trying to attach.

Copy link
Author

Choose a reason for hiding this comment

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

7305502: now using gdbserver_port to check if the correct port was set

CHANGELOG.md Outdated Show resolved Hide resolved
+ gdb.debug port is now also used for qemu
+ GDB Python API is now tested for tubes.process.process a warning for ssh.process and an error for everything else
+ updated docs to use mention that gdbserver ports are randomized by default
+ now using gdbserver_port to check if the correct port was set
+ fixed CHANGELOG.md structure
@gfelber
Copy link
Author

gfelber commented May 14, 2024

Addressed change requests in 7305502.

regarding:

Your "plugin" looks nice, do you have plans to upstream features? I'd rather have nice docker and qemu integration than patchwork changes to make the external tool work. :D

The initial reason this has turned into a "plugin" i maintain myself is so i can basically make changes whenever i encounter a problem or run into an issue (or want to add a new feature).

Generally speaking I don't mind adding the functionality to upstream, but this might require some time because

  • i will have to rewrite a lot of stuff to work with python2.7
  • i will have to read into the pwntools project to understand exactly how your docs, code structure and testing setups work
    I would probably also keep my repo around, so it's easier for me to add and test new stuff

Maybe we can move this discussion somewhere else? My discord tag is @gfelber (I'm also on the official discord server).

@peace-maker peace-maker changed the title improved gdb.debug() functionality Add port, gdb_args, and gdbserver_args to gdb.debug() May 23, 2024
raise ValueError('GDB Python API is supported only for local processes')
if runner is not ssh.process:
raise ValueError('GDB Python API is supported only for local processes')
log.warn('GDB Python API for ssh processes is not officially tested')
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to add the tests instead of this vague warning. I think we can copy the API tests and slap a ssh=shell argument to the .debug call.

See here for instructions on how to test locally. make -C travis/docker ANDROID=no TARGET=gdb.rst should do the job.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, lol. gdb tests are disabled in the docker testing setup apparently. CI will do the work then.

# GDB tests currently don't work inside Docker for unknown reasons.
# Disable these tests until we can get them working.
echo > ~/pwntools/docs/source/gdb.rst

Copy link
Member

Choose a reason for hiding this comment

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

I just tried running the gdb tests in docker and they all passed, so I guess we can just remove that line from doctest3 and include them.

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.

None yet

2 participants