From 01eaee2a343e235e6494ff26c6bc6264190d73de Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 11 May 2023 10:15:37 -0400 Subject: [PATCH 1/5] builder: don't upload pubkey if private specified In the SDK prior to version 0.3.2, the public key for the SSH communicator was only set if it was specified in the configuration. This changed however, and now when a private key file is specified in a packer template, the corresponding public key is set during the preparation steps. This changes the login for the instance creation, as with the update, the public key should only be uploaded to the instance metadata if it was generated, not if it was configured from a file. Therefore, this commit adds a predicate to the creation logic for the metadata of the instance so that it doesn't upload this key to the instance metadata at creation. --- builder/googlecompute/step_create_instance.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index ad0b36e5..40eaf7e7 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -42,10 +42,7 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) } } - // Merge any existing ssh keys with our public key, unless there is no - // supplied public key. This is possible if a private_key_file was - // specified. - if sshPublicKey != "" { + if c.Comm.SSHPrivateKeyFile == "" && sshPublicKey != "" { sshMetaKey := "ssh-keys" sshPublicKey = strings.TrimSuffix(sshPublicKey, "\n") sshKeys := fmt.Sprintf("%s:%s %s", c.Comm.SSHUsername, sshPublicKey, c.Comm.SSHUsername) From 938689a6c44e12571701e61e7f467b3b492fe377 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 11 May 2023 10:31:41 -0400 Subject: [PATCH 2/5] os_login: remove redundant check on cleanup During the import_os_login_ssh_key step, we rely on both a state value and an extra condition to determine whether or not we should clean-up some keys from os-login. This is redundant, as the state key/value is only set when OSLogin is enabled in the configs, and some other conditions are true. Because of the redundancy, we can remove the extra check and only rely on the value being present in the state in order to continue clearing up the data from os-login. --- builder/googlecompute/step_import_os_login_ssh_key.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/builder/googlecompute/step_import_os_login_ssh_key.go b/builder/googlecompute/step_import_os_login_ssh_key.go index 98de2702..f5725300 100644 --- a/builder/googlecompute/step_import_os_login_ssh_key.go +++ b/builder/googlecompute/step_import_os_login_ssh_key.go @@ -122,14 +122,9 @@ func (s *StepImportOSLoginSSHKey) Run(ctx context.Context, state multistep.State // Cleanup the SSH Key that we added to the POSIX account func (s *StepImportOSLoginSSHKey) Cleanup(state multistep.StateBag) { - config := state.Get("config").(*Config) driver := state.Get("driver").(Driver) ui := state.Get("ui").(packersdk.Ui) - if !config.UseOSLogin { - return - } - fingerprint, ok := state.Get("ssh_key_public_sha256").(string) if !ok || fingerprint == "" { return From 6b8174a682ccf6bfcbd31babf2ba74db70808359 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 11 May 2023 10:34:02 -0400 Subject: [PATCH 3/5] os_login: don't upload key if private key is spec As for the instance metadata, we relied on the sshPublicKey existing in order to determine whether or not to upload it to os-login. This is not true anymore in the case someone specifies a private key file in their template since v0.3.2 of the SDK, as the public key is derived from the private key specified in the template, so we cannot rely on this not to upload the key. So, in addition to the other checks, we check that the private key file was not specified in the configurations, and if it is, we skip the step. --- .../googlecompute/step_import_os_login_ssh_key.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/builder/googlecompute/step_import_os_login_ssh_key.go b/builder/googlecompute/step_import_os_login_ssh_key.go index f5725300..18e17499 100644 --- a/builder/googlecompute/step_import_os_login_ssh_key.go +++ b/builder/googlecompute/step_import_os_login_ssh_key.go @@ -38,8 +38,16 @@ func (s *StepImportOSLoginSSHKey) Run(ctx context.Context, state multistep.State return multistep.ActionContinue } - // If no public key information is available chances are that a private key was provided - // or that the user is using a SSH agent for authentication. + // If the user specified a private key, the assumption is that the instance + // will already know the private key, and therefore doesn't need to be + // registered for OSLogin. + if config.Comm.SSHPrivateKeyFile != "" { + ui.Say("Private key file specified, won't import SSH key for OSLogin") + return multistep.ActionContinue + } + + // If no public key information is available chances are that the user + // is using a SSH agent for authentication. if config.Comm.SSHPublicKey == nil { ui.Say("No public SSH key found; skipping SSH public key import for OSLogin...") return multistep.ActionContinue From 957770ecb4ca9f3508c0688d5421b30e7fa2f66a Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 11 May 2023 10:36:36 -0400 Subject: [PATCH 4/5] builder: add os-login acceptance test with privkey --- builder/googlecompute/builder_acc_test.go | 76 +++++++++++++++++++ .../oslogin/default-token-and-pkey.pkr.hcl | 45 +++++++++++ 2 files changed, 121 insertions(+) create mode 100644 builder/googlecompute/testdata/oslogin/default-token-and-pkey.pkr.hcl diff --git a/builder/googlecompute/builder_acc_test.go b/builder/googlecompute/builder_acc_test.go index 3c48fcc1..2f345823 100644 --- a/builder/googlecompute/builder_acc_test.go +++ b/builder/googlecompute/builder_acc_test.go @@ -4,9 +4,15 @@ package googlecompute import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" "embed" + "encoding/pem" "fmt" + "os" "os/exec" + "strings" "testing" "github.com/hashicorp/packer-plugin-sdk/acctest" @@ -55,6 +61,76 @@ func TestAccBuilder_DefaultTokenSource(t *testing.T) { acctest.TestPlugin(t, testCase) } +// generateSSHPrivateKey generates a PEM encoded ssh private key file +// +// The file's deletion is the responsibility of the caller. +func generateSSHPrivateKey() (string, error) { + outFile := fmt.Sprintf("%s/temp_key", os.TempDir()) + + priv, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return "", fmt.Errorf("failed to generate SSH key: %s", err) + } + + x509key := x509.MarshalPKCS1PrivateKey(priv) + + pemKey := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509key, + }) + + err = os.WriteFile(outFile, pemKey, 0600) + if err != nil { + return "", fmt.Errorf("failed to write private key to %q: %s", outFile, err) + } + + return outFile, nil +} + +func TestAccBuilder_DefaultTokenSourceWithPrivateKey(t *testing.T) { + keyFile, err := generateSSHPrivateKey() + if err != nil { + t.Fatalf("failed to generate SSH private key: %s", err) + } + + defer os.Remove(keyFile) + + tmpl, err := testDataFs.ReadFile("testdata/oslogin/default-token-and-pkey.pkr.hcl") + if err != nil { + t.Fatalf("failed to read testdata file %s", err) + } + + testCase := &acctest.PluginTestCase{ + Name: "googlecompute-packer-default-ts", + Template: fmt.Sprintf(string(tmpl), keyFile), + Check: func(buildCommand *exec.Cmd, logfile string) error { + if buildCommand.ProcessState != nil { + if buildCommand.ProcessState.ExitCode() == 0 { + return fmt.Errorf("Packer build should have failed because of the unknown SSH key for the target instance, but succeeded. Logfile: %s", logfile) + } + } + + rawLogs, err := os.ReadFile(logfile) + if err != nil { + return fmt.Errorf("failed to read logfile %q: %s", logfile, err) + } + + logs := string(rawLogs) + + if !strings.Contains(logs, "Private key file specified, won't import SSH key for OSLogin") { + return fmt.Errorf("did not find message stating that a private key file was specified") + } + + if strings.Contains(logs, "Deleting SSH public key for OSLogin...") { + return fmt.Errorf("found a message about deleting OSLogin SSH public key, shouldn't have") + } + + return nil + }, + } + acctest.TestPlugin(t, testCase) +} + func TestAccBuilder_WrappedStartupScriptSuccess(t *testing.T) { tmpl, err := testDataFs.ReadFile("testdata/wrapped-startup-scripts/successful.pkr.hcl") if err != nil { diff --git a/builder/googlecompute/testdata/oslogin/default-token-and-pkey.pkr.hcl b/builder/googlecompute/testdata/oslogin/default-token-and-pkey.pkr.hcl new file mode 100644 index 00000000..d9196852 --- /dev/null +++ b/builder/googlecompute/testdata/oslogin/default-token-and-pkey.pkr.hcl @@ -0,0 +1,45 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +variable "project" { + type = string + default = env("GOOGLE_PROJECT_ID") +} + +variable "ssh_private_key" { + type = string + default = "" +} + +variable "ssh_username" { + type = string + default = "root" +} + +variable "zone" { + type = string + default = "us-central1-a" +} + +locals { timestamp = regex_replace(timestamp(), "[- TZ:]", "") } + +# No provided access_token or account_file should read contents of env GOOGLE_APPLICATION_CREDENTIALS +source "googlecompute" "autogenerated_1" { + image_name = "packer-oslogin-tester-${local.timestamp}" + project_id = var.project + source_image_family = "centos-7" + ssh_username = var.ssh_username + ssh_private_key_file = "%s" + ssh_timeout = "30s" + use_os_login = true + skip_create_image = true + zone = var.zone +} + +build { + sources = ["source.googlecompute.autogenerated_1"] + + provisioner "shell" { + inline = ["echo hello from the other side, username is $(whoami)"] + } +} From b1d69ab655d4e50486a16eb5c8b19bb2f0981bb1 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 12 May 2023 10:26:24 -0400 Subject: [PATCH 5/5] step_oslogin: update ssh priv key test The private key test for OS Login used to rely on a private key being defined, and not the public key. Since this is not a workflow that will happen in normal usage of the plugin, we update the test with a generated private key so we can use it for that test. --- .../googlecompute/step_import_os_login_ssh_key_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/builder/googlecompute/step_import_os_login_ssh_key_test.go b/builder/googlecompute/step_import_os_login_ssh_key_test.go index d08a66f7..46cc2003 100644 --- a/builder/googlecompute/step_import_os_login_ssh_key_test.go +++ b/builder/googlecompute/step_import_os_login_ssh_key_test.go @@ -7,6 +7,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "os" "testing" "github.com/hashicorp/packer-plugin-sdk/multistep" @@ -203,10 +204,15 @@ func TestStepImportOSLoginSSHKey_withPrivateSSHKey(t *testing.T) { step := new(StepImportOSLoginSSHKey) defer step.Cleanup(state) + pkey, err := generateSSHPrivateKey() + if err != nil { + t.Fatalf("failed to generate SSH key: %s", err) + } + defer os.Remove(pkey) + config := state.Get("config").(*Config) config.UseOSLogin = true - config.Comm.SSHPrivateKey = []byte{'k', 'e', 'y'} - config.Comm.SSHPublicKey = nil + config.Comm.SSHPrivateKeyFile = pkey if action := step.Run(context.Background(), state); action != multistep.ActionContinue { t.Fatalf("bad action: %#v", action)