search

Azure / Azure-Verified-Modules

Azure Verified Modules (AVM) is an initiative to consolidate and set the standards for what a good Infrastructure-as-Code module looks like. Modules will then align to these standards, across languages (Bicep, Terraform etc.) and will then be classified as AVMs and available from their respective language specific registries.
https://aka.ms/AVM
MIT License
335 stars 71 forks source link

[PSRule] Item tracker #349

Open AlexanderSehr opened 10 months ago

AlexanderSehr commented 10 months ago

Description

This issue should be used to track PSRule items that we may want to disable / exclude from a WAF-aligned rule set. Please just comment on the issue.

One example:

JPEasier commented 10 months ago

Rule: Use secure parameters for sensitive information Reasoning: Parameters with the term "secure" in it will be marked. Only real keys, secrets, passwords should higlighted. Exsample

BernieWhite commented 10 months ago

@AlexanderSehr Thanks for the feedback. A persons intent to use an identity vs not is hard to codify. Let's look at options to improve the detection on when a managed identity is actually required. Tracking Azure/PSRule.Rules.Azure#2559.

BernieWhite commented 10 months ago

Rule: Use secure parameters for sensitive information Reasoning: Parameters with the term "secure" in it will be marked. Only real keys, secrets, passwords should higlighted. Exsample

@JPEasier Thanks for the feedback. We'd like to improve this rule, however it might be helpful for me to add some additional context to further discussion on this rule. This rule is intended to detect cases when a secret is not available, which relates to a recent issue.

Consider a case where a developer is instructed to not commit secrets into the repository, which is an important security recommendation. If a developer follows this advice, it doesn't prove or disprove that the module they created is actually handling sensitive data properly, and tooling that is trained to look for valid passwords and tokens won't detect this. PSRule is not going to detect every possible case, but obvious cases ideally should be so they can be detected automatically before manual peer review.

For example, they could provide a password parameter (being obvious) that is using a string parameter type. Someone later deploys this module manually or through an unscanned repository and the password parameter value is now capture in logs and deployment history.

If a developer ignores the instruction, and commits a valid password/ secret/ token into the repository, tooling such as GitHub Advanced Security will detect and block already.

Hopefully that clarifies the issue we are trying to solve with this rule.

Options you have today:

If you have further thoughts I'd love to hear them.

Based on your feedback, we could also consider making this a warning instead of an error, to reduce impact of false positives.

Tracking the false positives as a bug here: https://github.com/Azure/PSRule.Rules.Azure/issues/2556

rahalan commented 9 months ago

Rule: Use Azure Hybrid Benefit Reasoning: We should not enforce to use Hybrid benefit, as it is a license question. We cannot assume, the customer is eligible to it and in that case, enforce a compliance issue.

BernieWhite commented 9 months ago

Rule: Use secure parameters for sensitive information Reasoning: Parameters with the term "secure" in it will be marked. Only real keys, secrets, passwords should higlighted. Exsample

Additional parameter names have been added to reduce some false positives with PSRule for Azure v1.32.0.

BernieWhite commented 9 months ago

Description

This issue should be used to track PSRule items that we may want to disable / exclude from a WAF-aligned rule set. Please just comment on the issue.

One example:

Additional improvements to this will ship with PSRule for Azure v1.33.0.

BernieWhite commented 9 months ago

Rule: Use Azure Hybrid Benefit Reasoning: We should not enforce to use Hybrid benefit, as it is a license question. We cannot assume, the customer is eligible to it and in that case, enforce a compliance issue.

This rule will be disabled by default from PSRule for Azure v1.33.0. Customers can enable this rule if it applies to their environment.

matebarabas commented 5 months ago

@alexandersehr, @JPEasier, @BernieWhite, @rahalan - the AVM issue triage team is reviewing issues tagged as long-term. As part of this effort, we'd like to double check with you if this issue can be closed now? Thank you!

BernieWhite commented 5 months ago

@matebarabas I think all the original issues have been resolved, however I believe this issue was intended as an on-going thread for collaboration, particularly as modules are migrated from CARML.

Deferring to @AlexanderSehr.

AlexanderSehr commented 5 months ago

That's correct. To be fair, more exceptions have been added to the suppression rules in the meantime. Got to go through them some time. cc: @eriqua

matebarabas commented 5 months ago

Thanks @BernieWhite, @AlexanderSehr for the insights! FYI, the transition from CARML is in the finish line: we only have 6 of ~130 modules left. As long as leaving this issue open serves a purpose, all is good. :) I just asked as we were doing some housekeeping. Please make sure it's closed when all related efforts concluded. Thanks!

eriqua commented 3 months ago

@matebarabas @BernieWhite as @AlexanderSehr correctly mentioned, before closing this issue we should at least go through the rules added to the suppression rules, and mention them here if applicable.

However, although the CARML migration is concluded, since both the AVM library and PSRule (not to mention Azure 😄 ) are evolving, we can definitely expect additional rules issues to be added overtime.

How to better track these efforts going forward? Would you advise to close this issue and open new ones as needed or to keep this generic one open as done so far?

Either way, let's define where to host them (AVM/BRM/PSRule) and, if needed, how to categorize them.

Once defined, ideally, module owners should also be informed on the process and be empowered to raise issues themselves.