feat(nifcloud): add nifcloud engine support

[tunakyonn]

Proposed Changes

• Related feat(queries): add terraform nifcloud queries #6265 .
• Added these Terraform NIFCLOUD engine support.
  • feat(queries): add terraform nifcloud queries #6265 (comment)

I submit this contribution under the Apache-2.0 license.

[tunakyonn]

Hi @gabriel-cx ,

I created a PR with the content you suggested.
But I am getting an error in unit-tests because the 'assets/queries/terraform/nifcloud/*' files are missing.
Should the queries file be added with this PR?

[tunakyonn]

Hi @gabriel-cx ,

Can you confirm the sentence above?

[gabriel-cx]

Hi @tunakyonn ,

Sorry for the delay on your previous question!
Yes, you right. You can add one query into this PR. And then, in the future, we can create separated PRs to add more queries.

What do you think?

[tunakyonn]

Hi @gabriel-cx ,

Thank you for your reply.
I added one nifcloud query to PR.
However, the content of kicsbot's comment should be supported, but an error is occurring.
Is it because I imported the latest branch midway through?
Please let me know if I should create a new pr.

[gabriel-cx]

Hi @tunakyonn ,

I updated your PR title and description to match our requirements, for now should be good, don't worry! :D

In terms of the query itself, thank you very much!
I noticed that you created the query in the root folder of "nifcloud", kindly create a subfolder inside "nifcloud" with the name of the query, like "nifcloud_computing_has_common_private_network" and inside of that subfolder you can add the metadata and query.rego files + the test folder of the query.

[tunakyonn]

Hi @gabriel-cx ,

Thanks for your comment!
Sorry, I pushed the wrong query file path.
I fixed it, but is there any problem here?

[gabriel-cx]

Hi @tunakyonn

Looks like your PR is failing because of the Unit Tests. Seems like your query is not catching the vulnerability you expect. Could you have a look? If you need support, just ping us!

[tunakyonn]

Hi @gabriel-cx ,

Thanks for your comment!
I'm currently fixing the error, but I have a question.
I would like to detect even if there are multiple network_interfaces like the tf file below.

resource "nifcloud_instance" "positive" {
  image_id        = data.nifcloud_image.ubuntu.id
  security_group  = nifcloud_security_group.example.group_name
  network_interface {
    network_id = "net-COMMON_GLOBAL"
  }
  network_interface {
    network_id = "net-COMMON_PRIVATE"
  }
}

However, if you write the following in query.rego, an error will occur during testing and it cannot be detected.
Could you please give me some advice on how to write query.rego?

	instance := input.document[i].resource.nifcloud_instance[name]
	instance.network_interface[_].network_id == "net-COMMON_PRIVATE"

[gabriel-cx]

Hi @tunakyonn ,

No problem!

In order to catch single network_interface or multiple network_interfaces try to have 2 policies in your rego file, like:

CxPolicy[result] {

	instance := input.document[i].resource.nifcloud_instance[name]
    instance.network_interface[_].network_id == "net-COMMON_PRIVATE"

	result := {
		"documentId": input.document[i].id,
		"resourceType": "nifcloud_instance",
		"resourceName": tf_lib.get_resource_name(instance, name),
		"searchKey": sprintf("nifcloud_instance[%s]", [name]),
		"issueType": "IncorrectValue",
		"keyExpectedValue": sprintf("'nifcloud_instance[%s]' should use a private LAN to isolate the private side network from the shared network", [name]),
		"keyActualValue": sprintf("'nifcloud_instance[%s]' has common private network", [name]),
	}
}

and

CxPolicy[result] {

	instance := input.document[i].resource.nifcloud_instance[name]
    instance.network_interface.network_id == "net-COMMON_PRIVATE"

	result := {
		"documentId": input.document[i].id,
		"resourceType": "nifcloud_instance",
		"resourceName": tf_lib.get_resource_name(instance, name),
		"searchKey": sprintf("nifcloud_instance[%s]", [name]),
		"issueType": "IncorrectValue",
		"keyExpectedValue": sprintf("'nifcloud_instance[%s]' should use a private LAN to isolate the private side network from the shared network", [name]),
		"keyActualValue": sprintf("'nifcloud_instance[%s]' has common private network", [name]),
	}
}

Where the first policy tries to find if there is more than one network_interfaces and the second tries to check if there is only one network_interface.

Kindly tell us if this information was useful!

[tunakyonn]

Hi @gabriel-cx ,

Thanks for your advice!
I implemented it based on the comments and now the unit test passes!
I would like to ask for your review again.

Also, I would like to ask, why can't I catch a single network_interface by writing something like the following?

instance.network_interface[_].network_id == "net-COMMON_PRIVATE"

[gabriel-cx]

Hi @tunakyonn ,

No problem! Thanks for the fix!

Yeah, is a good question!
The reason is, when you only have a single network_interface, the parser in the background creates it as a single item, and that's why you need to catch it by using instance.network_interface.network_id.

When you have more than one network_interface the parser in the background detects that there is more than one item of that type and creates a list of them, and to 'catch' those we need to specify it as a list, like instance.network_interface[_].network_id

You can obtain the 'parser result' (internal JSON representation, e.g: the 2 images I sent previously) by checking the payload. And you can create/access the payload by running the -d (--payload-path) KICS flag.

Hope this information clarifies your doubts!

[tunakyonn]

Hi @gabriel-cx ,

Thank you for your comment!
I understand about instance.network_interface[_].network_id logic.

My fix is complete, but I still have go-ci-integration and go-e2e errors.
Is there anything I should do myself?

[gabriel-cx]

Hi @tunakyonn ,

We are checking what can be the issue and we will notify you if you need to do some changes.

[gabriel-cx]

Hi @tunakyonn ,

Looks like you need to add "nifcloud" here! :D

[tunakyonn]

Hi @gabriel-cx ,

Thank you for your advice!
I tried modifying the relevant file.
I also rebased and pushed the contents of the latest master branch to this branch.

[gabriel-cx]

@tunakyonn looks like everything is green now, thanks!!
Now what will happen is, we have this PR open and we are testing it and reviewing it. When that PR is merged, we will be able to test your PR and then merge it as well (after small changes we will request you)! We plan to do it in the next 1-2 weeks.

[gabriel-cx]

Hi @tunakyonn ,

We plan to merge this PR and make it available in the next KICS version (1.7.9).
It means, in the next weeks we will try to review your PR and merge it and make it available in the KICS version 1.7.10! :D
In the next 2 weeks we will only ask you to perform a little change so your feature can be merged. We will keep you updated. Thanks again for the help!

[tunakyonn]

Hi @gabriel-cx ,

Thank you for contacting!
I look forward to seeing this pull request merged.

Is it okay to create the second pull request that was discussed in the link below after this pull request is merged?
#6265 (comment)

[gabriel-cx]

Hi @tunakyonn ,

Sure! Feel free to create a new PR with new nifcloud queries! We'd love to see that new PR!

[gabriel-cx]

@tunakyonn if you prefer, you can create now the new PR with new branch based on this current branch. So, like that we can review both and merge the 2 when everything looks good!

Steps that we recommend:

1. create new branch (add_nifcloud_queries) from this branch
2. create PR from the new branch to Checkmarx:master

That way we can merge this current PR first and then the new one

[gabriel-cx]

Hi @tunakyonn ,

Hope this message finds you well!

I'm happy to announce that soon we will be able to merge your PR! :D
We want to ask you to add the nifcloud logo under the "Beta Features" here and here. You can just remove the "Soon..." and add your logo.
Kindly store your logo image here with a name like "logo-nifcloud.png".

Thank you again!!

[tunakyonn]

Hi @gabriel-cx ,

Sorry for the late confirmation.

I understand about the logo.
I am currently checking to see if I can use the logo internally.
It's possible that I won't be able to upload it, but is that still okay?

[gabriel-cx]

Hi @tunakyonn ,

No problem!

The logo is not a mandatory thing.
But, there is some blocker regarding adding the logo?

In case you can't add the logo for some reason, tell us and we can support you on that if needed.

[tunakyonn]

Hi @gabriel-cx ,

Thank you for your kind communication.
I check with the relevant department within the company regarding the handling of the logo.
However, since I have not received a response yet, I may not be able to provide the logo in time for release.

[gabriel-cx]

Hi @tunakyonn ,

Thank you for your information.

No problem, in the end, if you can't add the logo we can provide it from our side.

Currently, we changed the way we want to approach the "Beta features". This means, we are waiting this PR to be merged so we can merge yours as well. I will keep you updated!

[tunakyonn]

Hi @gabriel-cx ,

Thank you for your kind communication!
I don't think we can provide the logo because I haven't received a reply from the department in charge yet.

I also understand about PR.

[gabriel-cx]

Hi @tunakyonn ,

Thank you for your inputs.
I understand, we can manage the logo part from our side.

The only changes we'd like to ask you is, kindly add "experimental": "true" in every metadata.json files of every query (in this PR you just have one query so you just need to change one metadata file).

KICS will have a new flag --experimental-queries, this means, KICS will only run queries that have the "experimental": "true" if the user runs KICS with that flag.
Since our AppSec team didn't review yet your queries, that's why we want to set them as "experimental". When our AppSec team review your queries and agree with them, we will remove the "experimental": "true" field from the metadata and the queries will became official and not experimental. ^^

And I also noticed that you have some conflicts in your PR, if you need our support to tackle it, just ping me!

[tunakyonn]

Hi @gabriel-cx ,

Thank you for contacting me.
The experimental setup you requested has been completed.
I have also resolved the conflict, so please check it.

[gabriel-cx]

@tunakyonn thank you! It's merged! :D