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 RFC: libobs keychain APIs #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

derrod
Copy link
Member

@derrod derrod commented Jun 14, 2023

Summary

Proposes a libobs platform API for keychain access which can be used to securely store credentials.

Motivation and Context

While the credentials stored by OBS in profiles are not of severe sensitivity due to limited scope, as a modern desktop application we should still do our best to store them securely.

Link to RFC

@derrod
Copy link
Member Author

derrod commented Jun 24, 2023

In implemeting this in obsproject/obs-studio#9122 I was wondering if perhaps there is a need for something like bool os_keychain_available(void) to query keychain support? This may be important mostly on Linux where the keychain is not guaranteed to be provided by the OS e.g. to be able to determine whether or not to fall back to unsecured file storage or warn the users.

@derrod
Copy link
Member Author

derrod commented Jun 25, 2023

Updated with os_keychain_available() and a label parameter that is used to specify a user-facing name for the group/service a keychain entry belongs to, e.g. "OBS Studio OAuth Credentials".

- Linux: `libsecret`

The `label` is a user-visible component that may be displayed in keychain management applications as the label or name of an entry. It must be specified, ASCII only, and must be human-readable.
In most cases it should identify the component and contents, e.g. for OAuth credentials stored by the OBS Studio UI this could be "OBS Studio OAuth Credentials".
Copy link
Member

Choose a reason for hiding this comment

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

For better discoverability (and transparency) I'd prefer if any keychain item is automatically prefixed with "OBS Studio" followed by the label chosen by any given implementation.

This would reduce the need for implementors to be "good citizens" and prefixing their labels themselves.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, though that gets into the libobs vs OBS Studio discussion. While I personally think that this distinction doesn't matter in any practical sense, others have disagreed. Perhaps this could be solved by setting a prefix to use on libobs initialisation or something.

Copy link
Member

Choose a reason for hiding this comment

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

Right, that would require an API function that allows the frontend to set the prefix.

Is this functionality serving a need of libobs or of our UI? Because if it's the latter, code would only be implemented in the UI and as such the implementation can use the default "OBS Studio" prefix, because it's not a libobs functionality.

Copy link
Member Author

Choose a reason for hiding this comment

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

With the goal of migrating service integrations into plugins, I believed it would make the most sense to have this functionality within libobs rather than the frontend. Implementing it within the frontend API may also be possible, I'm not sure if there are any plugins that store credentials (e.g. websockets, cloud captions) that don't also link against the frontend.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that makes sense then.

I wouldn't mind if the prefix ends up being libobs, though a "product name" would be common as a prefix and probably also more recognisable to users.

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

3 participants