Improve searching for configured AWS credentials
The previous approach for finding AWS credentials was pretty naive and only covered contents of a single file (~/.aws/credentials by default). The AWS CLI documentation states various other ways to configure credentials which weren't covered: https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#credentials Even that aren't all ways, a look into the code shows: https://github.com/boto/botocore/blob/develop/botocore/credentials.py This commit changes the behavior so the hook will behave in a way that if the AWS CLI is able to obtain credentials from local files, the hook will find them as well. The changes in detail are: - detect AWS session tokens and handle them like secret keys. - always search credentials in the default AWS CLI file locations ( ~/.aws/config, ~/.aws/credentials, /etc/boto.cfg and ~/.boto) - detect AWS credentials configured via environment variables in AWS_SECRET_ACCESS_KEY, AWS_SECURITY_TOKEN and AWS_SESSION_TOKEN - check additional configuration files configured via environment variables (AWS_CREDENTIAL_FILE, AWS_SHARED_CREDENTIALS_FILE and BOTO_CONFIG) - print out the first four characters of each secret found in files to be checked in, to make it easier to figure out, what the secrets were, which were going to be checked in - improve error handling for parsing ini-files - improve tests There is a major functional change introduced by this commit: Locations the AWS CLI gets credentials from are always searched and there is no way to disable them. --credentials-file is still there to specify one or more additional files to search credentials in. It's the purpose of this hook to find and check files for found credentials, so it should work in any case. As this commit also improves error handling for not-existing or malformed configuration files, it should be no big deal. Receiving credentials via the EC2 and ECS meta data services is not covered intentionally, to not further increase the amount of changes in this commit and as it's probably an edge case anyway to have this hook running in such an environment.
This commit is contained in:
Daniel Roschka
parent
9573c13884
commit
b0d4cdb1ee
|
|
@ -40,7 +40,12 @@ Add this to your `.pre-commit-config.yaml`
|
|||
- `check-xml` - Attempts to load all xml files to verify syntax.
|
||||
- `check-yaml` - Attempts to load all yaml files to verify syntax.
|
||||
- `debug-statements` - Check for pdb / ipdb / pudb statements in code.
|
||||
- `detect-aws-credentials` - Checks for the existence of AWS secrets that you have set up with the AWS CLI.
|
||||
- `detect-aws-credentials` - Checks for the existence of AWS secrets that you
|
||||
have set up with the AWS CLI.
|
||||
The following arguments are available:
|
||||
- `--credential-file` - additional AWS CLI style configuration file in a
|
||||
non-standard location to fetch configured credentials from. Can be repeated
|
||||
multiple times.
|
||||
- `detect-private-key` - Checks for the existence of private keys.
|
||||
- `double-quote-string-fixer` - This hook replaces double quoted strings
|
||||
with single quoted strings.
|
||||
|
|
|
|||
|
|
@ -7,62 +7,118 @@ import os
|
|||
from six.moves import configparser
|
||||
|
||||
|
||||
def get_your_keys(credentials_file):
|
||||
"""reads the secret keys in your credentials file in order to be able to
|
||||
look for them in the submitted code.
|
||||
def get_aws_credential_files_from_env():
|
||||
"""Extract credential file paths from environment variables."""
|
||||
files = set()
|
||||
for env_var in {'AWS_CREDENTIAL_FILE', 'AWS_SHARED_CREDENTIALS_FILE',
|
||||
'BOTO_CONFIG'}:
|
||||
try:
|
||||
files.add(os.environ[env_var])
|
||||
except KeyError:
|
||||
pass
|
||||
return files
|
||||
|
||||
|
||||
def get_aws_secrets_from_env():
|
||||
"""Extract AWS secrets from environment variables."""
|
||||
keys = set()
|
||||
for env_var in {'AWS_SECRET_ACCESS_KEY', 'AWS_SECURITY_TOKEN',
|
||||
'AWS_SESSION_TOKEN'}:
|
||||
try:
|
||||
keys.add(os.environ[env_var])
|
||||
except KeyError:
|
||||
pass
|
||||
return keys
|
||||
|
||||
|
||||
def get_aws_secrets_from_file(credentials_file):
|
||||
"""Extract AWS secrets from configuration files.
|
||||
|
||||
Read an ini-style configuration file and return a set with all found AWS
|
||||
secret access keys.
|
||||
"""
|
||||
aws_credentials_file_path = os.path.expanduser(credentials_file)
|
||||
if not os.path.exists(aws_credentials_file_path):
|
||||
return None
|
||||
return set()
|
||||
|
||||
parser = configparser.ConfigParser()
|
||||
parser.read(aws_credentials_file_path)
|
||||
try:
|
||||
parser.read(aws_credentials_file_path)
|
||||
except configparser.MissingSectionHeaderError:
|
||||
return set()
|
||||
|
||||
keys = set()
|
||||
for section in parser.sections():
|
||||
keys.add(parser.get(section, 'aws_secret_access_key'))
|
||||
for var in {'aws_secret_access_key', 'aws_security_token',
|
||||
'aws_session_token'}:
|
||||
try:
|
||||
keys.add(parser.get(section, var))
|
||||
except configparser.NoOptionError:
|
||||
pass
|
||||
return keys
|
||||
|
||||
|
||||
def check_file_for_aws_keys(filenames, keys):
|
||||
"""Check if files contain AWS secrets.
|
||||
|
||||
Return a list of all files containing AWS secrets and keys found, with all
|
||||
but the first four characters obfuscated to ease debugging.
|
||||
"""
|
||||
bad_files = []
|
||||
|
||||
for filename in filenames:
|
||||
with open(filename, 'r') as content:
|
||||
text_body = content.read()
|
||||
if any(key in text_body for key in keys):
|
||||
# naively match the entire file, low chance of incorrect collision
|
||||
bad_files.append(filename)
|
||||
|
||||
for key in keys:
|
||||
# naively match the entire file, low chance of incorrect
|
||||
# collision
|
||||
if key in text_body:
|
||||
bad_files.append({'filename': filename,
|
||||
'key': key[:4].ljust(32, str('*'))})
|
||||
return bad_files
|
||||
|
||||
|
||||
def main(argv=None):
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument('filenames', nargs='*', help='Filenames to run')
|
||||
parser.add_argument('filenames', nargs='+', help='Filenames to run')
|
||||
parser.add_argument(
|
||||
'--credentials-file',
|
||||
default='~/.aws/credentials',
|
||||
dest='credential_files',
|
||||
action='append',
|
||||
default=['~/.aws/config', '~/.aws/credentials', '/etc/boto.cfg',
|
||||
'~/.boto'],
|
||||
help=(
|
||||
'location of aws credentials file from which to get the secret '
|
||||
"keys we're looking for"
|
||||
),
|
||||
'Location of additional AWS credential files from which to get '
|
||||
'secret keys from'
|
||||
)
|
||||
)
|
||||
args = parser.parse_args(argv)
|
||||
keys = get_your_keys(args.credentials_file)
|
||||
|
||||
credential_files = set(args.credential_files)
|
||||
|
||||
# Add the credentials files configured via environment variables to the set
|
||||
# of files to to gather AWS secrets from.
|
||||
credential_files |= get_aws_credential_files_from_env()
|
||||
|
||||
keys = set()
|
||||
for credential_file in credential_files:
|
||||
keys |= get_aws_secrets_from_file(credential_file)
|
||||
|
||||
# Secrets might be part of environment variables, so add such secrets to
|
||||
# the set of keys.
|
||||
keys |= get_aws_secrets_from_env()
|
||||
|
||||
if not keys:
|
||||
print(
|
||||
'No aws keys were configured at {0}\n'
|
||||
'Configure them with --credentials-file'.format(
|
||||
args.credentials_file,
|
||||
),
|
||||
)
|
||||
print('No AWS keys were found in the configured credential files and '
|
||||
'environment variables.\nPlease ensure you have the correct '
|
||||
'setting for --credentials-file')
|
||||
return 2
|
||||
|
||||
bad_filenames = check_file_for_aws_keys(args.filenames, keys)
|
||||
if bad_filenames:
|
||||
for bad_file in bad_filenames:
|
||||
print('AWS secret key found: {0}'.format(bad_file))
|
||||
print('AWS secret found in {filename}: {key}'.format(
|
||||
**bad_file))
|
||||
return 1
|
||||
else:
|
||||
return 0
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
# this is an aws credentials configuration file. obviously not real credentials :P
|
||||
# file with AWS access key ids, AWS secret access keys and AWS session tokens in multiple sections
|
||||
[default]
|
||||
aws_access_key_id = AKIASLARTIBARTFAST11
|
||||
aws_secret_access_key = 7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e
|
||||
|
|
@ -8,3 +8,5 @@ aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
|
|||
[staging]
|
||||
aws_access_key_id = AKIAJIMMINYCRICKET0A
|
||||
aws_secret_access_key = ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez
|
||||
[test]
|
||||
aws_session_token = foo
|
||||
|
|
@ -1,5 +1,4 @@
|
|||
#file with a secret key, you'll notice it is a section of sample_aws_credentials
|
||||
|
||||
# file with an AWS access key id and an AWS secret access key
|
||||
[production]
|
||||
aws_access_key_id = AKIAVOGONSVOGONS0042
|
||||
aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
# file with an AWS access key id, an AWS secret access key and an AWS session token
|
||||
[production]
|
||||
aws_access_key_id = AKIAVOGONSVOGONS0042
|
||||
aws_secret_access_key = z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb
|
||||
aws_session_token = foo
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
# file with an AWS session token
|
||||
[production]
|
||||
aws_session_token = foo
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
# file with an AWS access key id but no AWS secret access key
|
||||
[production]
|
||||
aws_access_key_id = AKIASLARTIBARTFAST11
|
||||
|
|
@ -1,5 +0,0 @@
|
|||
# file with an access key but no secrets
|
||||
# you'll notice it is a redacted section of sample_aws_credentials
|
||||
|
||||
[production]
|
||||
aws_access_key_id = AKIASLARTIBARTFAST11
|
||||
|
|
@ -1,13 +1,70 @@
|
|||
import pytest
|
||||
|
||||
from pre_commit_hooks.detect_aws_credentials import get_aws_credential_files_from_env
|
||||
from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_env
|
||||
from pre_commit_hooks.detect_aws_credentials import get_aws_secrets_from_file
|
||||
from pre_commit_hooks.detect_aws_credentials import main
|
||||
from testing.util import get_resource_path
|
||||
|
||||
|
||||
def test_get_aws_credentials_file_from_env(monkeypatch):
|
||||
"""Test that reading credential files names from environment variables works."""
|
||||
monkeypatch.delenv('AWS_CREDENTIAL_FILE', raising=False)
|
||||
monkeypatch.delenv('AWS_SHARED_CREDENTIALS_FILE', raising=False)
|
||||
monkeypatch.delenv('BOTO_CONFIG', raising=False)
|
||||
assert get_aws_credential_files_from_env() == set()
|
||||
monkeypatch.setenv('AWS_CREDENTIAL_FILE', '/foo')
|
||||
assert get_aws_credential_files_from_env() == {'/foo'}
|
||||
monkeypatch.setenv('AWS_SHARED_CREDENTIALS_FILE', '/bar')
|
||||
assert get_aws_credential_files_from_env() == {'/foo', '/bar'}
|
||||
monkeypatch.setenv('BOTO_CONFIG', '/baz')
|
||||
assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz'}
|
||||
monkeypatch.setenv('AWS_DUMMY_KEY', 'foobar')
|
||||
assert get_aws_credential_files_from_env() == {'/foo', '/bar', '/baz'}
|
||||
|
||||
|
||||
def test_get_aws_secrets_from_env(monkeypatch):
|
||||
"""Test that reading secrets from environment variables works."""
|
||||
monkeypatch.delenv('AWS_SECRET_ACCESS_KEY', raising=False)
|
||||
monkeypatch.delenv('AWS_SESSION_TOKEN', raising=False)
|
||||
assert get_aws_secrets_from_env() == set()
|
||||
monkeypatch.setenv('AWS_SECRET_ACCESS_KEY', 'foo')
|
||||
assert get_aws_secrets_from_env() == {'foo'}
|
||||
monkeypatch.setenv('AWS_SESSION_TOKEN', 'bar')
|
||||
assert get_aws_secrets_from_env() == {'foo', 'bar'}
|
||||
monkeypatch.setenv('AWS_SECURITY_TOKEN', 'baz')
|
||||
assert get_aws_secrets_from_env() == {'foo', 'bar', 'baz'}
|
||||
monkeypatch.setenv('AWS_DUMMY_KEY', 'baz')
|
||||
assert get_aws_secrets_from_env() == {'foo', 'bar', 'baz'}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(('filename', 'expected_keys'), (
|
||||
('aws_config_with_secret.ini', {
|
||||
'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb'}),
|
||||
('aws_config_with_session_token.ini', {'foo'}),
|
||||
('aws_config_with_secret_and_session_token.ini',
|
||||
{'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb', 'foo'}),
|
||||
('aws_config_with_multiple_sections.ini', {
|
||||
'7xebzorgm5143ouge9gvepxb2z70bsb2rtrh099e',
|
||||
'z2rpgs5uit782eapz5l1z0y2lurtsyyk6hcfozlb',
|
||||
'ixswosj8gz3wuik405jl9k3vdajsnxfhnpui38ez',
|
||||
'foo'}),
|
||||
('aws_config_without_secrets.ini', set()),
|
||||
('nonsense.txt', set()),
|
||||
('ok_json.json', set()),
|
||||
))
|
||||
def test_get_aws_secrets_from_file(filename, expected_keys):
|
||||
"""Test that reading secrets from files works."""
|
||||
keys = get_aws_secrets_from_file(get_resource_path(filename))
|
||||
assert keys == expected_keys
|
||||
|
||||
|
||||
# Input filename, expected return value
|
||||
TESTS = (
|
||||
('with_no_secrets.txt', 0),
|
||||
('with_secrets.txt', 1),
|
||||
('aws_config_with_secret.ini', 1),
|
||||
('aws_config_with_session_token.ini', 1),
|
||||
('aws_config_with_multiple_sections.ini', 1),
|
||||
('aws_config_without_secrets.ini', 0),
|
||||
('nonsense.txt', 0),
|
||||
('ok_json.json', 0),
|
||||
)
|
||||
|
|
@ -15,24 +72,30 @@ TESTS = (
|
|||
|
||||
@pytest.mark.parametrize(('filename', 'expected_retval'), TESTS)
|
||||
def test_detect_aws_credentials(filename, expected_retval):
|
||||
"""Test if getting configured AWS secrets from files to be checked in works."""
|
||||
|
||||
# with a valid credentials file
|
||||
ret = main((
|
||||
get_resource_path(filename),
|
||||
"--credentials-file=testing/resources/sample_aws_credentials",
|
||||
"--credentials-file=testing/resources/aws_config_with_multiple_sections.ini",
|
||||
))
|
||||
assert ret == expected_retval
|
||||
|
||||
|
||||
def test_non_existent_credentials(capsys):
|
||||
# with a non-existent credentials file
|
||||
def test_non_existent_credentials(capsys, monkeypatch):
|
||||
"""Test behavior with no configured AWS secrets."""
|
||||
monkeypatch.setattr(
|
||||
'pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_env',
|
||||
lambda: set())
|
||||
monkeypatch.setattr(
|
||||
'pre_commit_hooks.detect_aws_credentials.get_aws_secrets_from_file',
|
||||
lambda x: set())
|
||||
ret = main((
|
||||
get_resource_path('with_secrets.txt'),
|
||||
get_resource_path('aws_config_without_secrets.ini'),
|
||||
"--credentials-file=testing/resources/credentailsfilethatdoesntexist"
|
||||
))
|
||||
assert ret == 2
|
||||
out, _ = capsys.readouterr()
|
||||
assert out == (
|
||||
'No aws keys were configured at '
|
||||
'testing/resources/credentailsfilethatdoesntexist\n'
|
||||
'Configure them with --credentials-file\n'
|
||||
)
|
||||
assert out == ('No AWS keys were found in the configured credential files '
|
||||
'and environment variables.\nPlease ensure you have the '
|
||||
'correct setting for --credentials-file\n')
|
||||
|
|
|
|||
Loading…
Reference in New Issue