From 17c4c09170de61847c85320ce3393a29de74d105 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Thu, 7 Sep 2017 22:04:26 +0800 Subject: [PATCH] Fix bug with CSEK where the key stored in state might be associated with the wrong disk (#327) * Fix bug with CSEK where the key stored in state might be associated with the wrong disk * preserve original order of attached disks * use the disk index to figure out the raw key --- google/resource_compute_instance.go | 44 ++++--- google/resource_compute_instance_test.go | 141 ++++++++++++++++++++--- 2 files changed, 154 insertions(+), 31 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index c154120fbbb..e0674888c49 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -1,16 +1,19 @@ package google import ( + "crypto/sha256" + "encoding/base64" "fmt" "log" "strings" + "regexp" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" computeBeta "google.golang.org/api/compute/v0.beta" "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" - "regexp" ) var InstanceBaseApiVersion = v1 @@ -1083,16 +1086,15 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("Expected %d disks, API returned %d", expectedDisks, len(instance.Disks)) } - attachedDiskSources := make(map[string]struct{}, attachedDisksCount) + attachedDiskSources := make(map[string]int, attachedDisksCount) for i := 0; i < attachedDisksCount; i++ { - attachedDiskSources[d.Get(fmt.Sprintf("attached_disk.%d.source", i)).(string)] = struct{}{} + attachedDiskSources[d.Get(fmt.Sprintf("attached_disk.%d.source", i)).(string)] = i } dIndex := 0 - adIndex := 0 sIndex := 0 disks := make([]map[string]interface{}, 0, disksCount) - attachedDisks := make([]map[string]interface{}, 0, attachedDisksCount) + attachedDisks := make([]map[string]interface{}, attachedDisksCount) scratchDisks := make([]map[string]interface{}, 0, scratchDisksCount) for _, disk := range instance.Disks { if _, ok := d.GetOk("boot_disk"); ok && disk.Boot { @@ -1115,24 +1117,29 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error "device_name": d.Get(fmt.Sprintf("disk.%d.device_name", dIndex)), "disk_encryption_key_raw": d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)), } - if disk.DiskEncryptionKey != nil && disk.DiskEncryptionKey.Sha256 != "" { - di["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 + if d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)) != "" { + sha, err := hash256(d.Get(fmt.Sprintf("disk.%d.disk_encryption_key_raw", dIndex)).(string)) + if err != nil { + return err + } + di["disk_encryption_key_sha256"] = sha } disks = append(disks, di) dIndex++ } else { + adIndex := attachedDiskSources[disk.Source] di := map[string]interface{}{ - "source": disk.Source, - "device_name": disk.DeviceName, - "disk_encryption_key_raw": d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)), + "source": disk.Source, + "device_name": disk.DeviceName, } - if disk.DiskEncryptionKey != nil && disk.DiskEncryptionKey.Sha256 != "" { - di["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256 + if key := disk.DiskEncryptionKey; key != nil { + di["disk_encryption_key_raw"] = d.Get(fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", adIndex)) + di["disk_encryption_key_sha256"] = key.Sha256 } - attachedDisks = append(attachedDisks, di) - adIndex++ + attachedDisks[adIndex] = di } } + d.Set("disk", disks) d.Set("attached_disk", attachedDisks) d.Set("scratch_disk", scratchDisks) @@ -1614,6 +1621,15 @@ func getProjectFromSubnetworkLink(subnetwork string) string { return r.FindStringSubmatch(subnetwork)[1] } +func hash256(raw string) (string, error) { + decoded, err := base64.StdEncoding.DecodeString(raw) + if err != nil { + return "", err + } + h := sha256.Sum256(decoded) + return base64.StdEncoding.EncodeToString(h[:]), nil +} + func createAcceleratorPartialUrl(zone, accelerator string) string { return fmt.Sprintf("zones/%s/acceleratorTypes/%s", zone, accelerator) } diff --git a/google/resource_compute_instance_test.go b/google/resource_compute_instance_test.go index 88eea4404a0..c951ea5aa31 100644 --- a/google/resource_compute_instance_test.go +++ b/google/resource_compute_instance_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "regexp" + "strconv" "strings" "testing" @@ -225,7 +226,22 @@ func TestAccComputeInstance_deprecated_disksWithAutodelete(t *testing.T) { func TestAccComputeInstance_diskEncryption(t *testing.T) { var instance compute.Instance var instanceName = fmt.Sprintf("instance-test-%s", acctest.RandString(10)) - var diskName = fmt.Sprintf("instance-testd-%s", acctest.RandString(10)) + bootEncryptionKey := "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=" + bootEncryptionKeyHash := "esTuF7d4eatX4cnc4JsiEiaI+Rff78JgPhA/v1zxX9E=" + diskNameToEncryptionKey := map[string]*compute.CustomerEncryptionKey{ + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + RawKey: "Ym9vdDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + Sha256: "awJ7p57H+uVZ9axhJjl1D3lfC2MgA/wnt/z88Ltfvss=", + }, + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + RawKey: "c2Vjb25kNzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + Sha256: "7TpIwUdtCOJpq2m+3nt8GFgppu6a2Xsj1t0Gexk13Yc=", + }, + fmt.Sprintf("instance-testd-%s", acctest.RandString(10)): { + RawKey: "dGhpcmQ2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=", + Sha256: "b3pvaS7BjDbCKeLPPTx7yXBuQtxyMobCHN1QJR43xeM=", + }, + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -233,13 +249,11 @@ func TestAccComputeInstance_diskEncryption(t *testing.T) { CheckDestroy: testAccCheckComputeInstanceDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeInstance_disks_encryption(diskName, instanceName), + Config: testAccComputeInstance_disks_encryption(bootEncryptionKey, diskNameToEncryptionKey, instanceName), Check: resource.ComposeTestCheckFunc( testAccCheckComputeInstanceExists( "google_compute_instance.foobar", &instance), - testAccCheckComputeInstanceDisk(&instance, instanceName, true, true), - testAccCheckComputeInstanceDisk(&instance, diskName, true, false), - testAccCheckComputeInstanceDiskEncryptionKey("google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceDiskEncryptionKey("google_compute_instance.foobar", &instance, bootEncryptionKeyHash, diskNameToEncryptionKey), ), }, }, @@ -982,7 +996,7 @@ func testAccCheckComputeInstanceScratchDisk(instance *compute.Instance, interfac } } -func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.Instance) resource.TestCheckFunc { +func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.Instance, bootDiskEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -990,16 +1004,58 @@ func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.In } for i, disk := range instance.Disks { - attr := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)] - if attr == "" && disk.Boot { - attr = rs.Primary.Attributes["boot_disk.0.disk_encryption_key_sha256"] + if disk.Boot { + attr := rs.Primary.Attributes["boot_disk.0.disk_encryption_key_sha256"] + if attr == "" { + attr = rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)] + } + if attr != bootDiskEncryptionKey { + return fmt.Errorf("Boot disk has wrong encryption key in state.\nExpected: %s\nActual: %s", bootDiskEncryptionKey, attr) + } + if disk.DiskEncryptionKey == nil && attr != "" { + return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: ", i, attr) + } + if disk.DiskEncryptionKey != nil && attr != disk.DiskEncryptionKey.Sha256 { + return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: %+v", + i, attr, disk.DiskEncryptionKey.Sha256) + } + } else { + if disk.DiskEncryptionKey != nil { + sourceUrl := strings.Split(disk.Source, "/") + expectedKey := diskNameToEncryptionKey[sourceUrl[len(sourceUrl)-1]].Sha256 + if disk.DiskEncryptionKey.Sha256 != expectedKey { + return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, disk.DiskEncryptionKey.Sha256) + } + } } - if disk.DiskEncryptionKey == nil && attr != "" { - return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: ", i, attr) + } + + numDisks, err := strconv.Atoi(rs.Primary.Attributes["disk.#"]) + if err != nil { + return fmt.Errorf("Error converting value of disk.#") + } + for i := 0; i < numDisks; i++ { + diskName := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk", i)] + encryptionKey := rs.Primary.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)] + expectedEncryptionKey := diskNameToEncryptionKey[diskName].Sha256 + if encryptionKey != expectedEncryptionKey { + return fmt.Errorf("Disk %d has unexpected encryption key in state.\nExpected: %s\nActual: %s", i, expectedEncryptionKey, encryptionKey) } - if disk.DiskEncryptionKey != nil && attr != disk.DiskEncryptionKey.Sha256 { - return fmt.Errorf("Disk %d has mismatched encryption key.\nTF State: %+v\nGCP State: %+v", - i, attr, disk.DiskEncryptionKey.Sha256) + } + + numAttachedDisks, err := strconv.Atoi(rs.Primary.Attributes["attached_disk.#"]) + if err != nil { + return fmt.Errorf("Error converting value of attached_disk.#") + } + for i := 0; i < numAttachedDisks; i++ { + diskSourceUrl := strings.Split(rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.source", i)], "/") + diskName := diskSourceUrl[len(diskSourceUrl)-1] + encryptionKey := rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.disk_encryption_key_sha256", i)] + if key, ok := diskNameToEncryptionKey[diskName]; ok { + expectedEncryptionKey := key.Sha256 + if encryptionKey != expectedEncryptionKey { + return fmt.Errorf("Attached disk %d has unexpected encryption key in state.\nExpected: %s\nActual: %s", i, expectedEncryptionKey, encryptionKey) + } } } return nil @@ -1452,13 +1508,44 @@ resource "google_compute_instance" "foobar" { `, disk, instance, autodelete) } -func testAccComputeInstance_disks_encryption(disk, instance string) string { +func testAccComputeInstance_disks_encryption(bootEncryptionKey string, diskNameToEncryptionKey map[string]*compute.CustomerEncryptionKey, instance string) string { + diskNames := []string{} + for k, _ := range diskNameToEncryptionKey { + diskNames = append(diskNames, k) + } return fmt.Sprintf(` resource "google_compute_disk" "foobar" { name = "%s" size = 10 type = "pd-ssd" zone = "us-central1-a" + + disk_encryption_key_raw = "%s" +} + +resource "google_compute_disk" "foobar2" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key_raw = "%s" +} + +resource "google_compute_disk" "foobar3" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" + + disk_encryption_key_raw = "%s" +} + +resource "google_compute_disk" "foobar4" { + name = "%s" + size = 10 + type = "pd-ssd" + zone = "us-central1-a" } resource "google_compute_instance" "foobar" { @@ -1470,11 +1557,26 @@ resource "google_compute_instance" "foobar" { initialize_params{ image = "debian-8-jessie-v20160803" } - disk_encryption_key_raw = "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=" + disk_encryption_key_raw = "%s" } disk { disk = "${google_compute_disk.foobar.name}" + disk_encryption_key_raw = "%s" + } + + attached_disk { + source = "${google_compute_disk.foobar2.self_link}" + disk_encryption_key_raw = "%s" + } + + attached_disk { + source = "${google_compute_disk.foobar4.self_link}" + } + + attached_disk { + source = "${google_compute_disk.foobar3.self_link}" + disk_encryption_key_raw = "%s" } network_interface { @@ -1485,7 +1587,12 @@ resource "google_compute_instance" "foobar" { foo = "bar" } } -`, disk, instance) +`, diskNames[0], diskNameToEncryptionKey[diskNames[0]].RawKey, + diskNames[1], diskNameToEncryptionKey[diskNames[1]].RawKey, + diskNames[2], diskNameToEncryptionKey[diskNames[2]].RawKey, + "instance-testd-"+acctest.RandString(10), + instance, bootEncryptionKey, + diskNameToEncryptionKey[diskNames[0]].RawKey, diskNameToEncryptionKey[diskNames[1]].RawKey, diskNameToEncryptionKey[diskNames[2]].RawKey) } func testAccComputeInstance_attachedDisk(disk, instance string) string {