From 495826444bde4efe24456f6f96e15d0b751e6486 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Wed, 14 Nov 2018 14:52:46 -0500 Subject: [PATCH 1/4] plugin/discovery: Use GPG keys from Registry When verifying the signature of the SHA256SUMS file, we have been hardcoding HashiCorp's public GPG key and using it as the keyring. Going forward, Terraform will get a list of valid public keys for a provider from the Terraform Registry (registry.terraform.io), and use them as the keyring for the openpgp verification func. --- plugin/discovery/get.go | 48 +++++++++--------- plugin/discovery/get_test.go | 36 +++++++++++++- plugin/discovery/signature.go | 39 ++------------- plugin/discovery/testdata/hashicorp.asc | 30 +++++++++++ registry/response/terraform_provider.go | 28 +++++++++++ registry/response/terraform_provider_test.go | 52 ++++++++++++++++++++ 6 files changed, 174 insertions(+), 59 deletions(-) create mode 100644 plugin/discovery/testdata/hashicorp.asc create mode 100644 registry/response/terraform_provider_test.go diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index ed96bfabd..d26e8e346 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -15,7 +15,6 @@ import ( getter "github.com/hashicorp/go-getter" multierror "github.com/hashicorp/go-multierror" - "github.com/hashicorp/terraform/httpclient" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/registry/regsrc" @@ -353,12 +352,36 @@ func (i *ProviderInstaller) PurgeUnused(used map[string]PluginMeta) (PluginMetaS } func (i *ProviderInstaller) getProviderChecksum(urls *response.TerraformProviderPlatformLocation) (string, error) { - checksums, err := getPluginSHA256SUMs(urls.ShasumsURL, urls.ShasumsSignatureURL) + // Get SHA256SUMS file. + shasums, err := getFile(urls.ShasumsURL) + if err != nil { + return "", fmt.Errorf("error fetching checksums: %s", err) + } + + // Get SHA256SUMS.sig file. + signature, err := getFile(urls.ShasumsSignatureURL) + if err != nil { + return "", fmt.Errorf("error fetching checksums signature: %s", err) + } + + // Verify GPG signature. + asciiArmor := urls.SigningKeys.GPGASCIIArmor() + signer, err := verifySig(shasums, signature, asciiArmor) if err != nil { return "", err } - return checksumForFile(checksums, urls.Filename), nil + // Display identity for GPG key which succeeded verifying the signature. + // This could also be used to display to the user with i.Ui.Info(). + identities := []string{} + for k, _ := range signer.Identities { + identities = append(identities, k) + } + identity := strings.Join(identities, ", ") + log.Printf("[DEBUG] verified GPG signature with key from %s", identity) + + // Extract checksum for this os/arch platform binary. + return checksumForFile(shasums, urls.Filename), nil } // list all versions available for the named provider @@ -487,25 +510,6 @@ func checksumForFile(sums []byte, name string) string { return "" } -// fetch the SHA256SUMS file provided, and verify its signature. -func getPluginSHA256SUMs(sumsURL, sigURL string) ([]byte, error) { - sums, err := getFile(sumsURL) - if err != nil { - return nil, fmt.Errorf("error fetching checksums: %s", err) - } - - sig, err := getFile(sigURL) - if err != nil { - return nil, fmt.Errorf("error fetching checksums signature: %s", err) - } - - if err := verifySig(sums, sig); err != nil { - return nil, err - } - - return sums, nil -} - func getFile(url string) ([]byte, error) { resp, err := httpClient.Get(url) if err != nil { diff --git a/plugin/discovery/get_test.go b/plugin/discovery/get_test.go index ce833ee36..534a01fa5 100644 --- a/plugin/discovery/get_test.go +++ b/plugin/discovery/get_test.go @@ -369,23 +369,57 @@ func TestProviderInstallerPurgeUnused(t *testing.T) { // Test fetching a provider's checksum file while verifying its signature. func TestProviderChecksum(t *testing.T) { + hashicorpKey, err := ioutil.ReadFile("testdata/hashicorp.asc") + if err != nil { + t.Fatal(err) + } + tests := []struct { + Name string URLs *response.TerraformProviderPlatformLocation Err bool }{ { + "good", &response.TerraformProviderPlatformLocation{ + Filename: "terraform-provider-template_0.1.0_darwin_amd64.zip", ShasumsURL: "http://127.0.0.1:8080/terraform-provider-template/0.1.0/terraform-provider-template_0.1.0_SHA256SUMS", ShasumsSignatureURL: "http://127.0.0.1:8080/terraform-provider-template/0.1.0/terraform-provider-template_0.1.0_SHA256SUMS.sig", - Filename: "terraform-provider-template_0.1.0_darwin_amd64.zip", + SigningKeys: response.SigningKeyList{ + GPGKeys: []*response.GPGKey{ + &response.GPGKey{ + ASCIIArmor: string(hashicorpKey), + }, + }, + }, }, false, }, { + "bad", &response.TerraformProviderPlatformLocation{ + Filename: "terraform-provider-template_0.1.0_darwin_amd64.zip", ShasumsURL: "http://127.0.0.1:8080/terraform-provider-badsig/0.1.0/terraform-provider-badsig_0.1.0_SHA256SUMS", ShasumsSignatureURL: "http://127.0.0.1:8080/terraform-provider-badsig/0.1.0/terraform-provider-badsig_0.1.0_SHA256SUMS.sig", + SigningKeys: response.SigningKeyList{ + GPGKeys: []*response.GPGKey{ + &response.GPGKey{ + ASCIIArmor: string(hashicorpKey), + }, + }, + }, + }, + true, + }, + { + "no keys", + &response.TerraformProviderPlatformLocation{ Filename: "terraform-provider-template_0.1.0_darwin_amd64.zip", + ShasumsURL: "http://127.0.0.1:8080/terraform-provider-template/0.1.0/terraform-provider-template_0.1.0_SHA256SUMS", + ShasumsSignatureURL: "http://127.0.0.1:8080/terraform-provider-template/0.1.0/terraform-provider-template_0.1.0_SHA256SUMS.sig", + SigningKeys: response.SigningKeyList{ + GPGKeys: []*response.GPGKey{}, + }, }, true, }, diff --git a/plugin/discovery/signature.go b/plugin/discovery/signature.go index b6686a5d5..4e941aec6 100644 --- a/plugin/discovery/signature.go +++ b/plugin/discovery/signature.go @@ -10,44 +10,11 @@ import ( // Verify the data using the provided openpgp detached signature and the // embedded hashicorp public key. -func verifySig(data, sig []byte) error { - el, err := openpgp.ReadArmoredKeyRing(strings.NewReader(hashiPublicKey)) +func verifySig(data, sig []byte, armor string) (*openpgp.Entity, error) { + el, err := openpgp.ReadArmoredKeyRing(strings.NewReader(armor)) if err != nil { log.Fatal(err) } - _, err = openpgp.CheckDetachedSignature(el, bytes.NewReader(data), bytes.NewReader(sig)) - return err + return openpgp.CheckDetachedSignature(el, bytes.NewReader(data), bytes.NewReader(sig)) } - -// this is the public key that signs the checksums file for releases. -const hashiPublicKey = `-----BEGIN PGP PUBLIC KEY BLOCK----- -Version: GnuPG v1 - -mQENBFMORM0BCADBRyKO1MhCirazOSVwcfTr1xUxjPvfxD3hjUwHtjsOy/bT6p9f -W2mRPfwnq2JB5As+paL3UGDsSRDnK9KAxQb0NNF4+eVhr/EJ18s3wwXXDMjpIifq -fIm2WyH3G+aRLTLPIpscUNKDyxFOUbsmgXAmJ46Re1fn8uKxKRHbfa39aeuEYWFA -3drdL1WoUngvED7f+RnKBK2G6ZEpO+LDovQk19xGjiMTtPJrjMjZJ3QXqPvx5wca -KSZLr4lMTuoTI/ZXyZy5bD4tShiZz6KcyX27cD70q2iRcEZ0poLKHyEIDAi3TM5k -SwbbWBFd5RNPOR0qzrb/0p9ksKK48IIfH2FvABEBAAG0K0hhc2hpQ29ycCBTZWN1 -cml0eSA8c2VjdXJpdHlAaGFzaGljb3JwLmNvbT6JATgEEwECACIFAlMORM0CGwMG -CwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEFGFLYc0j/xMyWIIAIPhcVqiQ59n -Jc07gjUX0SWBJAxEG1lKxfzS4Xp+57h2xxTpdotGQ1fZwsihaIqow337YHQI3q0i -SqV534Ms+j/tU7X8sq11xFJIeEVG8PASRCwmryUwghFKPlHETQ8jJ+Y8+1asRydi -psP3B/5Mjhqv/uOK+Vy3zAyIpyDOMtIpOVfjSpCplVRdtSTFWBu9Em7j5I2HMn1w -sJZnJgXKpybpibGiiTtmnFLOwibmprSu04rsnP4ncdC2XRD4wIjoyA+4PKgX3sCO -klEzKryWYBmLkJOMDdo52LttP3279s7XrkLEE7ia0fXa2c12EQ0f0DQ1tGUvyVEW -WmJVccm5bq25AQ0EUw5EzQEIANaPUY04/g7AmYkOMjaCZ6iTp9hB5Rsj/4ee/ln9 -wArzRO9+3eejLWh53FoN1rO+su7tiXJA5YAzVy6tuolrqjM8DBztPxdLBbEi4V+j -2tK0dATdBQBHEh3OJApO2UBtcjaZBT31zrG9K55D+CrcgIVEHAKY8Cb4kLBkb5wM -skn+DrASKU0BNIV1qRsxfiUdQHZfSqtp004nrql1lbFMLFEuiY8FZrkkQ9qduixo -mTT6f34/oiY+Jam3zCK7RDN/OjuWheIPGj/Qbx9JuNiwgX6yRj7OE1tjUx6d8g9y -0H1fmLJbb3WZZbuuGFnK6qrE3bGeY8+AWaJAZ37wpWh1p0cAEQEAAYkBHwQYAQIA -CQUCUw5EzQIbDAAKCRBRhS2HNI/8TJntCAClU7TOO/X053eKF1jqNW4A1qpxctVc -z8eTcY8Om5O4f6a/rfxfNFKn9Qyja/OG1xWNobETy7MiMXYjaa8uUx5iFy6kMVaP -0BXJ59NLZjMARGw6lVTYDTIvzqqqwLxgliSDfSnqUhubGwvykANPO+93BBx89MRG -unNoYGXtPlhNFrAsB1VR8+EyKLv2HQtGCPSFBhrjuzH3gxGibNDDdFQLxxuJWepJ -EK1UbTS4ms0NgZ2Uknqn1WRU1Ki7rE4sTy68iZtWpKQXZEJa0IGnuI2sSINGcXCJ -oEIgXTMyCILo34Fa/C6VCm2WBgz9zZO8/rHIiQm1J5zqz0DrDwKBUM9C -=LYpS ------END PGP PUBLIC KEY BLOCK-----` diff --git a/plugin/discovery/testdata/hashicorp.asc b/plugin/discovery/testdata/hashicorp.asc new file mode 100644 index 000000000..010c9271c --- /dev/null +++ b/plugin/discovery/testdata/hashicorp.asc @@ -0,0 +1,30 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- +Version: GnuPG v1 + +mQENBFMORM0BCADBRyKO1MhCirazOSVwcfTr1xUxjPvfxD3hjUwHtjsOy/bT6p9f +W2mRPfwnq2JB5As+paL3UGDsSRDnK9KAxQb0NNF4+eVhr/EJ18s3wwXXDMjpIifq +fIm2WyH3G+aRLTLPIpscUNKDyxFOUbsmgXAmJ46Re1fn8uKxKRHbfa39aeuEYWFA +3drdL1WoUngvED7f+RnKBK2G6ZEpO+LDovQk19xGjiMTtPJrjMjZJ3QXqPvx5wca +KSZLr4lMTuoTI/ZXyZy5bD4tShiZz6KcyX27cD70q2iRcEZ0poLKHyEIDAi3TM5k +SwbbWBFd5RNPOR0qzrb/0p9ksKK48IIfH2FvABEBAAG0K0hhc2hpQ29ycCBTZWN1 +cml0eSA8c2VjdXJpdHlAaGFzaGljb3JwLmNvbT6JATgEEwECACIFAlMORM0CGwMG +CwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEFGFLYc0j/xMyWIIAIPhcVqiQ59n +Jc07gjUX0SWBJAxEG1lKxfzS4Xp+57h2xxTpdotGQ1fZwsihaIqow337YHQI3q0i +SqV534Ms+j/tU7X8sq11xFJIeEVG8PASRCwmryUwghFKPlHETQ8jJ+Y8+1asRydi +psP3B/5Mjhqv/uOK+Vy3zAyIpyDOMtIpOVfjSpCplVRdtSTFWBu9Em7j5I2HMn1w +sJZnJgXKpybpibGiiTtmnFLOwibmprSu04rsnP4ncdC2XRD4wIjoyA+4PKgX3sCO +klEzKryWYBmLkJOMDdo52LttP3279s7XrkLEE7ia0fXa2c12EQ0f0DQ1tGUvyVEW +WmJVccm5bq25AQ0EUw5EzQEIANaPUY04/g7AmYkOMjaCZ6iTp9hB5Rsj/4ee/ln9 +wArzRO9+3eejLWh53FoN1rO+su7tiXJA5YAzVy6tuolrqjM8DBztPxdLBbEi4V+j +2tK0dATdBQBHEh3OJApO2UBtcjaZBT31zrG9K55D+CrcgIVEHAKY8Cb4kLBkb5wM +skn+DrASKU0BNIV1qRsxfiUdQHZfSqtp004nrql1lbFMLFEuiY8FZrkkQ9qduixo +mTT6f34/oiY+Jam3zCK7RDN/OjuWheIPGj/Qbx9JuNiwgX6yRj7OE1tjUx6d8g9y +0H1fmLJbb3WZZbuuGFnK6qrE3bGeY8+AWaJAZ37wpWh1p0cAEQEAAYkBHwQYAQIA +CQUCUw5EzQIbDAAKCRBRhS2HNI/8TJntCAClU7TOO/X053eKF1jqNW4A1qpxctVc +z8eTcY8Om5O4f6a/rfxfNFKn9Qyja/OG1xWNobETy7MiMXYjaa8uUx5iFy6kMVaP +0BXJ59NLZjMARGw6lVTYDTIvzqqqwLxgliSDfSnqUhubGwvykANPO+93BBx89MRG +unNoYGXtPlhNFrAsB1VR8+EyKLv2HQtGCPSFBhrjuzH3gxGibNDDdFQLxxuJWepJ +EK1UbTS4ms0NgZ2Uknqn1WRU1Ki7rE4sTy68iZtWpKQXZEJa0IGnuI2sSINGcXCJ +oEIgXTMyCILo34Fa/C6VCm2WBgz9zZO8/rHIiQm1J5zqz0DrDwKBUM9C +=LYpS +-----END PGP PUBLIC KEY BLOCK----- diff --git a/registry/response/terraform_provider.go b/registry/response/terraform_provider.go index b9d78e329..0a4c3f9ef 100644 --- a/registry/response/terraform_provider.go +++ b/registry/response/terraform_provider.go @@ -2,6 +2,7 @@ package response import ( "sort" + "strings" version "github.com/hashicorp/go-version" ) @@ -50,11 +51,38 @@ type TerraformProviderPlatformLocation struct { DownloadURL string `json:"download_url"` ShasumsURL string `json:"shasums_url"` ShasumsSignatureURL string `json:"shasums_signature_url"` + Shasum string `json:"shasum"` + + SigningKeys SigningKeyList `json:"signing_keys"` +} + +// SigningKeyList is the response structure for a list of signing keys. +type SigningKeyList struct { + GPGKeys []*GPGKey `json:"gpg_public_keys"` +} + +// GPGKey is the response structure for a GPG key. +type GPGKey struct { + ASCIIArmor string `json:"ascii_armor"` + Source string `json:"source"` + SourceURL *string `json:"source_url"` } // Collection type for TerraformProviderVersion type ProviderVersionCollection []*TerraformProviderVersion +// GPGASCIIArmor returns an ASCII-armor-formatted string for all of the gpg +// keys in the response. +func (signingKeys *SigningKeyList) GPGASCIIArmor() string { + keys := []string{} + + for _, gpgKey := range signingKeys.GPGKeys { + keys = append(keys, gpgKey.ASCIIArmor) + } + + return strings.Join(keys, "\n") +} + // Sort sorts versions from newest to oldest. func (v ProviderVersionCollection) Sort() { sort.Slice(v, func(i, j int) bool { diff --git a/registry/response/terraform_provider_test.go b/registry/response/terraform_provider_test.go new file mode 100644 index 000000000..09a976b95 --- /dev/null +++ b/registry/response/terraform_provider_test.go @@ -0,0 +1,52 @@ +package response + +import ( + "fmt" + "testing" +) + +var ( + testGPGKeyOne = &GPGKey{ + ASCIIArmor: "---\none\n---", + } + testGPGKeyTwo = &GPGKey{ + ASCIIArmor: "---\ntwo\n---", + } +) + +func TestSigningKeyList_GPGASCIIArmor(t *testing.T) { + var tests = []struct { + name string + gpgKeys []*GPGKey + expected string + }{ + { + name: "no keys", + gpgKeys: []*GPGKey{}, + expected: "", + }, + { + name: "one key", + gpgKeys: []*GPGKey{testGPGKeyOne}, + expected: testGPGKeyOne.ASCIIArmor, + }, + { + name: "two keys", + gpgKeys: []*GPGKey{testGPGKeyOne, testGPGKeyTwo}, + expected: fmt.Sprintf("%s\n%s", + testGPGKeyOne.ASCIIArmor, testGPGKeyTwo.ASCIIArmor), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + signingKeys := &SigningKeyList{ + GPGKeys: tt.gpgKeys, + } + actual := signingKeys.GPGASCIIArmor() + + if actual != tt.expected { + t.Errorf("expected %s, got %s", tt.expected, actual) + } + }) + } +} From 9a8a74b9bba63955db52e8a764627341e512e5b7 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Fri, 16 Nov 2018 14:05:38 -0500 Subject: [PATCH 2/4] plugin/discovery: Print name before verification This is so that any errors output from the checksum/signature verification show up in the expected place in the output. --- plugin/discovery/get.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index d26e8e346..558a277ee 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -172,6 +172,9 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e downloadURLs, err := i.listProviderDownloadURLs(provider, versionMeta.Version) providerURL := downloadURLs.DownloadURL + i.Ui.Info(fmt.Sprintf("- Downloading plugin for provider %q (%s)...", provider, versionMeta.Version)) + log.Printf("[DEBUG] getting provider %q version %q", provider, versionMeta.Version) + if !i.SkipVerify { sha256, err := i.getProviderChecksum(downloadURLs) if err != nil { @@ -184,8 +187,6 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e } } - i.Ui.Info(fmt.Sprintf("- Downloading plugin for provider %q (%s)...", provider, versionMeta.Version)) - log.Printf("[DEBUG] getting provider %q version %q", provider, versionMeta.Version) err = i.install(provider, v, providerURL) if err != nil { return PluginMeta{}, err From 06825bf46db55b8e5c8771742b07b084fe55e88f Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Fri, 16 Nov 2018 16:32:31 -0500 Subject: [PATCH 3/4] plugin/discovery: Add friendly gpg err msg When GPG verification fails, display a helpful message to the user instead of the generic openpgp error. --- plugin/discovery/get.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index 558a277ee..2f6ac1a91 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -27,6 +27,12 @@ import ( const protocolVersionHeader = "x-terraform-protocol-version" +const gpgVerificationError = `GPG signature verification error: +Terraform was unable to verify the GPG signature of the downloaded provider +files using the keys downloaded from the Terraform Registry. This may mean that +the publisher of the provider removed the key it was signed with, or that the +distributed files were changed after this version was released.` + var httpClient *http.Client var errVersionNotFound = errors.New("version not found") @@ -369,13 +375,14 @@ func (i *ProviderInstaller) getProviderChecksum(urls *response.TerraformProvider asciiArmor := urls.SigningKeys.GPGASCIIArmor() signer, err := verifySig(shasums, signature, asciiArmor) if err != nil { - return "", err + log.Printf("[ERROR] error verifying signature: %s", err) + return "", fmt.Errorf(gpgVerificationError) } // Display identity for GPG key which succeeded verifying the signature. // This could also be used to display to the user with i.Ui.Info(). identities := []string{} - for k, _ := range signer.Identities { + for k := range signer.Identities { identities = append(identities, k) } identity := strings.Join(identities, ", ") From c993e9bed690873b65809b40b0843b430051336c Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Mon, 19 Nov 2018 16:33:09 -0500 Subject: [PATCH 4/4] registry/response: Add protocols to DL resp --- registry/response/terraform_provider.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/registry/response/terraform_provider.go b/registry/response/terraform_provider.go index 0a4c3f9ef..08d382a48 100644 --- a/registry/response/terraform_provider.go +++ b/registry/response/terraform_provider.go @@ -45,13 +45,14 @@ type TerraformProviderPlatform struct { // structure for a provider platform with all details required to perform a // download. type TerraformProviderPlatformLocation struct { - OS string `json:"os"` - Arch string `json:"arch"` - Filename string `json:"filename"` - DownloadURL string `json:"download_url"` - ShasumsURL string `json:"shasums_url"` - ShasumsSignatureURL string `json:"shasums_signature_url"` - Shasum string `json:"shasum"` + Protocols []string `json:"protocols"` + OS string `json:"os"` + Arch string `json:"arch"` + Filename string `json:"filename"` + DownloadURL string `json:"download_url"` + ShasumsURL string `json:"shasums_url"` + ShasumsSignatureURL string `json:"shasums_signature_url"` + Shasum string `json:"shasum"` SigningKeys SigningKeyList `json:"signing_keys"` }