Allow secure credentials to be retrieved from environment, not disk #159
Comments
|
@bhunut-adobe suggests that the Python KeyRing may be appropriate for Windows platforms, as it uses the Windows Credential Vault. On Mac it also supports the Keychain, and on many platforms it supports both SecretService and KWallet. Other credential stores are support for keyring by the keyrings.alt package. |
|
On AWS, all EC2 instance types support fetching credential files from S3 using IAM roles as a form of secure access. |
|
@adobeDan I look at Credential Manager class and there isn't much in there. Is it already implemented or it just a placeholder? Documentation on Credential Hook would be awesome. 👍 |
|
@bhunut-adobe It's just a placeholder, although it's compiled. The proposal is to document it and provide, perhaps, one implementation (say via KeyRing) for this release. Then to provide more implementations in the future. |
|
After discussing it this morning, @phil-levy and I have decided not to try and do anything without some more input from customers about what's needed and what might work. In particular, we don't think the credential manager as it stands is even worth documenting; probably we want to replace it with something much clearer and cleaner. So I am deferring this out of the 2.0 release, which we want to release today, and once we understand what customers need we can do a fast-follow with this enhancement. @bhunut-adobe, @jameslockman, @adorton-adobe, please note. We need your help getting clearer requirements for this feature from the field. |
|
@adobeDan They want to ensure that the password is not in cleartext in the config file. ACL on the file is not secure enough. If the password lives in the config file, it should at least be encrypted and can't be decrypted outside of that machine. The customer also recommending using azure vault credential but this won't be generic enough since most people will be running UST on-prem not azure. What would the timeline look like to get the new credential manager hook in place? |
|
@bhunut-adobe Thanks for the requirements update. That's from one customer, I gather. I'll look at some possible approaches and see if @phil-levy and I can agree on something. We should be able to estimate how long this will take by the end of this week. |
|
We have other customers. The clear text credential throws a yellow flag on almost every engagement, but conversations with Infosec typically move the engagement along. Every customer wants a more "enterprise" approach to storing this credential, and providing some interface to external encrypted credentials is welcome. |
|
So here's my proposal:
This will provide for both a no-code-required path and a custom-code path without rebuilding User Sync, and should cover the full range of InfoSec customer needs. @phil-levy @jameslockman @bhunut-adobe @adorton-adobe @Luci2015 Feedback requested. I am thinking that we would also provide sample external processes, one for AWS and one for Azure, that use role-based security to fetch the credentials. |
|
I like this approach. I also like Azure and AWS, as it will apply to some existing Azure customers. |
|
I was able to get a working POC for windows credential manager using the existing credential manager hook. Do you have an estimated timeline for this implementation? is this something that will happen in the next release? We have a meeting with the customer next week and wanted to see if it worth an effort to go with my fork build until the official release. |
|
Yes, go with your fork build, because based on detailed examination of implementation options I'm going to update my proposal to involve eliminating the credential manager and always rely on an external process to obtain secure credentials. And I don't want to do that release until we have some good examples of external processes that we release at the same time. Can you please make your credential-manager-based implementation available in a branch of this repo (if it's not already)? I am open to reviewing it and integrating the functionality directly (rather than via the credential manager) as part of the next release, if it looks clean enough and the external dependencies make sense on all platforms. |
|
@adobeDan If you can create windows_credential_manager branch in this repo and I will submit the pull request from my fork. |
|
@adobeDan Done. I forked Adobe v2 repo and created a new branch based on it. bhunut-adobe@27eb75e Let me know if you want me to submit a PR into Adobe or keep it separately. |
* allow specifying private key data in UMAPI config, using `priv_key_data` config setting (requires the YAML `|` convention with indented PEM key data on following lines) * get LDAP password from keyring, using `secure_password_key` config setting * get UMAPI client secret from keyring, using `secure_client_secret_key` config setting * get UMAPI private key data from keyring, using `secure_priv_key_data_key` config setting * remove the credential manager * clean up conventions around if parens and None comparisons in config.py * get rid of all PEP8 style warnings in config.py and the directory modules.
|
@adobeDan - Looking to test the latest change. Can you please provide an example of UMAPI config to use credential store? |
* Add get_credential accessor to DictConfig, and integrate all use of keyring there. * Refactor ldap and umapi connectors to use get_credential. * move to consistent naming of connector configs. * catch a duplicated function definition in umapi connector.
* added a counter to the excluded_stray case in rules.py * while I was there, cleaned up all PEP 8 style warnings (mostly redundant parentheses and better comparisons with None). * make the umapi api_key setting a credential (a 1-line change, thanks to the refactoring done in my last commit! :)
|
@bhunut-adobe I sent you the files separately. If you build the latest v2 it will have this change. |
Fix #159 - more secure credential handling
|
So this is not quite fixed yet, for a silly reason: I added a check for unused parameters in the server section of the UMAPI config file, and that check blows up if you just use the default configuration completely. So I am fixing this issue as part of #167. |
With this fix, we are unicode-compliant (even in Python 2.7): * add a `--config-file-encoding` command-line parameter, so the config files (which contain the group mapping) can contain non-ascii characters. * add a `string_encoding` setting to the directory and ldap configurations, which controls the encoding of the input values. * Update the sample config files to show the new settings. * require v2.4.1 of umapi-client, to ensure that it's unicode-capable and matches its validations to the server-side validations. * Fix #159 again by catching some edge cases around empty credential files. * Fix #173 again by actually counting the number of primary org users read from Adobe.
Customer feedback has made it clear that we need to support some more secure options (probably a number of different platform-specific options) for managing credentials. Since we already have the credential manager hook class for doing this, we should document it clearly and eventually provide some standard implementations that we release as part of the build.
The text was updated successfully, but these errors were encountered: