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

[feature request] gracefully handle Secret Service absence #25

Closed
samuela opened this issue Mar 22, 2019 · 8 comments
Closed

[feature request] gracefully handle Secret Service absence #25

samuela opened this issue Mar 22, 2019 · 8 comments

Comments

@samuela
Copy link

samuela commented Mar 22, 2019

There are a number of scenarios in which there's either no Secret Service running on the machine or it's not available, eg. running in a Docker image. When trying to run an executable that depends on keyring-rs, the shell will respond with simply "No such file or directory". Debugging a bit shows us that the binary can't be run without gmp and dbus-1 available on the host system:

$ ldd nuvemfs-cli
	linux-vdso.so.1 (0x00007ffd69df9000)
	libgmp.so.10 => /usr/lib/x86_64-linux-gnu/libgmp.so.10 (0x00007f734d206000)
	libdbus-1.so.3 => not found
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f734ce15000)
	/lib/ld64.so.1 => /lib64/ld-linux-x86-64.so.2 (0x00007f734d487000)

It would be nice to be able to actually run the executable but return an error at runtime when the program attempts to access the keyring.

@hwchen
Copy link
Owner

hwchen commented Apr 5, 2019

ah, sorry I didn't respond! I thought I had.

I like this idea, I'll try to implement this soon.

As for the gmp dependency, removing that is part of a long-awaited refactor.

@hwchen
Copy link
Owner

hwchen commented Apr 20, 2019

hmmm... I thought more about the "file not found" error. I'm not sure how to handle this inside the library. secret service has to dynamically link libdbus, and from my understanding shared libraries are loaded before symbols in the executable.

The solution that comes to mind first is to put the keyring dependency behind a feature flag.

Let me know if you have any other ideas; I'm definitely not an expert in building and linking against shared libraries.

@samuela
Copy link
Author

samuela commented Apr 20, 2019

Hmm yeah I see how that gets complicated... This looks relevant though: https://www.reddit.com/r/rust/comments/3hrzsk/optionalruntime_loading_of_system_libraries/

@hwchen
Copy link
Owner

hwchen commented Apr 21, 2019

Interesting; am I correct in thinking that this would have to be implemented at the level of the wrapper around libdbus, or would a check at a higher level (in keyring) work? I'd have to do a decent bit more digging on my part to be sure, so if you happen to know it would help me out.

@samuela
Copy link
Author

samuela commented Apr 21, 2019

It looks like you could check in keyring pretty easily: https://doc.rust-lang.org/1.0.0/std/dynamic_lib/struct.DynamicLibrary.html. It would involve a limited bit of unsafe code but it would be possible.

@hwchen
Copy link
Owner

hwchen commented May 3, 2019

I don’t have the bandwidth to look into this right now, but it’s something I can look at for my next round of maintenance.

@adobeDan
Copy link
Collaborator

This looks like a candidate for an error code and improved handling in #70. I'll take a look.

@adobeDan
Copy link
Collaborator

So I've done the research and I stand corrected about this having anything to do with error handling. Although the Rust infrastructure for using dynamically loaded libraries has gotten pretty good over time, it would still be a tremendous amount of work for us to use secret-service this way. In fact, I believe it would be far easier for our clients to use this approach with keyring than for us to use it with secret-service. That is, if you want to make an application that uses keyring when secret-service available, then:

  1. make a dynamic library that statically links keyring (which has a super-simple interface) and exposes it.
  2. make your client check for the presence of secret-service before loading this dynamic library.

I'm going to close this request as an enhancement that we won't fix.

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

3 participants