From a23610ef4164fa3314a614fe50688ee7bf1243be Mon Sep 17 00:00:00 2001 From: Billie Cleek Date: Sat, 6 Feb 2016 18:09:15 -0800 Subject: [PATCH 1/3] cleanup ansible provisioner key generation * Clearly separate host signer and user key generation into separate functions and data structures. * Remove inaccurate comment about needing to specify both files if either one is specified. * Rename parameters for clarity according to their meaning to the callee. * Style the code with gofmt. --- provisioner/ansible/provisioner.go | 206 +++++++++--------- .../docs/provisioners/ansible.html.markdown | 29 ++- 2 files changed, 117 insertions(+), 118 deletions(-) diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index 04efe52b4..74824483a 100644 --- a/provisioner/ansible/provisioner.go +++ b/provisioner/ansible/provisioner.go @@ -78,8 +78,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { errs = packer.MultiErrorAppend(errs, err) } - // Check that the authorized key file exists ( this should really be called the public key ) - // Check for either file ( if you specify either file you must specify both files ) + // Check that the authorized key file exists if len(p.config.SSHAuthorizedKeyFile) > 0 { err = validateFileConfig(p.config.SSHAuthorizedKeyFile, "ssh_authorized_key_file", true) if err != nil { @@ -109,107 +108,18 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { return nil } -type Keys struct { - //! This is the public key that we will allow to authenticate - public ssh.PublicKey - - //! This is the name of the file of the private key that coorlates - //the the public key - filename string - - //! This is the servers private key ( Set in case the public key - //is autogenerated ) - private ssh.Signer - - //! This is the flag to say the server key was generated - generated bool -} - func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { ui.Say("Provisioning with Ansible...") - keyFactory := func(pubKeyFile string, privKeyFile string) (*Keys, error) { - var public ssh.PublicKey - var private ssh.Signer - var filename string = "" - var generated bool = false - - if len(pubKeyFile) > 0 { - pubKeyBytes, err := ioutil.ReadFile(pubKeyFile) - if err != nil { - return nil, errors.New("Failed to read public key") - } - public, _, _, _, err = ssh.ParseAuthorizedKey(pubKeyBytes) - if err != nil { - return nil, errors.New("Failed to parse authorized key") - } - } else { - key, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - return nil, errors.New("Failed to generate key pair") - } - public, err = ssh.NewPublicKey(key.Public()) - if err != nil { - return nil, errors.New("Failed to extract public key from generated key pair") - } - - // To support Ansible calling back to us we need to write - // this file down - privateKeyDer := x509.MarshalPKCS1PrivateKey(key) - privateKeyBlock := pem.Block{ - Type: "RSA PRIVATE KEY", - Headers: nil, - Bytes: privateKeyDer, - } - tf, err := ioutil.TempFile("", "ansible-key") - if err != nil { - return nil, errors.New("failed to create temp file for generated key") - } - _, err = tf.Write(pem.EncodeToMemory(&privateKeyBlock)) - if err != nil { - return nil, errors.New("failed to write private key to temp file") - } - - err = tf.Close() - if err != nil { - return nil, errors.New("failed to close private key temp file") - } - filename = tf.Name() - } - - if len(privKeyFile) > 0 { - privateBytes, err := ioutil.ReadFile(privKeyFile) - if err != nil { - return nil, errors.New("Failed to load private host key") - } - - private, err = ssh.ParsePrivateKey(privateBytes) - if err != nil { - return nil, errors.New("Failed to parse private host key") - } - } else { - key, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - return nil, errors.New("Failed to generate server key pair") - } - - private, err = ssh.NewSignerFromKey(key) - if err != nil { - return nil, errors.New("Failed to extract private key from generated key pair") - } - generated = true - } - return &Keys{public, filename, private, generated}, nil - } - - k, err := keyFactory(p.config.SSHAuthorizedKeyFile, p.config.SSHHostKeyFile) + k, err := newUserKey(p.config.SSHAuthorizedKeyFile) if err != nil { return err } + hostSigner, err := newSigner(p.config.SSHHostKeyFile) // Remove the private key file - if len(k.filename) > 0 { - defer os.Remove(k.filename) + if len(k.privKeyFile) > 0 { + defer os.Remove(k.privKeyFile) } keyChecker := ssh.CertChecker{ @@ -219,7 +129,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { return nil, errors.New("authentication failed") } - if !bytes.Equal(k.public.Marshal(), pubKey.Marshal()) { + if !bytes.Equal(k.Marshal(), pubKey.Marshal()) { ui.Say("unauthorized key") return nil, errors.New("authentication failed") } @@ -236,7 +146,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { //NoClientAuth: true, } - config.AddHostKey(k.private) + config.AddHostKey(hostSigner) localListener, err := func() (net.Listener, error) { port, err := strconv.ParseUint(p.config.LocalPort, 10, 16) @@ -299,7 +209,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { }() } - if err := p.executeAnsible(ui, comm, k.filename, k.generated); err != nil { + if err := p.executeAnsible(ui, comm, k.privKeyFile, !hostSigner.generated); err != nil { return fmt.Errorf("Error executing Ansible: %s", err) } @@ -317,20 +227,19 @@ func (p *Provisioner) Cancel() { os.Exit(0) } -func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, authToken string, generated bool) error { +func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, privKeyFile string, checkHostKey bool) error { playbook, _ := filepath.Abs(p.config.PlaybookFile) inventory := p.config.inventoryFile args := []string{playbook, "-i", inventory} - if len(authToken) > 0 { - args = append(args, "--private-key", authToken) + if len(privKeyFile) > 0 { + args = append(args, "--private-key", privKeyFile) } args = append(args, p.config.ExtraArguments...) cmd := exec.Command(p.config.Command, args...) - // If we have autogenerated the key files turn off host key checking - if generated { + if !checkHostKey { cmd.Env = os.Environ() cmd.Env = append(cmd.Env, "ANSIBLE_HOST_KEY_CHECKING=False") } @@ -385,6 +294,97 @@ func validateFileConfig(name string, config string, req bool) error { return nil } +type userKey struct { + ssh.PublicKey + privKeyFile string +} + +func newUserKey(pubKeyFile string) (*userKey, error) { + userKey := new(userKey) + if len(pubKeyFile) > 0 { + pubKeyBytes, err := ioutil.ReadFile(pubKeyFile) + if err != nil { + return nil, errors.New("Failed to read public key") + } + userKey.PublicKey, _, _, _, err = ssh.ParseAuthorizedKey(pubKeyBytes) + if err != nil { + return nil, errors.New("Failed to parse authorized key") + } + + return userKey, nil + } + + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, errors.New("Failed to generate key pair") + } + userKey.PublicKey, err = ssh.NewPublicKey(key.Public()) + if err != nil { + return nil, errors.New("Failed to extract public key from generated key pair") + } + + // To support Ansible calling back to us we need to write + // this file down + privateKeyDer := x509.MarshalPKCS1PrivateKey(key) + privateKeyBlock := pem.Block{ + Type: "RSA PRIVATE KEY", + Headers: nil, + Bytes: privateKeyDer, + } + tf, err := ioutil.TempFile("", "ansible-key") + if err != nil { + return nil, errors.New("failed to create temp file for generated key") + } + _, err = tf.Write(pem.EncodeToMemory(&privateKeyBlock)) + if err != nil { + return nil, errors.New("failed to write private key to temp file") + } + + err = tf.Close() + if err != nil { + return nil, errors.New("failed to close private key temp file") + } + userKey.privKeyFile = tf.Name() + + return userKey, nil +} + +type signer struct { + ssh.Signer + generated bool +} + +func newSigner(privKeyFile string) (*signer, error) { + signer := new(signer) + + if len(privKeyFile) > 0 { + privateBytes, err := ioutil.ReadFile(privKeyFile) + if err != nil { + return nil, errors.New("Failed to load private host key") + } + + signer.Signer, err = ssh.ParsePrivateKey(privateBytes) + if err != nil { + return nil, errors.New("Failed to parse private host key") + } + + return signer, nil + } + + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, errors.New("Failed to generate server key pair") + } + + signer.Signer, err = ssh.NewSignerFromKey(key) + if err != nil { + return nil, errors.New("Failed to extract private key from generated key pair") + } + signer.generated = true + + return signer, nil +} + // Ui provides concurrency-safe access to packer.Ui. type Ui struct { sem chan int diff --git a/website/source/docs/provisioners/ansible.html.markdown b/website/source/docs/provisioners/ansible.html.markdown index 9a8a54444..76d744659 100644 --- a/website/source/docs/provisioners/ansible.html.markdown +++ b/website/source/docs/provisioners/ansible.html.markdown @@ -47,18 +47,17 @@ Required Parameters: Optional Parameters: -- `ssh_host_key_file` (string) - The SSH key that will be used to run - the SSH server on the host machine to forward commands to the target - machine. Ansible connects to this server and will validate the - identity of the server using the system known_hosts. The default behaviour is - to generate and use a one time key, and disable - host_key_verification in ansible to allow it to connect to the - server - -- `ssh_authorized_key_file` (string) - The SSH public key of the - Ansible `ssh_user`. The default behaviour is to generate and use a - one time key. If this file is generated the coorisponding private - key will be passed via the `--private-key` option to Ansible. +- `ssh_host_key_file` (string) - The SSH key that will be used to run the SSH + server on the host machine to forward commands to the target machine. Ansible + connects to this server and will validate the identity of the server using + the system known_hosts. The default behaviour is to generate and use a one + time key, and disable host_key_verification in ansible to allow it to connect + to the server + +- `ssh_authorized_key_file` (string) - The SSH public key of the Ansible + `ssh_user`. The default behaviour is to generate and use a one time key. If + this file is generated the coorisponding private key will be passed via the + `--private-key` option to Ansible. - `local_port` (string) - The port on which to attempt to listen for SSH connections. This value is a starting point. The provisioner will attempt @@ -67,9 +66,9 @@ Optional Parameters: listen on a system-chosen port. -- `sftp_command` (string) - The command to run on the provisioned machine to handle the - SFTP protocol that Ansible will use to transfer files. The command should - read and write on stdin and stdout, respectively. Defaults to +- `sftp_command` (string) - The command to run on the provisioned machine to + handle the SFTP protocol that Ansible will use to transfer files. The command + should read and write on stdin and stdout, respectively. Defaults to `/usr/lib/sftp-server -e`. - `extra_arguments` (string) - Extra arguments to pass to Ansible From a5ab53bf5e1edb59b4f66daf6dcda15eda9ccb86 Mon Sep 17 00:00:00 2001 From: Billie Cleek Date: Sat, 6 Feb 2016 18:22:38 -0800 Subject: [PATCH 2/3] update ansible tests Update tests to account for changes made in #3149 --- provisioner/ansible/provisioner_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/provisioner/ansible/provisioner_test.go b/provisioner/ansible/provisioner_test.go index 80074ebde..6fac4b8af 100644 --- a/provisioner/ansible/provisioner_test.go +++ b/provisioner/ansible/provisioner_test.go @@ -157,12 +157,19 @@ func TestProvisionerPrepare_AuthorizedKeyFile(t *testing.T) { } defer os.Remove(playbook_file.Name()) + filename := make([]byte, 10) + n, err := io.ReadFull(rand.Reader, filename) + if n != len(filename) || err != nil { + t.Fatal("could not create random file name") + } + config["ssh_host_key_file"] = hostkey_file.Name() config["playbook_file"] = playbook_file.Name() + config["ssh_authorized_key_file"] = fmt.Sprintf("%x", filename) err = p.Prepare(config) if err == nil { - t.Fatal("should have error") + t.Errorf("should error if ssh_authorized_key_file does not exist") } publickey_file, err := ioutil.TempFile("", "publickey") @@ -174,7 +181,7 @@ func TestProvisionerPrepare_AuthorizedKeyFile(t *testing.T) { config["ssh_authorized_key_file"] = publickey_file.Name() err = p.Prepare(config) if err != nil { - t.Fatalf("err: %s", err) + t.Errorf("err: %s", err) } } From fbb32d2f15ab0d3f84a459fcb3d6e25b904d4d74 Mon Sep 17 00:00:00 2001 From: "Billie H. Cleek" Date: Wed, 10 Feb 2016 12:45:17 -0800 Subject: [PATCH 3/3] documentation edits Fix spelling and grammar mistakes in the ansible provisioner documentation. --- .../source/docs/provisioners/ansible.html.markdown | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/website/source/docs/provisioners/ansible.html.markdown b/website/source/docs/provisioners/ansible.html.markdown index 76d744659..5b8127508 100644 --- a/website/source/docs/provisioners/ansible.html.markdown +++ b/website/source/docs/provisioners/ansible.html.markdown @@ -50,13 +50,13 @@ Optional Parameters: - `ssh_host_key_file` (string) - The SSH key that will be used to run the SSH server on the host machine to forward commands to the target machine. Ansible connects to this server and will validate the identity of the server using - the system known_hosts. The default behaviour is to generate and use a one - time key, and disable host_key_verification in ansible to allow it to connect - to the server + the system known_hosts. The default behaviour is to generate and use a + onetime key, and disable host_key_verification in Ansible to allow it to + connect to the server. - `ssh_authorized_key_file` (string) - The SSH public key of the Ansible - `ssh_user`. The default behaviour is to generate and use a one time key. If - this file is generated the coorisponding private key will be passed via the + `ssh_user`. The default behaviour is to generate and use a onetime key. If + this file is generated, the corresponding private key will be passed via the `--private-key` option to Ansible. - `local_port` (string) - The port on which to attempt to listen for SSH @@ -71,7 +71,7 @@ Optional Parameters: should read and write on stdin and stdout, respectively. Defaults to `/usr/lib/sftp-server -e`. -- `extra_arguments` (string) - Extra arguments to pass to Ansible +- `extra_arguments` (string) - Extra arguments to pass to Ansible. ## Limitations