botocore icon indicating copy to clipboard operation
botocore copied to clipboard

Add keyring support to config credential provider

Open mwgamble opened this issue 7 years ago • 7 comments

This adds support for the keyring module to the config credential provider. This allows users to store just the access key in a config file while keeping the secret key in a more secure storage facility. The excellent keyring module handles all of the cross-platform code required to make this work.

There have been numerous discussions about this in the past, including some currently open feature requests. Support for it was in boto2, but never made it across to boto3/botocore. Despite a lot of work being put in for #72, that PR was never merged due to the belief that it would make more sense for the keyring to be set up as an external credential provider plugin.

I don't believe this is the optimal approach. The point of having keyring support at all is to automate the process of retrieving the credentials while still keeping the secret key in a secure storage facility. This process still requires some configuration - namely, the profile name and the access key. This configuration still has to go somewhere, and the AWS config file is the best choice as far as I can tell. It follows that integrating keyring support into the config credential provider makes the most sense.

The other reason for having this go into the core config credential provider rather than act as a third party plugin is down to the fact that a lot of the time users are not interacting with botocore directly. Rather, they are using the AWS CLI tool, or through ansible's various modules, or some other integration on top of botocore.

The issue of integrating keyring support so it can be used automatically by AWS CLI was raised again last year. Unfortunately, it appears no answer was provided by any of the boto maintainers. I definitely agree with @handlerbot that having this integrated into one of the projects as core functionality would make everyones' lives easier.

Please note that at this stage I have only added support for the AWS config file, due to the documentation stating that the boto config file is mainly for backwards compatibility with boto2. However I would be happy to update the PR and extend to the boto config file too if that is desired.

mwgamble avatar Aug 15 '17 00:08 mwgamble

Codecov Report

Merging #1262 into develop will increase coverage by 0.01%. The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1262      +/-   ##
===========================================
+ Coverage    98.05%   98.06%   +0.01%     
===========================================
  Files           45       45              
  Lines         7389     7402      +13     
===========================================
+ Hits          7245     7259      +14     
+ Misses         144      143       -1
Impacted Files Coverage Δ
botocore/credentials.py 97.97% <85.71%> (+0.23%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 69bbd96...f0e0644. Read the comment docs.

codecov-io avatar Aug 15 '17 00:08 codecov-io

I've updated this PR with some additional error handling, and some unit tests. I'd really appreciate any feedback on the implementation.

mwgamble avatar Sep 11 '17 01:09 mwgamble

Is there any chance of an update on this?

mwgamble avatar Sep 25 '17 04:09 mwgamble

looks like coverage @mwgamble

jamesongithub avatar Oct 05 '17 04:10 jamesongithub

@stealthycoin Wondering what the "large" label applied to this PR indicates, is it possible to have some feedback on anything else that could be done to increase the chances of this being looked at in the near(er) future?

lloydhazlett avatar Oct 18 '17 22:10 lloydhazlett

I'm curious on your thoughts about sourcing credentials from an external process addressing some of your concerns about getting keyring support in the CLI as well.

joguSD avatar Dec 08 '17 00:12 joguSD

Code here looks perfect. Any reason why this has had no review in all these years? I hate the fact that AWS credentials must be stored in plain text and unprotected.

WhyNotHugo avatar Apr 17 '20 09:04 WhyNotHugo

Hi all, thanks for your patience. I brought this PR up for discussion with the team and was advised that if this feature is still needed, it should be tracked as an issue in our cross-SDK repository (https://github.com/aws/aws-sdk). Any proposed changes that involve credentials need to be considered at a cross-SDK level in order to maintain consistency.

tim-finnigan avatar May 01 '23 22:05 tim-finnigan

Any proposed changes that involve credentials need to be considered at a cross-SDK level in order to maintain consistency.

I'm curious why you guys came to that conclusion. I don't have the intimate understanding of the AWS CLI and Boto framework that you guys do but from my perspective as a customer, all I really want to have the option to point the AWS CLI to a place in the MacOS Keychain, Windows Credential Manager, or whatever the Linux equivalent is instead of pointing the CLI to the plain-text credentials file.

I think it would make sense to set up a set of 'drivers' for fetching stored credentials, either from the default file or from the local credential manager, so there'd be no reason to alter the overall flow for handling credentials inside the rest of CLI's inner-workings, as they'd be a standard set of values coming from an arbitrary source the user's configured.

This would raise the bar on security as there'd be no plain-text secrets on the machine. IDK about everyone else but we use Identity Center in my org and most engineers are rarely interacting with the actual access tokens on the workstation, so it's no real loss in functionality for us... just tighter security and better compliance achievement :)

ptcrash avatar Apr 17 '24 18:04 ptcrash