-
Notifications
You must be signed in to change notification settings - Fork 48
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
derrod
wants to merge
1
commit into
obsproject:master
Choose a base branch
from
derrod:libobs-keychain
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Summary | ||
|
||
This RFC proposes the addition of OS keychain APIs to the libobs platform utilities to faciliate secure storage of credentials. | ||
|
||
# Motivation | ||
|
||
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. | ||
|
||
# Detailed Changes | ||
|
||
This RFC proposes the addition of the following libobs API methods: | ||
|
||
- `bool os_keychain_available(void)` | ||
- `bool os_keychain_save(const char *label, const char *key, const char *data)` | ||
- `bool os_keychain_load(const char *label, const char *key, char **data)` | ||
- `bool os_keychain_delete(const char *label, const char *key)` | ||
|
||
As the API requires OS-specific implementations it shall become part of the `util/platform.h` header. | ||
|
||
**Implementation notes:** | ||
|
||
Operating-system backends: | ||
- Windows: `wincred.h` / `Cred*` Win32 API functions | ||
- macOS: Keychain Services API | ||
- 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". | ||
An entry can only be loaded or deleted if the label and key match the ones specified when saving. | ||
|
||
`os_keychain_available()` will always return `true` on Windows and macOS, on Linux it will return `true` if libobs was compiled with libsecret support, but does not indicate whether a keychain backend is available. | ||
|
||
**Usage examples:** | ||
Saving: | ||
```c | ||
const char *label = "OBS Studio Stream Keys"; | ||
const char *key = "StreamKey_abcef"; | ||
const char *value = "mystreamkey"; | ||
bool success = os_keychain_save(label, key, value); | ||
``` | ||
|
||
Loading: | ||
```c | ||
const char *label = "OBS Studio Stream Keys"; | ||
const char *key = "StreamKey_abcef"; | ||
char *value = NULL; | ||
bool success = os_keychain_load(label, key, &value); | ||
|
||
... do stuff ... | ||
|
||
if (value) | ||
bfree(value); | ||
``` | ||
|
||
Deletion: | ||
```c | ||
const char *label = "OBS Studio Stream Keys"; | ||
const char *key = "StreamKey_abcef"; | ||
bool success = os_keychain_delete(label, key); | ||
``` | ||
|
||
# Drawbacks | ||
|
||
Accessing keychain information can reduce portability of profiles by not storing information in config files. OBS Studio may opt out of using a keychain if it is running in portable mode. | ||
derrod marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
In some cases accessing the keychain may require user confirmation, the API may block in such cases, which has to be accounted for by API users. | ||
|
||
If no keychain is available API users will have to fall back to storing data otherwise (e.g. obs data or config files). | ||
|
||
If keychain operations fail API users may have to also fall back to an insecure storage method or warn the user of faiure. | ||
|
||
# Additional Information | ||
|
||
- Windows credential store documentation: https://learn.microsoft.com/en-us/windows/win32/api/wincred/ | ||
- macOS keychain documentation: https://developer.apple.com/documentation/security/keychain_services?language=objc | ||
- Linux/GNOME libsecret documentation: https://gnome.pages.gitlab.gnome.org/libsecret/ | ||
- Flatpak "Secret" Portal: https://docs.flatpak.org/en/latest/portal-api-reference.html#gdbus-org.freedesktop.portal.Secret | ||
- Pull request: https://github.com/obsproject/obs-studio/pull/9122 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 alibobs
functionality.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.