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 DPAPI based credential store on Windows #464

Merged
merged 7 commits into from
Sep 27, 2021

Conversation

mjcheetham
Copy link
Collaborator

Introduce a new credential store for Windows that uses DPAPI to encrypt the credentials on disk.

The default credential store on Windows will continue to be the Windows Credential Manager (wincredman), but by offering DPAPI we will allow the following limitations to be overcome:

  • Network logon sessions (such as over SSH) cannot persist credentials
  • There is a small maximum size of credential that can be stored (2560 bytes)

We only continue to use the wincredman solution as it offers a reasonable GUI for viewing and deleting stored credentials for the user. The plan would be to change the default to the DPAPI credential store if/when we even create our own easy to use GUI for managing GCM's stored credentials.

As part of this change it is now possible to also use the plaintext credential store on Windows and macOS, following the same model as we have for selecting the credential store on Linux. The only difference is that Windows and macOS have a default store, and on Linux you must select one.

Fixes #452

Implement a unified proxy pattern for the available credential stores.
This allows users to select from multiple stores on all platforms, as we
currently support for Linux.
Add a new credential store on Windows that uses DPAPI to encrypt the
password field in a file. The file format is otherwise identical to the
Plaintext credential store.

The default location for the DPAPI store is ~/.gcm/dpapi_store and this
can be configured with the GCM_DPAPI_STORE_PATH or
credential.dpapiStorePath.

The addition of the DPAPI credential store allows for secure credential
storage on Windows in cases where the Windows Credential Manager cannot
work/has limitations. Specifically, DPAPI works over network sessions
(like SSH) and is not subject to size limits.
Add an explicit test for credential persistence. If Windows indicates
that either we don't have a credential set for the logon session, or the
maximum allowed persistence for generic credentials is less than "local
machine" then fail.
Copy link
Contributor

@vtbassmatt vtbassmatt left a comment

Choose a reason for hiding this comment

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

Approving at a conceptual level + doc review. Nice!

Copy link
Contributor

@vdye vdye left a comment

Choose a reason for hiding this comment

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

Looks great! The only thing I'd like (if it was easier to integrate) would be tests for the DPAPI credential manager, but it looks like that's something that would be better suited to end-to-end tests anyway.

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

I don't know enough about DPAPI to know if there are any gotchas with that integration, but the code looks well structured.

@mjcheetham
Copy link
Collaborator Author

I don't know enough about DPAPI to know if there are any gotchas with that integration [..]

The DPAPI interface as exposed in .NET (ProtectedData) is very simple.. just those Protect and Unprotect calls with optional entropy (set to null for us). This protects the data for the current user.

This is the same pattern that MSAL.NET and Visual Studio use for protecting their token caches.
https://github.com/AzureAD/microsoft-authentication-extensions-for-dotnet/blob/835106e1148bc05405fb5fff3e235feb22b05a63/src/Microsoft.Identity.Client.Extensions.Msal/Accessors/DpApiEncryptedFileAccessor.cs#L38-L61

@mjcheetham
Copy link
Collaborator Author

The only thing I'd like (if it was easier to integrate) would be tests for the DPAPI credential manager [..]

That should be easy to add actually in the existing "unit" tests!

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.

Create alternative Windows credential store using DPAPI
4 participants