Remove storage of credentials, in favor of storing ADFS session cookies.#2
Remove storage of credentials, in favor of storing ADFS session cookies.#2venth merged 1 commit intoventh:masterfrom brandond:store_session_not_creds
Conversation
|
I realize you may have some philosophical differences that prevent you from accepting this PR. I'd love to have my team use your module, but we had some concerns about how it's storing credentials - keeping the plaintext encryption key right next to the encrypted data is hardly better than just storing the password itself in plaintext. Storing the session cookies allows you to re-login to STS without entering credentials for an extended period of time, without having to store the user's actual credentials. It also lets an organization control the period in which a user can re-login to STS without entering credentials, by altering the ADFS session lifetime. |
|
Thanks for the contribution ;) Let me look into it. It seems that you solved my issue too ;) I've a password protected private key and looked for a solution that will work with ssh agent - I found one - Paramiko. Usage of this library would introduce unnecessary complexity into the tool. So, as I said, if your contribution would help me get rid completely of password storing thingy, then I'm more than happy. I need just a little time to verify how it works. |
|
Works like a charm. If I could only ask for two things:
If you prefer, I can introduce the changes by myself. Just let me know, what do you think about them. |
aws_adfs/prepare.py
Outdated
| # region: The default AWS region that this script will connect | ||
| # to for all API calls | ||
| config.region = 'eu-central-1' | ||
| config.region = 'us-east-1' |
There was a problem hiding this comment.
please consider keeping eu-central-1 as default 😄
Storing username+password encrypted with the user's SSH key is a nice idea, but presents a bit of a security concern - since anyone who can access your .aws/config file will also likely be able to access .ssh/id_rsa. PyCrypto doesn't support PEM Encryption, so we can't offer support for passphrase-protected RSA keys as a work-around. To address this, this patch removes credential storage entirely, in favor of storing the ADFS session cookie. This session cookie can be used to re-authenticate without credentials for an administratively configurable time period that is (by default) longer than the 1-hour lifetime of an AWS STS token.
|
OK, I fixed the umask used when creating the cookie jar. I also redid the default config to use botocore's session library, which takes care of things like honoring the AWS_CONFIG_FILE and AWS_CREDENTIAL_FILE environment variables. It will also pull region and format defaults from the default profile or profile specified by AWS_DEFAULT_PROFILE, falling back to local defaults if an existing profile is not available. I also switched from ConfigParser to the Python 3 configparser backport, which allows for creation of a 'default' section, and fixed a related issue in storing config in the 'default' profile. |
|
Thank you 👍 |
2 participants
Storing username+password encrypted with the user's SSH key is a nice idea, but presents a bit of a security concern - since anyone who can access your .aws/config file will also likely be able to access .ssh/id_rsa. PyCrypto doesn't support PEM Encryption, so we can't offer support for passphrase-protected RSA keys as a work-around.
To address this, this patch removes credential storage entirely, in favor of storing the ADFS session cookie. This session cookie can be used to re-authenticate without credentials for an administratively configurable time period that is (by default) longer than the 1-hour lifetime of an AWS STS token.