terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Pin modules by Commit SHA

Open timmeinerzhagen opened this issue 3 years ago • 9 comments

Current Terraform Version

v1.0.10

Problem

Git tags like v3.2.0 are mutable. This means a tag is not guaranteed to point to the same commit forever. Terraform adopts these tags for module versioning e.g. in the registry.

Some malicious party can easily delete a tag and create it again under the same name, just pointing to a new commit which includes their malicious content. Thus, everybody using this version is now using a malicious module.

Attempted Solutions

Using the Terraform Registry directly, only tags are allowed. An option is to reference the source GitHub Repository with the corresponding Commit Hash directly.

Example:

module "acm" {
  source = "github.com/terraform-aws-modules/terraform-aws-acm?ref=21d41965c40fbbb10710ab36404023e4379c7524"
  ...
}

This option however is not very readable and it would mean not using the Terraform Registry is the only safe option. Furthermore, any automatic version upgrade e.g. Dependabot have a harder time with this formatting.

Proposal

Giving the option to directly reference a commit via the Terraform registry reference would be ideal. See below.

module "acm" {
  source  = "terraform-aws-modules/acm/aws"
  version = "21d41965c40fbbb10710ab36404023e4379c7524" # v3.2.0
  ...
}

References

timmeinerzhagen avatar Nov 03 '21 02:11 timmeinerzhagen

Hi @timmeinerzhagen, thanks for your report! In terms of the behavior you are identifying, I verified with the Registry team that your assumptions are correct in terms of the direct link between registry version and source control, and the potential for the code referenced by a tag to change. I have filed an internal feature request issue with the Registry team.

crw avatar Nov 16 '21 00:11 crw

FWIW with all the events of the last year (open source projects being replaced with malicious versions), this limitation makes the Terraform Registry unusable for our team.

zwass avatar Apr 28 '22 01:04 zwass

Hi @timmeinerzhagen, thanks for your report! In terms of the behavior you are identifying, I verified with the Registry team that your assumptions are correct in terms of the direct link between registry version and source control, and the potential for the code referenced by a tag to change. I have filed an internal feature request issue with the Registry team.

Hi @crw, I assume community cannot help with this internal request. Do you have any knowledge where we can see some results of Registry team work?

sobi3ch avatar May 06 '22 09:05 sobi3ch

Hi @sobi3ch, I do not. I have been checking in on it from time-to-time. The Registry team does not work out of this repository (they use an internal bug tracker) and typically I would close Registry issues here, however this issue seems interesting and important enough to keep a tracker open. If there is any update I'll be sure to let this thread know!

crw avatar May 11 '22 00:05 crw

What's the priority on this? Given the recent attention the ESF has given to software supply chain security, this is a critical oversight by Hashicorp. Misformation about "pinning" using tags is opening up massive attack vectors for bad actors. Pinning by hash should be implemented as soon as possible and be the official recommended approach from Hashicorp for module consumers.

jmgilman avatar Dec 29 '22 18:12 jmgilman

Any updates on this? Very concerned that we have lockfile just for providers..... Providers are much more mature and have tighter security on their codebase, generally, when you choose a module, you audit it, you'd hope you can pin it (for real, not just version tag).

fproulx-boostsecurity avatar Jan 26 '23 20:01 fproulx-boostsecurity

No updates. If you are a customer of Terraform Cloud or Terraform Enterprise, please file a ticket through the support channels: email [email protected] or open a new request. Thanks!

crw avatar Jan 26 '23 22:01 crw

This is not on the Registry's roadmap yet, but I will be moving this feature request into our internal backlog. @crw are you opposed to me closing it here?

opslivia avatar May 26 '23 06:05 opslivia

I created a module that addresses this problem. It's an interim solution, but worth trying. Still experimental so feedback would be appreciated.

https://registry.terraform.io/modules/Invicton-Labs/module-lock/null/latest

KyleKotowick avatar May 03 '24 23:05 KyleKotowick

This issue seems to be solved as explained in this comment: https://discuss.hashicorp.com/t/third-party-terraform-module-security-reproducibility/6995/8

ThomasVanR avatar Jun 20 '24 09:06 ThomasVanR

Modules are indeed pinned by commit SHA in the Terraform registry, mitigating the concern expressed in this issue.

As @apparentlymart says in the linked forum thread:

Today’s Terraform Registry now checks which commit the tag refers to at import time and then always returns that specific commit for any future request. Even if the tag in the repository changes to refer to a different commit later, the registry will continue returning the commit it originally discovered.

Note that the idea of client-side locking of modules by hash, such as is implemented by @KyleKotowick's module, is a separate feature and tracked by https://github.com/hashicorp/terraform/issues/17110.

kmoe avatar Jun 26 '24 15:06 kmoe

@kmoe HashiCorp is missing a huge security vulnerability here. Module versions are pinned by commit SHA in the registry, but if the module author simply deletes a module version in the Registry and re-publishes it after moving the version tag in GitHub to a malicious commit, then once the CloudFront cache expires (7 days) it will start responding with the new (malicious) SHA hash, even though the same version number is being used.

I have sent complete reproducibility steps and a CVE ID request to [email protected].

KyleKotowick avatar Jun 26 '24 16:06 KyleKotowick

If what @KyleKotowick is describing is true, that is very concerning. This issue should be reopened until that issue is remediated. @kmoe

zwass avatar Jun 26 '24 17:06 zwass

Thanks for this feedback. While we are still researching this new report, we continue to believe the correct solution will be in relation to https://github.com/hashicorp/terraform/issues/17110 -- so this issue will remain closed for the previously stated reasons. Thanks!

crw avatar Jun 26 '24 19:06 crw

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Jul 27 '24 02:07 github-actions[bot]