diff --git a/awscli/customizations/configure/__init__.py b/awscli/customizations/configure/__init__.py index 46aeed5f53db..9c37eb3a6953 100644 --- a/awscli/customizations/configure/__init__.py +++ b/awscli/customizations/configure/__init__.py @@ -10,8 +10,11 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. -import string -from awscli.compat import shlex +import os +import sys + +from awscli.compat import is_windows, shlex +from awscli.customizations.utils import uni_print NOT_SET = '' PREDEFINED_SECTION_NAMES = ('preview', 'plugins') @@ -47,3 +50,35 @@ def profile_to_section(profile_name): if any(c in _WHITESPACE for c in profile_name): profile_name = shlex.quote(profile_name) return 'profile %s' % profile_name + + +PERMISSIONS_WARNING_TEMPLATE = ( + "\naws: [WARNING]: The file '{path}' is accessible by other users. " + "Consider running 'chmod 600 {path}' to restrict access to only your " + "user.\n" +) + + +def warn_if_permissive(file_path, err_stream=None): + if is_windows: + return + + if not os.path.isfile(file_path): + return + + if err_stream is None: + err_stream = sys.stderr + + try: + file_mode = os.stat(file_path).st_mode + if is_overly_permissive(file_mode, 0o700): + uni_print( + PERMISSIONS_WARNING_TEMPLATE.format(path=file_path), + out_file=err_stream, + ) + except OSError: + return + + +def is_overly_permissive(file_mode, allowed_bits=0o700): + return bool((file_mode & 0o777) & ~allowed_bits) diff --git a/awscli/customizations/configure/configure.py b/awscli/customizations/configure/configure.py index 1f23021676e4..94932e066a62 100644 --- a/awscli/customizations/configure/configure.py +++ b/awscli/customizations/configure/configure.py @@ -138,4 +138,6 @@ def _write_out_creds_file_values(self, new_values, profile_name): self._session.get_config_variable('credentials_file')) self._config_writer.update_config( credential_file_values, - shared_credentials_filename) + shared_credentials_filename, + check_permissions=True, + ) diff --git a/awscli/customizations/configure/set.py b/awscli/customizations/configure/set.py index 81a0d3d66618..4d8d36f5bec5 100644 --- a/awscli/customizations/configure/set.py +++ b/awscli/customizations/configure/set.py @@ -95,14 +95,20 @@ def _run_main(self, args, parsed_globals): # of something in the [plugin] section. profile, varname = parts config_filename = self._get_config_file('config_file') + check_permissions = False if varname in self._WRITE_TO_CREDS_FILE: # When writing to the creds file, the section is just the profile section = profile config_filename = self._get_config_file('credentials_file') + check_permissions = True elif profile in PREDEFINED_SECTION_NAMES or profile == 'default': section = profile else: section = profile_to_section(profile) updated_config = {'__section__': section, varname: value} - self._config_writer.update_config(updated_config, config_filename) + self._config_writer.update_config( + updated_config, + config_filename, + check_permissions=check_permissions, + ) return 0 diff --git a/awscli/customizations/configure/writer.py b/awscli/customizations/configure/writer.py index 06808e0565a4..83825a53dab6 100644 --- a/awscli/customizations/configure/writer.py +++ b/awscli/customizations/configure/writer.py @@ -13,7 +13,7 @@ import os import re -from . import SectionNotFoundError +from . import SectionNotFoundError, warn_if_permissive class ConfigFileWriter(object): @@ -37,7 +37,9 @@ def _validate_no_newlines_or_carriage_returns( ) raise ValueError(err_msg) - def update_config(self, new_values, config_filename): + def update_config( + self, new_values, config_filename, check_permissions=False + ): """Update config file with new values. This method will update a section in a config file with @@ -63,6 +65,10 @@ def update_config(self, new_values, config_filename): :param config_filename: The config filename where values will be written. + :type check_permissions: bool + :param check_permissions: If True, warn if the file has + permissions more permissive than 0o600. + """ section_name = new_values.pop('__section__', 'default') self._validate_no_newlines_or_carriage_returns( @@ -111,6 +117,8 @@ def update_config(self, new_values, config_filename): f.write(''.join(contents)) except SectionNotFoundError: self._write_new_section(section_name, new_values, config_filename) + if check_permissions: + warn_if_permissive(config_filename) def _create_file(self, config_filename): # Create the file as well as the parent dir if needed. diff --git a/tests/unit/customizations/configure/test_configure.py b/tests/unit/customizations/configure/test_configure.py index fc5221043d07..3eacad507deb 100644 --- a/tests/unit/customizations/configure/test_configure.py +++ b/tests/unit/customizations/configure/test_configure.py @@ -37,7 +37,8 @@ def assert_credentials_file_updated_with(self, new_values): credentials_file_call = called_args[0] expected_creds_file = os.path.expanduser('~/fake_credentials_filename') self.assertEqual(credentials_file_call, - mock.call(new_values, expected_creds_file)) + mock.call(new_values, expected_creds_file, + check_permissions=True)) def test_configure_command_sends_values_to_writer(self): self.configure(args=[], parsed_globals=self.global_args) @@ -129,6 +130,22 @@ def test_session_says_profile_does_not_exist(self): 'region': 'new_value', 'output': 'new_value'}, 'myconfigfile') + def test_check_permissions_when_credential_values_provided(self): + self.configure(args=[], parsed_globals=self.global_args) + expected_creds_file = os.path.expanduser('~/fake_credentials_filename') + self.writer.update_config.assert_any_call( + mock.ANY, expected_creds_file, check_permissions=True + ) + + def test_no_check_permissions_when_no_credential_values_changed(self): + user_presses_enter = None + precanned = PrecannedPrompter(value=user_presses_enter) + self.configure = configure.ConfigureCommand( + self.session, prompter=precanned, config_writer=self.writer + ) + self.configure(args=[], parsed_globals=self.global_args) + self.writer.update_config.assert_not_called() + class TestInteractivePrompter(unittest.TestCase): diff --git a/tests/unit/customizations/configure/test_permissions.py b/tests/unit/customizations/configure/test_permissions.py new file mode 100644 index 000000000000..94eeea52a63a --- /dev/null +++ b/tests/unit/customizations/configure/test_permissions.py @@ -0,0 +1,81 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. +import os +from io import StringIO +from unittest.mock import patch + +import pytest + +from awscli.customizations.configure import ( + is_overly_permissive, + warn_if_permissive, +) + + +@pytest.mark.parametrize("mode", [0o700, 0o600, 0o400, 0o200, 0o000]) +def test_acceptable_modes_are_not_overly_permissive(mode): + assert is_overly_permissive(mode) is False + + +@pytest.mark.parametrize("mode", [0o644, 0o666, 0o777, 0o610, 0o601]) +def test_overly_permissive_modes_are_detected(mode): + assert is_overly_permissive(mode) is True + + +def _create_creds_file(tmp_path, mode): + path = tmp_path / "credentials" + path.write_text("[default]\naws_access_key_id=testing\n") + os.chmod(path, mode) + return str(path) + + +@patch("awscli.customizations.configure.is_windows", False) +def test_prints_warning_for_permissive_file(tmp_path): + path = _create_creds_file(tmp_path, 0o644) + err = StringIO() + warn_if_permissive(path, err_stream=err) + output = err.getvalue() + assert "aws: [WARNING]" in output + assert path in output + assert "accessible by other users" in output + + +@patch("awscli.customizations.configure.is_windows", False) +def test_no_warning_for_0o600_file(tmp_path): + path = _create_creds_file(tmp_path, 0o600) + err = StringIO() + warn_if_permissive(path, err_stream=err) + assert err.getvalue() == "" + + +@patch("awscli.customizations.configure.is_windows", False) +def test_no_warning_for_0o700_file(tmp_path): + path = _create_creds_file(tmp_path, 0o700) + err = StringIO() + warn_if_permissive(path, err_stream=err) + assert err.getvalue() == "" + + +@patch("awscli.customizations.configure.is_windows", False) +def test_skips_when_file_does_not_exist(tmp_path): + err = StringIO() + warn_if_permissive(str(tmp_path / "nonexistent"), err_stream=err) + assert err.getvalue() == "" + + +@patch("awscli.customizations.configure.is_windows", True) +def test_skips_on_windows(tmp_path): + path = _create_creds_file(tmp_path, 0o777) + err = StringIO() + warn_if_permissive(path, err_stream=err) + assert err.getvalue() == "" diff --git a/tests/unit/customizations/configure/test_set.py b/tests/unit/customizations/configure/test_set.py index 2b8f5c0d7dfe..3cab83b50d48 100644 --- a/tests/unit/customizations/configure/test_set.py +++ b/tests/unit/customizations/configure/test_set.py @@ -31,14 +31,16 @@ def test_configure_set_command(self): self.session, self.config_writer) set_command(args=['region', 'us-west-2'], parsed_globals=None) self.config_writer.update_config.assert_called_with( - {'__section__': 'default', 'region': 'us-west-2'}, 'myconfigfile') + {'__section__': 'default', 'region': 'us-west-2'}, 'myconfigfile', + check_permissions=False) def test_configure_set_command_dotted(self): set_command = ConfigureSetCommand( self.session, self.config_writer) set_command(args=['preview.emr', 'true'], parsed_globals=None) self.config_writer.update_config.assert_called_with( - {'__section__': 'preview', 'emr': 'true'}, 'myconfigfile') + {'__section__': 'preview', 'emr': 'true'}, 'myconfigfile', + check_permissions=False) def test_configure_set_command_dotted_with_default_profile(self): self.session.variables['profile'] = 'default' @@ -48,7 +50,8 @@ def test_configure_set_command_dotted_with_default_profile(self): args=['emr.instance_profile', 'my_ip_emr'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'default', - 'emr': {'instance_profile': 'my_ip_emr'}}, 'myconfigfile') + 'emr': {'instance_profile': 'my_ip_emr'}}, 'myconfigfile', + check_permissions=False) def test_configure_set_handles_predefined_plugins_section(self): self.session.variables['profile'] = 'default' @@ -58,7 +61,8 @@ def test_configure_set_handles_predefined_plugins_section(self): args=['plugins.foo', 'mypackage'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'plugins', - 'foo': 'mypackage'}, 'myconfigfile') + 'foo': 'mypackage'}, 'myconfigfile', + check_permissions=False) def test_configure_set_command_dotted_with_profile(self): self.session.profile = 'emr-dev' @@ -68,7 +72,8 @@ def test_configure_set_command_dotted_with_profile(self): args=['emr.instance_profile', 'my_ip_emr'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'profile emr-dev', 'emr': - {'instance_profile': 'my_ip_emr'}}, 'myconfigfile') + {'instance_profile': 'my_ip_emr'}}, 'myconfigfile', + check_permissions=False) def test_configure_set_with_profile(self): self.session.profile = 'testing' @@ -77,7 +82,7 @@ def test_configure_set_with_profile(self): set_command(args=['region', 'us-west-2'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'profile testing', 'region': 'us-west-2'}, - 'myconfigfile') + 'myconfigfile', check_permissions=False) def test_configure_set_triple_dotted(self): # aws configure set default.s3.signature_version s3v4 @@ -87,7 +92,7 @@ def test_configure_set_triple_dotted(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'default', 's3': {'signature_version': 's3v4'}}, - 'myconfigfile') + 'myconfigfile', check_permissions=False) def test_configure_set_with_profile_nested(self): # aws configure set default.s3.signature_version s3v4 @@ -97,7 +102,8 @@ def test_configure_set_with_profile_nested(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'profile foo', - 's3': {'signature_version': 's3v4'}}, 'myconfigfile') + 's3': {'signature_version': 's3v4'}}, 'myconfigfile', + check_permissions=False) def test_access_key_written_to_shared_credentials_file(self): set_command = ConfigureSetCommand( @@ -106,7 +112,8 @@ def test_access_key_written_to_shared_credentials_file(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'default', - 'aws_access_key_id': 'foo'}, self.fake_credentials_filename) + 'aws_access_key_id': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_secret_key_written_to_shared_credentials_file(self): set_command = ConfigureSetCommand( @@ -115,7 +122,8 @@ def test_secret_key_written_to_shared_credentials_file(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'default', - 'aws_secret_access_key': 'foo'}, self.fake_credentials_filename) + 'aws_secret_access_key': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_session_token_written_to_shared_credentials_file(self): set_command = ConfigureSetCommand( @@ -124,7 +132,8 @@ def test_session_token_written_to_shared_credentials_file(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'default', - 'aws_session_token': 'foo'}, self.fake_credentials_filename) + 'aws_session_token': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_security_token_written_to_shared_credentials_file(self): set_command = ConfigureSetCommand( @@ -142,7 +151,8 @@ def test_access_key_written_to_shared_credentials_file_profile(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'foo', - 'aws_access_key_id': 'bar'}, self.fake_credentials_filename) + 'aws_access_key_id': 'bar'}, self.fake_credentials_filename, + check_permissions=True) def test_credential_set_profile_with_space(self): self.session.profile = 'some profile' @@ -150,7 +160,8 @@ def test_credential_set_profile_with_space(self): set_command(args=['aws_session_token', 'foo'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'some profile', - 'aws_session_token': 'foo'}, self.fake_credentials_filename) + 'aws_session_token': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_credential_set_profile_with_space_dotted(self): set_command = ConfigureSetCommand(self.session, self.config_writer) @@ -158,7 +169,8 @@ def test_credential_set_profile_with_space_dotted(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'some profile', - 'aws_session_token': 'foo'}, self.fake_credentials_filename) + 'aws_session_token': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_configure_set_with_profile_with_space(self): self.session.profile = 'some profile' @@ -166,7 +178,7 @@ def test_configure_set_with_profile_with_space(self): set_command(args=['region', 'us-west-2'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': "profile 'some profile'", 'region': 'us-west-2'}, - 'myconfigfile') + 'myconfigfile', check_permissions=False) def test_configure_set_with_profile_with_space_dotted(self): set_command = ConfigureSetCommand(self.session, self.config_writer) @@ -174,7 +186,7 @@ def test_configure_set_with_profile_with_space_dotted(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': "profile 'some profile'", 'region': 'us-west-2'}, - 'myconfigfile') + 'myconfigfile', check_permissions=False) def test_credential_set_profile_with_tab(self): self.session.profile = 'some\tprofile' @@ -182,7 +194,8 @@ def test_credential_set_profile_with_tab(self): set_command(args=['aws_session_token', 'foo'], parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': 'some\tprofile', - 'aws_session_token': 'foo'}, self.fake_credentials_filename) + 'aws_session_token': 'foo'}, self.fake_credentials_filename, + check_permissions=True) def test_configure_set_with_profile_with_tab_dotted(self): set_command = ConfigureSetCommand(self.session, self.config_writer) @@ -190,4 +203,4 @@ def test_configure_set_with_profile_with_tab_dotted(self): parsed_globals=None) self.config_writer.update_config.assert_called_with( {'__section__': "profile 'some\tprofile'", 'region': 'us-west-2'}, - 'myconfigfile') + 'myconfigfile', check_permissions=False)