Resolve security group bug (#176)

brandold · web-flow · commit 9e421aa00e02 · 2022-09-30T10:10:46.000-07:00
* nfs access function integrated and test

* web configuration to check and warn of missing valid security groups
diff --git a/deployment/simple-file-manager-for-amazon-efs.yaml b/deployment/simple-file-manager-for-amazon-efs.yaml
@@ -73,6 +73,7 @@ Resources:
                   - "ec2:DescribeSecurityGroups"
                   - "ec2:DescribeSubnets"
                   - "ec2:DescribeVpcs"
+                  - "ec2:DescribeSecurityGroupRules"
                 Resource: "*"
               - Effect: Allow
                 Action:
diff --git a/source/api/app.py b/source/api/app.py
@@ -13,6 +13,7 @@
 """API for Simple File Manager for Amazon EFS"""
 import os
 import logging
+import ipaddress
 import json
 import botocore
 from botocore.config import Config
@@ -41,6 +42,7 @@
 EFS = boto3.client('efs', config=CONFIG)
 SERVERLESS = boto3.client('lambda', config=CONFIG)
 CFN = boto3.client('cloudformation', config=CONFIG)
+EC2 = boto3.client('ec2', config=CONFIG)
 
 
 # Helper functions
@@ -246,6 +248,52 @@ def format_operation_response(result, error_message):
     return response
 
 
+def test_nfs_access(group, mount_target_ip):
+    rules = EC2.describe_security_group_rules(
+        Filters=[
+                {
+                    'Name': 'group-id',
+                    'Values': [group]
+                }
+            ]
+        )
+
+    contains_valid_rule = False
+
+    for rule in rules['SecurityGroupRules']:
+        # check if rule is inbound
+        if rule['IsEgress'] is False:
+            if 'ReferencedGroupInfo' in rule:
+                # references a sg in the rule
+                ref_group_id = rule['ReferencedGroupInfo']['GroupId']
+                if ref_group_id == rule['GroupId']:
+                    if rule['IpProtocol'] == '-1' or rule['IpProtocol'] == 'tcp':
+                        if rule['FromPort'] and rule['ToPort'] == 2049 or rule['FromPort'] and rule['ToPort'] == -1:
+                            contains_valid_rule = True
+                        elif rule['FromPort'] <= 2049 <= rule['ToPort']:
+                            contains_valid_rule = True
+                        else:
+                            pass
+            elif 'CidrIpv4' in rule:
+                # references an ipv4 subnet
+                subnet = rule['CidrIpv4']
+                if ipaddress.ip_address(mount_target_ip) in ipaddress.ip_network(subnet):
+                    if rule['IpProtocol'] == '-1' or rule['IpProtocol'] == 'tcp':
+                        if rule['FromPort'] and rule['ToPort'] == 2049 or rule['FromPort'] and rule['ToPort'] == -1:
+                            contains_valid_rule = True
+                        elif rule['FromPort'] <= 2049 <= rule['ToPort']:
+                            contains_valid_rule = True
+                        else:
+                            pass
+            else:
+                # bad request
+                pass
+        else:
+            # outbound rule
+            pass
+
+    return contains_valid_rule
+
 # Routes
 
 @app.route('/filesystems', methods=["GET"], cors=True, authorizer=AUTHORIZER)
@@ -324,13 +372,13 @@ def describe_filesystem(filesystem_id):
     authorizer=AUTHORIZER)
 def get_netinfo_for_filesystem(filesystem_id):
     """
-    Retrieves network info for a specified filesystem
+    Retrieves mount target network info for a specified filesystem
 
     :param filesystem_id: The filesystem to get net info for
-    :returns: Filesystem network info
+    :returns: Filesystem mount target network info
     :raises ChaliceViewError
     """
-    netinfo = []
+    mount_target_info = []
     try:
         response = EFS.describe_mount_targets(
             FileSystemId=filesystem_id
@@ -343,6 +391,8 @@ def get_netinfo_for_filesystem(filesystem_id):
         app.log.debug(mount_targets)
         for target in mount_targets:
             mount_target_id = target['MountTargetId']
+            mount_target_ip = target['IpAddress']
+
             try:
                 response = EFS.describe_mount_target_security_groups(
                     MountTargetId=mount_target_id
@@ -352,11 +402,25 @@ def get_netinfo_for_filesystem(filesystem_id):
                 raise ChaliceViewError
             else:
                 security_groups = response['SecurityGroups']
-                vpc_item = {'{id}'.format(id=mount_target_id): {'security_groups': \
-                    security_groups, 'subnet_id': target['SubnetId']}}
-                netinfo.append(vpc_item)
-
-    return netinfo
+                
+                # test security groups to see if the mount target can be used
+                valid_security_groups = []
+                for group in security_groups:
+                    is_valid_group = test_nfs_access(group, mount_target_ip)
+                    if is_valid_group:
+                        valid_security_groups.append(group)
+                    else:
+                        pass
+                
+                mount_target_item = {'{id}'.format(id=mount_target_id): {'security_groups': \
+                    valid_security_groups, 'subnet_id': target['SubnetId']}}
+                
+                mount_target_info.append(mount_target_item)
+    
+    if len(mount_target_info) == 0:
+        raise BadRequestError('No mount target available with required network configuration. See the deployment guide for configuration details.')
+    else:
+        return mount_target_info
 
 
 @app.route('/filesystems/{filesystem_id}/lambda', methods=['POST'], cors=True, \
diff --git a/source/web/src/routes/Configure.vue b/source/web/src/routes/Configure.vue
@@ -35,10 +35,15 @@
                         <b-form-invalid-feedback :state="valid">
                             UID, GID, and Path must not be empty. The path must begin with a forward slash: /
                         </b-form-invalid-feedback>
-                        <b-button :disabled='!valid' type="submit">
-                            Submit
-                        <b-icon icon="Hammer" aria-hidden="true"></b-icon>
-                </b-button>
+                        <div v-if="!mountTargetNetinfo.securityGroups.length">
+                            <b-alert show variant="danger">No mount targets available for the filesystem with security group(s) configured for NFS access. See the deployment guide for further details.</b-alert>
+                        </div>
+                        <div v-else>
+                            <b-button :disabled='!valid' type="submit">
+                                    Submit
+                                <b-icon icon="Hammer" aria-hidden="true"></b-icon>
+                            </b-button>
+                        </div>
                 </b-form>
             </b-col>
             <b-col>
@@ -59,20 +64,24 @@ export default {
         processing: false,
         uid: '1000',
         gid: '1000',
-        path: '/efs'
+        path: '/efs',
+        mountTargetNetinfo: null
     }
   },
   computed: {
       valid() {
           return this.uid != "" && this.gid != "" && this.path != "" && this.path.charAt(0) == '/'
       }
   },
+  mounted: function () {
+      this.getFilesystemNetinfo()
+  },
   methods: {
       async onSubmit(evt) {
         evt.preventDefault();
 
         if (this.valid) {
-            let mountTargetNetinfo = await this.getFilesystemNetinfo()
+            let mountTargetNetinfo = this.mountTargetNetinfo
         
             mountTargetNetinfo['uid'] = this.uid
             mountTargetNetinfo['gid'] = this.gid
@@ -115,7 +124,7 @@ export default {
           try {
               let response = await API.get('fileManagerApi', '/api/filesystems/' + this.$route.params.id + '/netinfo')
               let formattedNetinfo = this.formatNetinfo(response)
-              return formattedNetinfo
+              this.mountTargetNetinfo = formattedNetinfo
           }
           catch (error) {
               alert('Unable to retrieve filesystem netinfo, check api logs')
diff --git a/test/unit/api/conftest.py b/test/unit/api/conftest.py
@@ -32,5 +32,12 @@ def cfn_client_stub(mock_env_variables):
 def lambda_client_stub(mock_env_variables):
     from app import SERVERLESS
     with Stubber(SERVERLESS) as stubber:
+        yield stubber
+        stubber.assert_no_pending_responses()
+
+@pytest.fixture
+def ec2_client_stub(mock_env_variables):
+    from app import EC2
+    with Stubber(EC2) as stubber:
         yield stubber
         stubber.assert_no_pending_responses()
diff --git a/test/unit/api/service_responses.py b/test/unit/api/service_responses.py
@@ -6,6 +6,73 @@
 
 test_stack_name = f'testStackPrefix-ManagedResources-{test_filesystem_id}'
 
+ec2_describe_security_group_rules_response = {
+	'SecurityGroupRules': [{
+		'SecurityGroupRuleId': 'sgr-0123abc',
+		'GroupId': 'sg-4567abcd',
+		'GroupOwnerId': '',
+		'IsEgress': False,
+		'IpProtocol': '-1',
+		'FromPort': -1,
+		'ToPort': -1,
+		'ReferencedGroupInfo': {
+			'GroupId': 'sg-4567abcd',
+			'UserId': ''
+		},
+		'Tags': []
+	}, {
+		'SecurityGroupRuleId': 'sgr-0123abc',
+		'GroupId': 'sg-4567abcd',
+		'GroupOwnerId': '',
+		'IsEgress': False,
+		'IpProtocol': 'tcp',
+		'FromPort': 2049,
+		'ToPort': 2049,
+		'ReferencedGroupInfo': {
+			'GroupId': 'sg-4567abcd',
+			'UserId': ''
+		},
+		'Tags': []
+	}, {
+		'SecurityGroupRuleId': 'sgr-0123abc',
+		'GroupId': 'sg-4567abcd',
+		'GroupOwnerId': '',
+		'IsEgress': True,
+		'IpProtocol': '-1',
+		'FromPort': -1,
+		'ToPort': -1,
+		'CidrIpv4': '0.0.0.0/0',
+		'Tags': []
+	},
+    {
+		'SecurityGroupRuleId': 'sgr-0123abc',
+		'GroupId': 'sg-4567abcd',
+		'GroupOwnerId': '',
+		'IsEgress': False,
+		'IpProtocol': '-1',
+		'FromPort': -1,
+		'ToPort': -1,
+		'CidrIpv4': '0.0.0.0/0',
+		'Tags': []
+	}
+    ],
+	'ResponseMetadata': {
+		'RequestId': '',
+		'HTTPStatusCode': 200,
+		'HTTPHeaders': {
+			'x-amzn-requestid': '',
+			'cache-control': 'no-cache, no-store',
+			'strict-transport-security': 'max-age=31536000; includeSubDomains',
+			'vary': 'accept-encoding',
+			'content-type': 'text/xml;charset=UTF-8',
+			'transfer-encoding': 'chunked',
+			'date': 'Wed, 21 Sep 2022 20:02:50 GMT',
+			'server': 'AmazonEC2'
+		},
+		'RetryAttempts': 0
+	}
+}
+
 efs_describe_file_systems_no_marker_response = {
     'FileSystems': [
         {
@@ -172,4 +239,5 @@
 
 EFS = {'describe_file_systems_no_marker': efs_describe_file_systems_no_marker_response, 'describe_mount_targets': efs_describe_mount_targets_response, 'describe_mount_target_security_groups': efs_describe_mount_target_security_groups_response}
 CFN = {'describe_stacks': cfn_describe_stacks_response, 'create_stack': cfn_create_stack_response}
-LAMBDA = {'upload': lambda_invoke_upload_response, 'delete': lambda_invoke_delete_response, 'list': lambda_invoke_list_response, 'make_dir': lambda_invoke_make_dir_response, 'download': lambda_invoke_download_response}
+LAMBDA = {'upload': lambda_invoke_upload_response, 'delete': lambda_invoke_delete_response, 'list': lambda_invoke_list_response, 'make_dir': lambda_invoke_make_dir_response, 'download': lambda_invoke_download_response}
+EC2 = {'describe_sec_rules': ec2_describe_security_group_rules_response}
diff --git a/test/unit/api/test_app.py b/test/unit/api/test_app.py
@@ -1,7 +1,7 @@
 import json
 import botocore.stub
 
-from service_responses import EFS, CFN, LAMBDA
+from service_responses import EFS, CFN, LAMBDA, EC2
 
 test_filesystem_id = 'fs-01234567'
 
@@ -46,7 +46,7 @@ def test_list_filesystems(test_client, efs_client_stub, cfn_client_stub):
     print('PASS')
 
 
-def test_get_netinfo_for_filesystem(test_client, efs_client_stub):
+def test_get_netinfo_for_filesystem(test_client, efs_client_stub, ec2_client_stub):
     print(f'GET /filesystems/{test_filesystem_id}/netinfo')
 
     efs_client_stub.add_response(
@@ -61,6 +61,12 @@ def test_get_netinfo_for_filesystem(test_client, efs_client_stub):
         service_response=EFS['describe_mount_target_security_groups']
     )
 
+    ec2_client_stub.add_response(
+        'describe_security_group_rules',
+        expected_params={'Filters': [{'Name': 'group-id', 'Values': ['sg-4567abcd']}]},
+        service_response=EC2['describe_sec_rules']
+    )
+
     response = test_client.http.get(f'/filesystems/{test_filesystem_id}/netinfo')
 
     formatted_response = json.loads(response.body)
