terraform-linters / tflint

A Pluggable Terraform Linter
Mozilla Public License 2.0
4.88k stars 354 forks source link

`tflint --init` does not check signing key expiration date #1768

Open wata727 opened 1 year ago

wata727 commented 1 year ago

Summary

When installing plugins with tflint --init, plugins under terraform-linters or plugins with explicitly set signing_key will have their signatures verified by PGP.

https://github.com/terraform-linters/tflint/blob/c1aef408b425530cc0509b66c8f253283a163e96/plugin/signature.go#L55

However, the golang.org/x/crypto/openpgp.CheckDetachedSignature does not return an error even if the signing key has expired.

I first noticed this issue when I made a mistake when extending the expiration date of a PGP key in https://github.com/terraform-linters/tflint/pull/1679. This PR updates the built-in key, but the process is not correct and the key expiration date is still on 2023-05-01.

However, TFLint v0.46.1 was able to successfully verify the signature of AWS ruleset v0.23.1 signed with a new key that was updated in a correct process, despite using an expired key. See also https://github.com/terraform-linters/tflint-ruleset-aws/issues/496.

Digging deeper into this issue, I came across the following PR on Terraform.

https://github.com/hashicorp/terraform/pull/32056

There is one change in the fork that's arguably backwards-incompatible: CheckDetachedSignature() will now return an error if the key used to sign the signature is expired (see https://github.com/ProtonMail/go-crypto/pull/60).

Indeed, looking at golang.org/x/crypto/openpgp.CheckDetachedSignature there is no key expiration date checked. This package is already deprecated and frozen, and we should switch to a community fork like ProtonMail/go-crypto as well to check the expiration date.

After replacing with this fork and running tflint --init, the installation will indeed fail due to key expiry.

$ tflint --init
Installing `aws` plugin...
Failed to install a plugin; Failed to check checksums.txt signature: openpgp: key expired

While this is a security issue, the most used built-in key has never been compromised, and the expiration date has never been important, so currently the user impact is minimal.

Command

tflint --init

Terraform Configuration

# None

TFLint Configuration

plugin "aws" {
  enabled = true
  version = "0.23.1"
  source  = "github.com/terraform-linters/tflint-ruleset-aws"
}

Output

Expected behavior:

Installing `aws` plugin...
Failed to install a plugin; Failed to check checksums.txt signature: openpgp: key expired

Actual behavior:

Installing `aws` plugin...
Installed `aws` (source: github.com/terraform-linters/tflint-ruleset-aws, version: 0.23.1)

TFLint Version

0.46.1

Terraform Version

No response

Operating System

wata727 commented 1 year ago

1769 solves this problem but at the cost of causing all tflint --init to fail.

Also, with expiration working correctly, there is an issue where tflint --init will stop working completely in older versions if the key expires. I'm concerned about the impact this will have on our users.

I think we need to think a little more about how key expiration should be managed.