Windows Credential blob is being stored in an unexpected way

[zschreur]

Overview

According to Microsofts documentation: when Type is CRED_TYPE_GENERIC, the CredentialBlob field type will be defined by the application.

From my understanding, this means that the application storing the credential blob is the one that decides what data is stored. This breaks on Windows when the string's bytes are converted to UTF-16 and also when the credential is read as UTF-16 instead of as a &str.

With the documentation stating: "If the Type member is CRED_TYPE_GENERIC, this member is defined by the application", I expect that when I call &str keyring will then take the as_bytes and store that.

Motivation

First Issue: Compatability

I am hoping to find an alternative to node-keytar because it is an archived repository. There is a rust crate keytar-rs, which recommends using keyring-rs instead.

On MacOS, keytar and keyring play nicely together. Any &str that is stored with keytar is then read back in keyring as the exact same &str. However on Windows, they do not play well together.

My expectation (note this is more so pseudocode for simplicity)

// set in keytar
keytar.set_password("service", "user", "secret");

// get in keytar
keytar.get_password("service", "user") // => "secret"

// get in keyring
keyring.get_password("service", "user") // => "secret"

Actual

// set in keytar
keytar.set_password("service", "user", "secret");

// get in keytar
keytar.get_password("service", "user") // => "secret"

// get in keyring
keyring.get_password("service", "user") // => "敳牣瑥"

// OR if the password is an odd number of bytes it returns an Error

Second Issue: Storing Authentication Strings

Our use of keytar has been to store long authentication tokens. For a utf-8 string, the Windows length requirement is more than enough space. However, if the string is converted into utf-16 for storage, it becomes too long.

When converting the string, the credential stored is not valid for the application and in many cases it becomes too long and thus is unable to be stored.

[brotskydotcom]

Thanks, @zschreur, for this carefully reasoned bug report. I especially appreciate your providing info about the keytar module, and its reference to keyring as a possible replacement. That really puts your needs into clear perspective.

So, as the old saying goes, I have good news and bad news :). The bad news is that I am not willing to suddenly change the native format keyring uses for Unicode passwords on Windows (more on that below). The good news is that I'm happy to work with you to extend the keyring interface to explicitly support Vec data (in addition to strings) for passwords on all platforms. (It's always been a pet peeve of mine that it doesn't.)

First, let me explain my motivation around the "bad news." While you are correct that the Windows API gives the authoring application authority over the format of credential data, that observation does not allow keyring to arbitrarily define how it should write credentials. Keyring was originally written to interoperate both with Python's keyring module and with native Windows programs that write string credentials using the Credential Manager, and all of those utilities use UTF-16 as the credential format, because that's the native Windows format for strings. (For more information about this, and to see how the Windows platform APIs handle generic credential data, I recommend this article on the Windows OSHub site.)

Second, let me suggest to you that the good news gets you what you want. By elevating your suggested set_password_blob and get_password_blob functions to the generic keyring API, and supporting them on all platforms, you can both get compatibility with keytar-stored passwords and get back the full storage capacity of credentials on Windows.

Please let me know your thoughts. Are you willing to extend your PR so that it adds Vec<u8> password support to the platform-independent keyring API? And then provide support for that data in all the platform keystores?

[zschreur]

Thank you for taking the time to look over this. I appreciate the added context of being compatible with the python keyring library.

As for the set_password_blob and get_password_blob functions, I'd be happy to add those to the generic api and extend the previous PR. Though I may not get around to it for a little bit.

Thank you again!

[brotskydotcom]

OK, I'm closing this issue and opening an enhancement issue for the API extension. Whenever you're ready to submit a PR, please tag that issue in the PR comments, thanks!