Tighten file permissions for virtual MFA bootstrap output (#10194)

Restrict MFA seed files to 0600 (owner-only) to prevent other local users from reading the bootstrap data.

Author: manthan29jamdagni

.changes/next-release/bugfix-iam-84710.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "``iam``",
+  "description": "Tighten file permissions for virtual MFA bootstrap output"
+}

awscli/customizations/iamvirtmfa.py

@@ -22,43 +22,55 @@
 to the specified file.  It will also remove the two bootstrap data
 fields from the response.
 """
-import base64
 
-from awscli.customizations.arguments import StatefulArgument
-from awscli.customizations.arguments import resolve_given_outfile_path
-from awscli.customizations.arguments import is_parsed_result_successful
+import base64
+import os
 
+from awscli.compat import compat_open
+from awscli.customizations.arguments import (
+    StatefulArgument,
+    is_parsed_result_successful,
+    resolve_given_outfile_path,
+)
 
 CHOICES = ('QRCodePNG', 'Base32StringSeed')
-OUTPUT_HELP = ('The output path and file name where the bootstrap '
-               'information will be stored.')
-BOOTSTRAP_HELP = ('Method to use to seed the virtual MFA.  '
-                  'Valid values are: %s | %s' % CHOICES)
+OUTPUT_HELP = (
+    'The output path and file name where the bootstrap '
+    'information will be stored.'
+)
+BOOTSTRAP_HELP = (
+    'Method to use to seed the virtual MFA.  '
+    'Valid values are: %s | %s' % CHOICES
+)
 
 
 class FileArgument(StatefulArgument):
-
     def add_to_params(self, parameters, value):
         # Validate the file here so we can raise an error prior
         # calling the service.
         value = resolve_given_outfile_path(value)
         super(FileArgument, self).add_to_params(parameters, value)
 
 
-class IAMVMFAWrapper(object):
-
+class IAMVMFAWrapper:
     def __init__(self, event_handler):
         self._event_handler = event_handler
         self._outfile = FileArgument(
-            'outfile', help_text=OUTPUT_HELP, required=True)
+            'outfile', help_text=OUTPUT_HELP, required=True
+        )
         self._method = StatefulArgument(
-            'bootstrap-method', help_text=BOOTSTRAP_HELP,
-            choices=CHOICES, required=True)
+            'bootstrap-method',
+            help_text=BOOTSTRAP_HELP,
+            choices=CHOICES,
+            required=True,
+        )
         self._event_handler.register(
             'building-argument-table.iam.create-virtual-mfa-device',
-            self._add_options)
+            self._add_options,
+        )
         self._event_handler.register(
-            'after-call.iam.CreateVirtualMFADevice', self._save_file)
+            'after-call.iam.CreateVirtualMFADevice', self._save_file
+        )
 
     def _add_options(self, argument_table, **kwargs):
         argument_table['outfile'] = self._outfile
@@ -71,7 +83,9 @@ def _save_file(self, parsed, **kwargs):
         outfile = self._outfile.value
         if method in parsed['VirtualMFADevice']:
             body = parsed['VirtualMFADevice'][method]
-            with open(outfile, 'wb') as fp:
+            with compat_open(outfile, 'wb', access_permissions=0o600) as fp:
+                if hasattr(os, 'fchmod'):
+                    os.fchmod(fp.fileno(), 0o600)
                 fp.write(base64.b64decode(body))
             for choice in CHOICES:
                 if choice in parsed['VirtualMFADevice']:

tests/functional/iam/test_create_virtual_mfa_device.py

@@ -11,25 +11,26 @@
 # 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.
-from awscli.testutils import BaseAWSCommandParamsTest
 import os
 
+from awscli.testutils import BaseAWSCommandParamsTest, skip_if_windows
 
-class TestCreateVirtualMFADevice(BaseAWSCommandParamsTest):
 
+class TestCreateVirtualMFADevice(BaseAWSCommandParamsTest):
     prefix = 'iam create-virtual-mfa-device'
 
     def setUp(self):
         super(TestCreateVirtualMFADevice, self).setUp()
         self.parsed_response = {
             'ResponseMetadata': {
                 'HTTPStatusCode': 200,
-                'RequestId': 'requset-id'
+                'RequestId': 'requset-id',
             },
             "VirtualMFADevice": {
                 "Base32StringSeed": (
                     "VFpYTVc2V1lIUFlFRFczSVhLUlpRUTJRVFdUSFRNRDNTQ0c3"
-                    "TkZDUVdQWDVETlNWM0IyUENaQVpWTEpQTlBOTA=="),
+                    "TkZDUVdQWDVETlNWM0IyUENaQVpWTEpQTlBOTA=="
+                ),
                 "SerialNumber": "arn:aws:iam::419278470775:mfa/fiebaz",
                 "QRCodePNG": (
                     "iVBORw0KGgoAAAANSUhEUgAAAPoAAAD6CAIAAAAHjs1qAAAFi"
@@ -74,12 +75,13 @@ def setUp(self):
                     "R3EVwF8FdBHcR3EVwF8Fd5F/+AgASajf850wfAAAAAElFTkSu"
                     "QmCC"
                 ),
-            }
+            },
         }
 
     def getpath(self, filename):
-        return os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                            filename)
+        return os.path.join(
+            os.path.abspath(os.path.dirname(__file__)), filename
+        )
 
     def remove_file_if_exists(self, filename):
         if os.path.isfile(filename):
@@ -91,7 +93,8 @@ def test_base32(self):
         cmdline = self.prefix
         cmdline += ' --virtual-mfa-device-name fiebaz'
         cmdline += (
-            ' --outfile %s --bootstrap-method Base32StringSeed' % outfile)
+            ' --outfile %s --bootstrap-method Base32StringSeed' % outfile
+        )
         result = {"VirtualMFADeviceName": 'fiebaz'}
         self.assert_params_for_cmd(cmdline, result)
         self.assertTrue(os.path.exists(outfile))
@@ -145,7 +148,8 @@ def test_bad_response(self):
             },
             'ResponseMetadata': {
                 'HTTPStatusCode': 409,
-                'RequestId': 'requset-id'}
+                'RequestId': 'requset-id',
+            },
         }
         self.http_response.status_code = 409
         cmdline = self.prefix
@@ -155,4 +159,34 @@ def test_bad_response(self):
         self.assert_params_for_cmd(
             cmdline,
             stderr_contains=self.parsed_response['Error']['Message'],
-            expected_rc=255)
+            expected_rc=255,
+        )
+
+    @skip_if_windows("Permissions test not valid on Windows.")
+    def test_output_file_permissions(self):
+        outfile = self.getpath('fiebaz_perms.b32')
+        self.addCleanup(self.remove_file_if_exists, outfile)
+        cmdline = self.prefix
+        cmdline += ' --virtual-mfa-device-name fiebaz'
+        cmdline += (
+            ' --outfile %s --bootstrap-method Base32StringSeed' % outfile
+        )
+        result = {"VirtualMFADeviceName": 'fiebaz'}
+        self.assert_params_for_cmd(cmdline, result)
+        self.assertEqual(os.stat(outfile).st_mode & 0xFFF, 0o600)
+
+    @skip_if_windows("Permissions test not valid on Windows.")
+    def test_output_file_permissions_existing_file(self):
+        outfile = self.getpath('fiebaz_perms_existing.b32')
+        self.addCleanup(self.remove_file_if_exists, outfile)
+        with open(outfile, 'wb') as f:
+            f.write(b'existing')
+        os.chmod(outfile, 0o644)
+        cmdline = self.prefix
+        cmdline += ' --virtual-mfa-device-name fiebaz'
+        cmdline += (
+            ' --outfile %s --bootstrap-method Base32StringSeed' % outfile
+        )
+        result = {"VirtualMFADeviceName": 'fiebaz'}
+        self.assert_params_for_cmd(cmdline, result)
+        self.assertEqual(os.stat(outfile).st_mode & 0xFFF, 0o600)