aws-logs: LogGroup.addToResourcePolicy 'ARNs must start with "arn:" and have at least 6 components: *'

[ahammond]

Describe the bug

When I create a new LogGroup and then call addToResourcePolicy with a valid policy, I get the error message

    ARNs must start with "arn:" and have at least 6 components: *

Expected Behavior

I should be able to addToResourcePolicy.

Current Behavior

Error message.

Reproduction Steps

https://github.com/ahammond/repro-loggroup-addtoresourcepolicy

import { App, RemovalPolicy, Stack, StackProps } from 'aws-cdk-lib';
import { AnyPrincipal, PolicyStatement } from 'aws-cdk-lib/aws-iam';
import { LogGroup } from 'aws-cdk-lib/aws-logs';
import { Construct } from 'constructs';

export class MyStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const r = new LogGroup(this, 'MyLogGroup', {
      removalPolicy: RemovalPolicy.DESTROY,
    });

    // This fails, no matter what the policy.
    r.addToResourcePolicy(new PolicyStatement({
      actions: ['logs:CreateLogGroupLogStream', 'logs:DescribeLogStreams', 'logs:PutLogEvents'],
      principals: [new AnyPrincipal()],
      resources: ['*'],
    }));
  }
}

// for development, use account/region from cdk cli
const devEnv = {
  account: process.env.CDK_DEFAULT_ACCOUNT,
  region: process.env.CDK_DEFAULT_REGION,
};

const app = new App();
new MyStack(app, 'MyStack',  { env: devEnv });

app.synth();

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.103.1 (build 3bb19ac)

Framework Version

same

Node.js Version

20.5.1

OS

MacOS latest

Language

TypeScript

Language Version

4.9.5

Other information

No response

[msambol]

Investigating.

[msambol]

@ahammond Is your intention to grant everyone access? With AnyPrincipal(), you are granting public or anonymous access.

[laurelmay]

It looks like the issue is that AnyPrincipal extends ArnPrincipal; however, the code in the setup for the resource policy assumes that any ArnPrincipal will be able to be parsed as a full ARN (and of course * can't be).

@msambol if you haven't started work on a patch already, I can take what I've done to look into this and open a PR.

[msambol]

@kylelaker I haven't but I did confirm the API doesn't allow wildcard (*) principals for this:

aws logs put-resource-policy --policy-name test --policy-document '{ "Version": "2012-10-17", "Statement": [ { "Sid": "Test", "Effect": "Allow", "Principal": "*", "Action": ["logs:PutLogEvents"], "Resource": "arn:aws:logs:us-east-1:REDACTED:log-group:/aws/lambda/Test*" } ] }'

An error occurred (InvalidParameterException) when calling the PutResourcePolicy operation: Principal '*' is not permitted for the Actions specified in the resource policy.

However, it does work when you specify a principal:

aws logs put-resource-policy --policy-name test --policy-document '{ "Version": "2012-10-17", "Statement": [ { "Sid": "Test", "Effect": "Allow", "Principal": { "Service": [ "route53.amazonaws.com" ]}, "Action": ["logs:PutLogEvents"], "Resource": "arn:aws:logs:us-east-1:REDACTED:log-group:/aws/lambda/Test*" } ] }'

The docs don't explicity state this, but I put in a feedback request for them to clarify. I was thinking of adding a check here to throw an error if (principal instanceof iam.AnyPrincipal). Feel free to take this if you're looking for something, @kylelaker.

[laurelmay]

So I think this is one of those cases where Principal: { AWS: '*' } and Principal: '*' have a different effect (the CDK differentiates these as AnyPrincipal and StarPrincipal respectively). It seems that the service does accept AnyPrincipal, it's just the CDK that parses it incorrectly in this case; and StarPrincipal is unsupported by the service.

[msambol]

@kylelaker ah, you are right! I thought those two were always synonymous, but alas, I learned something! Curious if you think this should come with a warning (I don't know what protocol is here?) ? Using Principal: { AWS: '*' } makes it public.

[laurelmay]

Curious if you think this should come with a warning (I don't know what protocol is here?)

Probably best for a core team member to weigh in on that. But as a user, my expectation is that if the service lets me do something, the CDK should probably let me do it. Having insecure defaults isn't ideal but it should be configurable. Most of the big security reminders (imo) are probably best left to tools like cdk-nag that are built do do it across the whole stack for a variety of concerns or custom aspects to enforce security constraints on a more granular scale.

[ahammond]

Our plans are all around leveraging conditions for ABAC. I could probably hack together an ARN Principal to get unblocked here. Thanks for the insight!!!

…

On Tue, Oct 31, 2023 at 7:42 PM Kyle Laker ***@***.***> wrote: Curious if you think this should come with a warning (I don't know what protocol is here?) Probably best for a core team member to weigh in on that. But as a user, my expectation is that if the service lets me do something, the CDK should probably let me do it. Having insecure *defaults* isn't ideal but it should be configurable. Most of the big security reminders (imo) are probably best left to tools like cdk-nag <https://github.com/cdklabs/cdk-nag> that are built do do it across the whole stack for a variety of concerns or custom aspects to enforce security constraints on a more granular scale. — Reply to this email directly, view it on GitHub <#27783 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADM2RBMV3WMPN77XIWPMMDYCGZKHAVCNFSM6AAAAAA6YPCNPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBYGMYTIMRXGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ahammond]

I should have read more carefully:

Any ARN Principals inside of the statement will be converted into AWS Account ID strings because CloudWatch Logs Resource Policies do not accept ARN principals.

Oh well. I did

    myLogGroup.addToResourcePolicy(
      new PolicyStatement({
        sid: 'ecs-exec',
        actions: ['logs:CreateLogGroupLogStream', 'logs:DescribeLogStreams', 'logs:PutLogEvents'],
        principals: [new ArnPrincipal(`arn:aws:iam::${this.account}:role/*`)],
        // resources: [
        //   'arn:aws:logs:region:account-id:log-group:log_group_name',
        //   'arn:aws:logs:region:account-id:log-group:log_group_name:*'
        // ],
      }),
    );

And my UTs barfed with:

        "Principal": Object {
    -     "AWS": "arn:aws:iam::514308641592:role/*",
    +     "AWS": "514308641592",
        },

So it appears that to solve this I need to use a

conditions: {
      ArnEquals: {
        'aws:PrincipalArn': `arn:aws:iam::${this.account}:role/*`,
      },
    };

[ahammond]

Also, myLogGroup.addToResourcePolicy() generates an account level policy?!? And... https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-logs-resourcepolicy.html says there's a limit of 10 of these???

I was going to ask why there's no linkage between the LogGroup and the ResourcePolicy, but... wow. Ok. So... ABAC for LogGroups is pretty much off the table?

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.