fixed added deny iam policy when user opt disable log for a function

[as19ish]

There is no way to disable the CloudWatch logs for a lambda function. One workaround is you can add the policy to your role to disable the CloudWatch logs.

I have added deny policy for that specific lamda so as per now by default all function have logs:PutLogEvents permission when there is disableLogs opt to true we are adding deny policy statement for that function

Closes: #8554

[codecov-io]

Codecov Report

Merging #8561 (ff2a89f) into master (3808471) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8561      +/-   ##
==========================================
+ Coverage   87.27%   87.28%   +0.01%     
==========================================
  Files         256      256              
  Lines        9516     9524       +8     
==========================================
+ Hits         8305     8313       +8     
  Misses       1211     1211              

Impacted Files	Coverage Δ
lib/plugins/aws/package/lib/mergeIamTemplates.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3808471...ff2a89f. Read the comment docs.

[medikoo]

@as19ish Thank you! Please see my comment

[medikoo]

Wouldn't it be good enough to just not add 'Allow' policy? I believe that's all needed (by default access is denied)

[as19ish]

@medikoo As serverless by default allow logs, so I think if we make by default access is denied so this makes more complicate the scenario. Let me know what u think?

[medikoo]

@as19ish What I meant that we should disable access simply by removing "Allow" rules, and not by by replacing them with "Deny" rules (they're effect-less, as they do no override anything)

[as19ish]

@medikoo

I think there is some misunderstanding, I m explaining the whole problem.

By default, we are allowing lambda execution role to this permission

1. logs:CreateLogStream
2. logs:CreateLogGroup
3. logs:PutLogEvents

Note: BY USING WILDCARD CHARACTER

"Statement": [
        {
            "Action": [
                "logs:CreateLogStream",
                "logs:CreateLogGroup"
            ],
            "Resource": [
                "arn:aws:logs:us-east-1:188267231256:log-group:/aws/lambda/serverless-dev*:*"
            ],
            "Effect": "Allow"
        },
        {
            "Action": [
                "logs:PutLogEvents"
            ],
            "Resource": [
                "arn:aws:logs:us-east-1:188267231256:log-group:/aws/lambda/serverless-dev*:*:*"
            ],
            "Effect": "Allow"
        }
    ]

Now the main problem here how can we deny to specific lambda to do not put logs and also by default all lambda has the same lambda execution role.

So here what i'm doing is adding a new statement that denies for put logs for a specific function (full ARN not using a wildcard) as per the Principle of Least Privilege

{
            "Action": "logs:PutLogEvents",
            "Resource": [
                "arn:aws:logs:us-east-1:188267231256:log-group:/aws/lambda/serverless-dev-hello:*"
            ],
            "Effect": "Deny"
}

I m a little confused here. Can we do it in a better way?

[medikoo]

@as19ish ah, indeed, thanks for explanations. I forgot that in common cases we use wildcards, and indeed for this, we'd need to deny the assigned rights.

Still it'll be good to integrate it better with logic which generates "Allow" rights, e.g. we should not create "Deny" rule, for functions not expressed by canonical naming (we should just prevent generation of "Allow" rights for them)

Therefore I would cover this in this part of logic:

serverless/lib/plugins/aws/package/lib/mergeIamTemplates.js

Lines 92 to 129 in 6dd9a31

	this.serverless.service.getAllFunctions().forEach(functionName => {
	const { name: resolvedFunctionName } = this.serverless.service.getFunction(functionName);
	if (!resolvedFunctionName || resolvedFunctionName.startsWith(canonicalFunctionNamePrefix)) {
	hasOneOrMoreCanonicallyNamedFunctions = true;
	return;
	}
	const customFunctionNamelogGroupsPrefix = this.provider.naming.getLogGroupName(
	resolvedFunctionName
	);
	policyDocumentStatements[0].Resource.push({
	'Fn::Sub':
	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
	`:log-group:${customFunctionNamelogGroupsPrefix}:*`,
	});
	policyDocumentStatements[1].Resource.push({
	'Fn::Sub':
	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
	`:log-group:${customFunctionNamelogGroupsPrefix}:*:*`,
	});
	});
	if (hasOneOrMoreCanonicallyNamedFunctions) {
	// Ensure general policies for functions with default name resolution
	policyDocumentStatements[0].Resource.push({
	'Fn::Sub':
	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
	`:log-group:${logGroupsPrefix}*:*`,
	});
	policyDocumentStatements[1].Resource.push({
	'Fn::Sub':
	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
	`:log-group:${logGroupsPrefix}*:*:*`,
	});
	}

1. For non canonical named functions, do not create "Allow" rights
2. Group names of all disabled canonically named functions, and generate "Deny" rights in scope of this condition:

   serverless/lib/plugins/aws/package/lib/mergeIamTemplates.js

   Lines 116 to 129 in 6dd9a31

   	if (hasOneOrMoreCanonicallyNamedFunctions) {
   	// Ensure general policies for functions with default name resolution
   	policyDocumentStatements[0].Resource.push({
   	'Fn::Sub':
   	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
   	`:log-group:${logGroupsPrefix}*:*`,
   	});
   	policyDocumentStatements[1].Resource.push({
   	'Fn::Sub':
   	'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
   	`:log-group:${logGroupsPrefix}*:*:*`,
   	});
   	}

[as19ish]

@medikoo Have a look. I just tried to incorporate your comment. Let me know if anything I missed.

[medikoo]

@as19ish Thank you! It look really good. I've proposed one final improvement we can do

[medikoo]

Let's name this var areLogsDisabled

[medikoo]

Let's put it in else clause (e.g. if all functions named canonically we should just stay at not creating "Allow" policies

[as19ish]

@medikoo ^

[medikoo]

Thank you @as19ish !