Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqladmin.googleapis.com/Instance requireSSL Not Triggering #124

Open
jdyke opened this issue Jul 23, 2019 · 8 comments · Fixed by #125
Open

sqladmin.googleapis.com/Instance requireSSL Not Triggering #124

jdyke opened this issue Jul 23, 2019 · 8 comments · Fixed by #125

Comments

@jdyke
Copy link

jdyke commented Jul 23, 2019

gcp_sql_ssl_v1.yaml template:

Is looking for:

asset.resource.settings.ipConfiguration.requireSsl == false

but the CAI returns:

[
  {
    "name": "<redacted>",
    "asset_type": "sqladmin.googleapis.com/Instance",
    "ancestry_path": "<redacted>",
    "resource": {
      "version": "v1beta4",
      "discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/sqladmin/v1beta4/rest",
      "discovery_name": "DatabaseInstance",
      "parent": "<redacted>",
      "data": {
        "databaseVersion": "POSTGRES_9_6",
        "name": "master-instance",
        "project": "<redacted>",
        "region": "us-central1",
        "settings": {
          "ipConfiguration": {
            "ipv4Enabled": true,
            "requireSsl": false
          },
          "pricingPlan": "PER_USE",
          "replicationType": "SYNCHRONOUS",
          "storageAutoResize": true,
          "tier": "db-f1-micro"
        }
      }
    }
  }
]

It looks like it is missing "data" so I added it to the template:

asset.resource.data.settings.ipConfiguration.requireSsl == false

but that still does not catch the entry:

variable project {}

provider "google" {
  project = "${var.project}"
  region  = "us-east1"
}

resource "google_sql_database_instance" "master" {
  name = "master-instance"
  database_version = "POSTGRES_9_6"
  region = "us-central1"

  settings {
    # Second-generation instance tiers are based on the machine
    # type. See argument reference below.
    tier = "db-f1-micro"
    ip_configuration {
        ipv4_enabled = "true"
        require_ssl = "false"
    }
  }
}
@morgante
Copy link
Contributor

morgante commented Jul 23, 2019

Please test in Forseti. Entangling separate problems in one issue makes it harder to test.

@morgante
Copy link
Contributor

@jdyke I opened #125 to address the issue in the constraint template.

I tested the conversion in Terraform Validator and it looks like it properly converts the requireSsl prop:

{
	"name": "//cloudsql.googleapis.com/projects/gcp-foundation-shared-devops/instances/master-instance",
	"asset_type": "sqladmin.googleapis.com/Instance",
	"ancestry_path": "organization/816421441114/project/gcp-foundation-shared-devops",
	"resource": {
		"version": "v1beta4",
		"discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/sqladmin/v1beta4/rest",
		"discovery_name": "DatabaseInstance",
		"parent": "//cloudresourcemanager.googleapis.com/projects/gcp-foundation-shared-devops",
		"data": {
			"databaseVersion": "POSTGRES_9_6",
			"name": "master-instance",
			"project": "gcp-foundation-shared-devops",
			"region": "us-central1",
			"settings": {
				"ipConfiguration": {
					"ipv4Enabled": true,
					"requireSsl": false
				},
				"pricingPlan": "PER_USE",
				"replicationType": "SYNCHRONOUS",
				"storageAutoResize": true,
				"tier": "db-f1-micro"
			}
		}
	}
}

@FanchenBao
Copy link
Contributor

The same problem happens with gcp_sql_allowed_authorized_networks_v1.yaml.

For gcp_sql_allowed_authorized_networks_v1.yaml, In addition to the missing data field, the bug is in the package. The original statement is package templates.gcp.GCPSQLAllowedAuthorizedNetworksV1. When I change it into package templates.gcp.GCPSQLAllowedAuthorizedNetworksConstraintV1 (just add Constraint at the end before V1), it worked like a charm.

So I suspect for gcp_sql_ssl_v1.yaml, the same procedure shall work as well:

  1. add data field
  2. Change package templates.gcp.GCPSQLSSLV1 into package templates.gcp.GCPSQLSSLConstraintV1

I haven't tested it yet for gcp_sql_ssl_v1.yaml yet.

@morgante
Copy link
Contributor

@FanchenBao Yes, I suspect you are correct. a PR is welcome, of course.

@ocervell
Copy link
Contributor

ocervell commented Sep 13, 2019

@morgante @AdrienWalkowiak I've verified this rule works from config-validator, but is still not working from the Forseti scanner, which reports no violation. Even after #125 and #127

@morgante morgante reopened this Sep 13, 2019
@morgante
Copy link
Contributor

@ocervell Does it work with Scorecard?

@AdrienWalkowiak
Copy link
Contributor

AdrienWalkowiak commented Apr 20, 2020

Any update on this @ocervell?

@gkowalski-google
Copy link
Contributor

gkowalski-google commented Apr 20, 2020

@AdrienWalkowiak @ocervell This was likely fixed for Forseti with this PR. Forseti had some incorrect logic where it was setting falsy values to None. The change can be tested from master branch of Forseti; will be included in v2.26.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants