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

Support debuginfod #459

Open
bobrik opened this issue Jan 19, 2022 · 5 comments
Open

Support debuginfod #459

bobrik opened this issue Jan 19, 2022 · 5 comments

Comments

@bobrik
Copy link

bobrik commented Jan 19, 2022

There's a mention of debuginfod in the code from #427 (cc @philipc). I suggest we also add support for looking up in debuginfod.

If DEBUGINFOD_URLS is set, the the debug info can be obtained from:

  • {URL}/buildid/{BUILD_ID}/debuginfo

The simplest option is to call out to debuginfod-find. Another option is to download the file directly, but then you also need to think whether to cache it and where. LLVM uses a separate directory from debuginfod-client, for example.

Both options are likely to have issues with sandboxing. Thinking of sentry use-case, the panic handling thread probably has network access to send the report, so downloading might be more feasible than shelling out.

I can probably find time to work on this if there's a consensus how to best implement this.

@bjorn3
Copy link
Member

bjorn3 commented Jan 20, 2022

I think this shouldn't be enabled by default at least. Having the executable directly download it would cause bloat and would expose it to (more) untrusted data (which means it would start to be necessary to recompile with newer dependencies for security when doing so may not have been necessary before). Shelling out to debuginfod-find won't work if debuginfod isn't installed. In either case automatically downloading debuginfo without user consent is a potential privacy issue.

@bobrik
Copy link
Author

bobrik commented Jan 20, 2022

If you don't want debug info from debuginfod, then you don't set DEBUGINFOD_URLS. If you don't trust the debuginfod, then you definitely shouldn't set the variable. By default the variable is not set, so it's a no-op. Setting the variable sounds like a very explicit way to establish trust and consent.

Naturally, the there would be a check for debuginfod-find presence before shelling out. Once again, it's a no-op without the env variable set.

I'm not following the part about recompiling with newer dependencies. Could you elaborate?

@philipc
Copy link
Contributor

philipc commented Jan 20, 2022

If we do decide to support this, we shouldn't be reimplementing a debuginfod client in this crate, so decisions about where to store the downloaded file are not needed. Instead, we should use an existing client (e.g. debuginfod-find, or libdebuginfod).

Thinking of sentry use-case, the panic handling thread probably has network access to send the report, so downloading might be more feasible than shelling out.

Why does sentry need debuginfod support in this crate? I thought sentry worked by only doing the symbolication on the sentry server.

I think this shouldn't be enabled by default at least. Having the executable directly download it would cause bloat and would expose it to (more) untrusted data (which means it would start to be necessary to recompile with newer dependencies for security when doing so may not have been necessary before). Shelling out to debuginfod-find won't work if debuginfod isn't installed. In either case automatically downloading debuginfo without user consent is a potential privacy issue.

I agree. I was expecting debuginfod to use a local daemon for configuration and downloading, but instead every client has its own configuration, does the downloading itself, and stores the files in the user's directory? To me it seems poorly designed for a use case like backtraces where it isn't being interactively initiated by the user (as opposed to using a debugger).

If you don't want debug info from debuginfod, then you don't set DEBUGINFOD_URLS. If you don't trust the debuginfod, then you definitely shouldn't set the variable. By default the variable is not set, so it's a no-op.

Recent versions of fedora and debian set this automatically:
https://fedoraproject.org/wiki/Debuginfod#Configuration
https://wiki.debian.org/Debuginfod#TL.3BDR:_How_do_I_configure_my_system_to_use_debuginfod.3F

@bobrik
Copy link
Author

bobrik commented Jan 20, 2022

Why does sentry need debuginfod support in this crate? I thought sentry worked by only doing the symbolication on the sentry server.

I didn't think of this, but it sounds like it would work for Sentry.

Recent versions of fedora and debian set this automatically

Fedora does, but on Debian you need to do this explicitly. You also trust your distro to provide you with libc, so there's some level of trust there.

@workingjubilee
Copy link
Member

If this doesn't torch this crate's maintainability I'm fine with adding it as a feature, seems cool.

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

No branches or pull requests

4 participants