Add clamev efs #725

fredericfran-gds · 2022-08-15T21:20:57Z

We want to create a EFS volume to store the Clamav virus
database. Before, we attempted to create the EFS in AWS only
and mount it as NFS in the Clamav pod but this was unsuccessful
since the EFS has root ownership and the pod is running as non-root.

We could create the access point of the EFS and sets the same ownership
as the pod in AWS but we would have to use the volume handler option of
k8s PersistentVolume for k8s to mount the access point properly. This
option requires the AWS EFS CSI driver as a prerequisite.

The chosen solution is:

install AWS EFS CSI driver and associated permissions
The storage class requires a EFS id so we create the EFS and
pass it to the driver.

Fixes: 1. In the worker pod, we run clamd (daemon verion of clamav). This seems to lead to no killed clamav scans and also this is how it is currently run in EC2 rather than being run standalone. 2. Remove the freshclam (updates virus database of clamav) container from continuously running in the worker pod and set it to run as presync and then run a cronjob every hour for it. 3. Use clamav EFS to share virus databases across all clamav containers. (alphagov/govuk-infrastructure#725) Related PRs: 1. [asset-manager dockerfile](alphagov/asset-manager#938)

sengi

Cool - took me a while to realise why we need a shared filer here but I think it's the right compromise - thanks! 👍

sengi · 2022-08-16T10:06:11Z

terraform/deployments/govuk-publishing-infrastructure/clamav_efs.tf

+  clamav_efs_name = "clamav-efs-${local.cluster_name}"
+}
+
+resource "aws_efs_file_system" "clamav-efs" {


Consider putting a Name and/or Description tag on this to make it clear that it's for the ClamAV signatures (database). Or maybe just work it into the name somehow? (Could probably lose the efs from the name in most contexts, but signposting that it's for the sigs as opposed to, say, the files being scanned or anything like that, is quite helpful.)

clamav-sigs-db? clamav-db? 🤷

I will, thanks

sengi · 2022-08-16T10:07:17Z

terraform/deployments/govuk-publishing-infrastructure/security.tf

@@ -207,3 +207,23 @@ resource "aws_security_group_rule" "licensify_frontend_from_eks_workers" {
  security_group_id        = data.terraform_remote_state.infra_security_groups.outputs.sg_licensify-frontend_internal_lb_id
  source_security_group_id = data.terraform_remote_state.cluster_infrastructure.outputs.node_security_group_id
 }
+
+resource "aws_security_group_rule" "clamav_efs_to_any_any" {
+  description       = "Clamav sends requests to anywhere over any protocol"


Suggested change

description = "Clamav sends requests to anywhere over any protocol"

description = "Clam DB EFS sends requests to anywhere over any protocol"

sengi · 2022-08-16T10:08:11Z

terraform/deployments/govuk-publishing-infrastructure/security.tf

+
+resource "aws_security_group_rule" "clamav_efs_to_any_any" {
+  description       = "Clamav sends requests to anywhere over any protocol"
+  type              = "egress"
+  from_port         = 0
+  to_port           = 0
+  protocol          = -1
+  cidr_blocks       = ["0.0.0.0/0"]
+  security_group_id = aws_security_group.clamav-efs.id
+}


I strongly suspect we don't need this rule at all - doesn't the client always initiate the TCP connection to the NFS server?

Yeah, it is stateful, could remove that. Thanks.

Fixes: 1. In the worker pod, we run clamd (daemon verion of clamav). This seems to lead to no killed clamav scans and also this is how it is currently run in EC2 rather than being run standalone. 2. Remove the freshclam (updates virus database of clamav) container from continuously running in the worker pod and set it to run as presync and then run a cronjob every hour for it. 3. Use clamav EFS to share virus databases across all clamav containers. (alphagov/govuk-infrastructure#725) Related PRs: 1. [asset-manager dockerfile](alphagov/asset-manager#938)

We want to create a EFS volume to store the Clamav virus database. Before, we attempted to create the EFS in AWS only and mount it as NFS in the Clamav pod but this was unsuccessful since the EFS has root ownership and the pod is running as non-root. We could create the access point of the EFS and sets the same ownership as the pod in AWS but we would have to use the volume handler option of k8s PersistentVolume for k8s to mount the access point properly. This option requires the AWS EFS CSI driver as a prerequisite. The chosen solution is: 1. install AWS EFS CSI driver and associated permissions 2. The storage class requires a EFS id so we create the EFS and pass it to the driver.

sengi · 2022-08-16T15:36:22Z

terraform/deployments/cluster-infrastructure/aws_efs_csi_iam.tf

+  oidc_fully_qualified_subjects = ["system:serviceaccount:kube-system:${local.efs_csi_driver_controller_service_account_name}"]
+}
+
+resource "aws_iam_role_policy_attachment" "eks_nodes_efs" {


Looks like you have a service account for the controller, but you're attaching the policy to the nodes. You want to attach the policy to the IAM role that corresponds to the k8s serviceaccount, right?

If you attach the policy to the nodes, you're granting all pods the ability to manage EFS — which is presumably what you're trying to avoid by creating the serviceaccount and corresponding IRSA IAM role etc.?

I got the info from here, the same specs for csi driver and eks nodes: https://github.com/kubernetes-sigs/aws-efs-csi-driver#installation

Right so where it talks about "several methods to grant IAM permission", you want the first one (IAM Role for Service Account). It looks like you're kinda doing both here at the moment though.

terraform/deployments/cluster-infrastructure/clamav_db_efs.tf

Due to the issue with non-root asset-manager trying to use a root NFS volume created in AWS/terraform directly. We move to using the EFS CSI driver and PersistentVolumeClaim. See related PR: alphagov/govuk-infrastructure#725

Description: - #725 introduced the EBS CSI Driver which created EFS for ClamAV - Next alphagov/govuk-helm-charts#508 allowed ClamAV to talk to EFS over NFS exposing over clamav-db-govuk.integration.govuk-internal.digital - However this didn’t work so ClamAV was switched to use the EFS CSI driver in alphagov/govuk-helm-charts#514. But this removes the reference to clamav-db-govuk.integration.govuk-internal.digital - #790 removes the EFS CSI driver - Next alphagov/govuk-helm-charts#572 makes ClamAV share the EFS instance via the same NFS mount as asset manager. - Now there is a dangling reference to ClamAV EFS instance which can be safely removed as nothing references it anymore. - As part of alphagov/govuk-helm-charts#1883

fredericfran-gds mentioned this pull request Aug 16, 2022

Fix Clamav for Asset-manager alphagov/govuk-helm-charts#508

Merged

fredericfran-gds marked this pull request as ready for review August 16, 2022 09:31

sengi approved these changes Aug 16, 2022

View reviewed changes

fredericfran-gds force-pushed the clamav_efs branch from fe1b84d to 11a674f Compare August 16, 2022 15:27

sengi reviewed Aug 16, 2022

View reviewed changes

terraform/deployments/cluster-infrastructure/clamav_db_efs.tf Show resolved Hide resolved

fredericfran-gds merged commit d05433f into main Aug 16, 2022

fredericfran-gds deleted the clamav_efs branch August 16, 2022 15:57

fredericfran-gds mentioned this pull request Aug 16, 2022

Asset-Manager: switch to EFS CSI driver and PersistentVolumeClaim alphagov/govuk-helm-charts#514

Merged

nimalank7 mentioned this pull request Dec 5, 2024

Remove clamav_db_efs #1540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add clamev efs #725

Add clamev efs #725

fredericfran-gds commented Aug 15, 2022 •

edited

Loading

sengi left a comment

sengi Aug 16, 2022

sengi Aug 16, 2022

fredericfran-gds Aug 16, 2022

sengi Aug 16, 2022

fredericfran-gds Aug 16, 2022

sengi Aug 16, 2022

fredericfran-gds Aug 16, 2022

sengi Aug 16, 2022

fredericfran-gds Aug 16, 2022 •

edited

Loading

sengi Aug 16, 2022

	description = "Clamav sends requests to anywhere over any protocol"
	description = "Clam DB EFS sends requests to anywhere over any protocol"

Add clamev efs #725

Add clamev efs #725

Conversation

fredericfran-gds commented Aug 15, 2022 • edited Loading

sengi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredericfran-gds Aug 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredericfran-gds commented Aug 15, 2022 •

edited

Loading

fredericfran-gds Aug 16, 2022 •

edited

Loading