search

GoogleCloudPlatform / policy-library

A library of constraint templates and sample constraints for Constraint Framework tools
Apache License 2.0
223 stars 130 forks source link

gcp-iam-audit-log-v1 Policy Is Not Working #176

Open blueandgold opened 4 years ago

blueandgold commented 4 years ago

Opening this in Forseti repo, since this affects Forseti and not other Config Validator clients.

The problem is that gcp-iam-audit-log-v1 policy does not work in forseti because the audit_config field gets ignored when converting from CAI json to proto.

Edit: Moving this to the policy-library since this is affecting not just Forseti.

blueandgold commented 4 years ago

@joecheuk is there a reason why we do the ignoring of fields here, while the other CV clients does not?

@katze120 for FYI.

katze120 commented 4 years ago

In cft scorecard, the conversion leverages terraform validator converter which only takes bindings field from iam_policy and implicitly dropping audit_configs field. And similarly in terraform validator itself. FYI @morgante

Is there any CV client that works fine with audit_configs field?

morgante commented 4 years ago

@katze120 Are you sure scorecard doesn't provide it? Scorecard uses the type from tfvalidator but nothing else.

katze120 commented 4 years ago

yes, from actual test results and also the Asset struct definition in addition to links provided above

When I bypass tfvalidator and directly try to convert interface{} variable to proto, it gives me Error: validating: FCV: converting asset to proto: unmarshaling to proto: unknown field "audit_configs" in iam.Policy

morgante commented 4 years ago

Got it, so we're likely encountering the same issue as Forseti (the protodef for IAM policies doesn't include audit_configs).

xingao267 commented 4 years ago

Is this due to the same issue as https://github.com/forseti-security/policy-library/issues/367?

xingao267 commented 4 years ago

From my understanding on the discussion, to fix this issue, we need

  1. Add audit_configs in the public IAM proto https://github.com/googleapis/googleapis/blob/master/google/iam/v1/policy.proto. I've contacted the IAM team for that. This should fix CV, CFT Scorecard and Forseti (when using the version of CV which has this fix).

  2. Add support for audit_configs in https://github.com/GoogleCloudPlatform/cloud-foundation-toolkit/blob/master/cli/vendor/github.com/GoogleCloudPlatform/terraform-validator/converters/google/convert.go#L226-L235, which should then let Terraform Validator also see audit_configs field.

Is that correct?

morgante commented 4 years ago

@xingao267 Your analysis is correct. I'd be happy to look at a PR if you have a chance to fix this.