Additional bootstrap work: winrm support, data callbacks

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

3 files changed, 202 insertions, 152 deletions

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 923ed019d5..2a632c58df 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -43,10 +43,6 @@ class Chef
         require "chef_core/text" # i18n and standardized error structures
         require "chef_core/target_host"
         require "chef_core/target_resolver"
-
-        # Because nothing else is using i18n out of Chef::Text yet, we're treating it
-        # as a dependency to avoid loading localization files before we need them.
-        ChefCore::Text.add_gem_localization("chef")
       end
 
       banner "knife bootstrap [PROTOCOL://][USER@]FQDN (options)"
@@ -170,6 +166,8 @@ class Chef
 
         $stdout.sync = true
 
+        bootstrap_path = nil
+
         # chef-vault integration must use the new client-side hawtness, otherwise to use the
         # new client-side hawtness, just delete your validation key.
         if chef_vault_handler.doing_chef_vault? ||
@@ -190,51 +188,72 @@ class Chef
           ui.info("")
         end
 
-        ui.info("Connecting to #{ui.color(server_name, :bold)}")
+        connect!
 
-        begin
-          # Resolve the given host name to a TargetHost instance. We will limit
-          # the number of hosts to 1 (effectivly eliminating wildcard support) since
-          # we only support running bootstrap against one host at a time.
-          resolver = ChefCore::TargetResolver.new(host_descriptor, config[:protocol] || "ssh",
-                                                  connection_opts, max_expanded_targets: 1)
-          @target_host = resolver.targets.first
-          # rescue: TargetResolverError
-          target_host.connect!
-          # Now that we have a connected target_host, we can use (by referencing it...)
-          # "bootstrap_context".
-          unless client_builder.client_path.nil?
-            bootstrap_context.client_pem = client_builder.client_path
-          end
-          bootstrap_path = render_and_upload_bootstrap
-          r = target_host.run_command(bootstrap_command(bootstrap_path))
-          if r.exit_status != 0
-            ui.error("The following error occurred on #{server_name}:")
-            ui.error(r.stderr)
-            exit 1
-          end
+        # Now that we have a connected target_host, we can use (by referencing it...)
+        # "bootstrap_context".
+        unless client_builder.client_path.nil?
+          bootstrap_context.client_pem = client_builder.client_path
+        end
+
+        bootstrap_path = render_and_upload_bootstrap
+        ui.info("Bootstrapping #{ui.color(server_name, :bold)}")
+        r = target_host.run_command(bootstrap_command(bootstrap_path)) do |data|
+          ui.msg("#{ui.color(" [#{target_host.hostname}]", :cyan)} #{data}")
+        end
+        if r.exit_status != 0
+          ui.error("The following error occurred on #{server_name}:")
+          ui.error(r.stderr)
+          exit 1
+        end
+      ensure
+        target_host.del_file(bootstrap_path) if target_host && bootstrap_path
+      end
 
-          # TODO mp 2019-02-22 this *should* be the same behavior under train without
-          # forcing the behavior here, but we need to verify that.
-          #
-          # rescue Net::SSH::AuthenticationFailed
-          #   if config[:password]
-          #     raise
-          #   else
-          #     ui.info("Failed to authenticate #{knife_ssh.config[:user]} - trying password auth")
-          #     knife_ssh_with_password_auth.run
-          # def knife_ssh_with_password_auth
-          #   # prompt for a password then return a knife ssh object with that password set
-          #   # and with ssh_identity_file set to nil
-          #   ssh = knife_ssh
-          #   ssh.config[:ssh_identity_file] = nil
-          #   ssh.config[:password] = ssh.get_password
-          #   ssh
-          # end
-          #   end
+      def connect!
+        ui.info("Connecting to #{ui.color(server_name, :bold)}")
+        opts = connection_opts.dup
+        do_connect(opts) # rescue: TargetResolverError
+      rescue => e
+        # Ugh. TODO 1: Train raises a Train::Transports::SSHFailed for a number of different errors. chef_core makes that
+        # a more general ConnectionFailed, with an error code based on the specific error text/reason provided from trainm.
+        # This means we have to look three layers intot he exception to find out what actually happened instead of just
+        # looking at the exception type
+        #
+        # It doesn't help to provide our own error if it does't let the caller know what they need to identify the problem.
+        # Let's update chef_core to be a bit smarter about resolving the errors to an appropriate exception type
+        # (eg ChefCore::ConnectionFailed::AuthError or similar) that will work across protocols, instead of just a single
+        # ConnectionFailure type
+        #
+        # # TODO 2 - it is possible for train to automatically do the reprompt for password
+        #            but that will take a little digging through the train ssh protocol layer.
+        if e.cause && e.cause.cause && e.cause.cause.class == Net::SSH::AuthenticationFailed
+          if opts[:password]
+            raise
+          else
+            ui.warn("Failed to authenticate #{target_host.user} - trying password auth")
+            password = ui.ask("Enter password for #{target_host.user}@#{target_host.hostname}: ") do |q|
+              q.echo = false
+            end
+            update_connection_opts_for_forced_password(opts, password)
+            do_connect(opts)
+          end
+        else
+          raise
         end
       end
 
+      def do_connect(conn_options)
+        # Resolve the given host name to a TargetHost instance. We will limit
+        # the number of hosts to 1 (effectivly eliminating wildcard support) since
+        # we only support running bootstrap against one host at a time.
+        resolver = ChefCore::TargetResolver.new(host_descriptor, config[:protocol] || "ssh",
+                                                conn_options, max_expanded_targets: 1)
+        @target_host = resolver.targets.first
+        @target_host.connect!
+        @target_host
+      end
+
       # fail if the server_name is nil
       def validate_name_args!
         if server_name.nil?
@@ -261,36 +280,50 @@ class Chef
         true
       end
 
-      def connection_protocol
-
-      end
-
-
       # Createa configuration object based on setup a Chef::Knife::Ssh object using the passed config options
+      # Includes connection information for both supported protocols at this time - unused config is ignored.
       #
-      # @return a configuration hash suitable for connecting to the remote host.
+      # @return a configuration hash suitable for connecting to the remote host via TargetHost.
       def connection_opts
-        # Mapping of our options ot train options - they're pretty similar with removal of
-        # the ssh- prefix, but there's more to corre2ct
-        # TODO - is now the time to change flag names for consistency?
+        # Mapping of our options to TargetHost/train options - they're pretty similar with removal of
+        # the ssh- prefix, but there's more to correct
         opts = {
           port: config[:port], # Default if it's not in the connection string
           user: config[:user], #  "
           password: config[:password], # TODO - check if we need to exclude if not set, diff behavior for nil?
-          key_files: config[:ssh_identity_file],
+          forward_agent: config[:forward_agent] || false ,
           logger:  Chef::Log,
+          key_files: [],
           # WinRM options - they will be ignored for ssh
           # TODO train will throw if this is not valid, should be OK as-is
           winrm_transport: config[:winrm_transport],
-          self_signed: config[:winrm_self_signed_cert],
-          ssl: config[:winrm_ssl]
+          self_signed: config[:winrm_no_verify_cert] === true,
+          winrm_basic_auth_only: config[:winrm_basic_auth_only],
+          ssl: config[:winrm_ssl],
+          ssl_peer_fingerprint: config[:winrm_ssl_peer_fingerprint]
+
+          # NOTE: 'ssl' true is different from using the ssl auth protocol which supoorts
+          #       using client cert+key (though we dongtgt
         }
 
+        if opts[:ssh_identity_file]
+          opts[:keys_only] = true
+          opts[:key_files] << config[:ssh_identity_file]
+        end
+
+        if config[:ssh_gateway]
+          gw_host, gw_user = config[:ssh_gateway].split("@").reverse
+          gw_host, gw_port = gw_host.split(":")
+          opts[:bastion_host] = gw_host
+          opts[:bastion_port] = gw_port
+          opts[:bastion_user] = gw_user
+          if config[:ssh_gatway_identity]
+            opts[:key_files] << config[:ssh_gateway_identity]
+          end
+        end
+
         if config[:use_sudo]
           opts[:sudo] = true
-          # TODO this preserves original logic - we're using the provided password for sudo
-          # if sudo is enabled.  Note that train supports a separate sudo password.
-          # TODO - check original, what if password was not given? Where do we validate?
           if opts[:use_sudo_password]
             opts[:sudo_password] = config[:password]
           end
@@ -299,38 +332,34 @@ class Chef
           end
         end
 
+        # REVIEWERS - maybe we combine this and winrm_no_verify_cert flags into "--no-verify-target"?
+        opts[:host_key_verify] = config[:host_key_verify].nil? ? true : config[:host_key_verify]
+
         if config[:password]
           opts[:password] = config[:password]
         end
 
-        if config[:ssh_identity_file]
-          # TODO - to get the matching original knife bootstrap fallback behavior of prompting for password
-          # when we don't provide it, I think we'll want  to _not_ do this here - we should get automatic
-          # keyboard-interactive auth if we don't set this and key fails.
-          opts[:keys_only] = true
+
+        opts[:winrm_transport] = config[:winrm_auth_method]
+        if config[:winrm_auth_method] == "kerberos"
+          opts[:kerberos_service] = config[:kerberos_service]
+          opts[:kerberos_realm] = config[:kerberos_realm]
         end
-        # ssh.config[:ssh_gateway_identity] = config[:ssh_gateway_identity]
-        # ssh.config[:forward_agent] = config[:forward_agent]
-        # ssh.config[:ssh_identity_file] = config[:ssh_identity_file]
-        # ssh.config[:manual] = true
-        # TODO train appears to false this to always false.  We'll need to make it an option.
-        # ssh.config[:host_key_verify] = config[:host_key_verify]
-        # ssh.config[:on_error] = true
-        # TODO: proxy command
-        # TODO - ssh_identity_file and ssh_gateway_identity appear to be implemented
-        #        as mutually exclsuive in knife ssh. Is there a valid case for two keys?
-        #        If so, train should accept more than one.
-        # key_files << config[:ssh_identity_file]
-        # TODO _ we're forcing knife ssh :on_error to true which will cause immediate exit on problem.
-        #        Need to see what that means, and if we have to implement anything in train to support it.
-        # option :on_error,
-        # short: "-e",
-        # long: "--exit-on-error",
-        # description: "Immediately exit if an error is encountered.",
+
+        opts[:ca_trust_path] = config[:ca_trust_path]
+
+        opts[:winrm_basic_auth_only] = config[:winrm_basic_auth_only] if config[:winrm_basic_auth_only]
         opts
       end
 
 
+      def update_connection_opts_for_forced_password(opts, password)
+        opts[:password] = password
+        opts[:non_interactive] = false
+        opts[:keys_only] = false
+        opts[:key_files] = nil
+        opts[:auth_methods] = [:password, :keyboard_interactive]
+      end
 
       def render_and_upload_bootstrap
         content = render_template
diff --git a/lib/chef/knife/bootstrap/options.rb b/lib/chef/knife/bootstrap/options.rb
index eeba60c9c5..37f19fef94 100644
--- a/lib/chef/knife/bootstrap/options.rb
+++ b/lib/chef/knife/bootstrap/options.rb
@@ -19,19 +19,27 @@ class Chef
   class Knife
     class Bootstrap
       module Options
+        # REVIEWER: let's talk about which protocols we want to support.
+        # TODO this should be available from train, which we can make our source of truth.
+        # TODO - we don't actually validate that the protocol is valid...
+        WINRM_AUTH_PROTOCOL_LIST = %w{plaintext kerberos ssl negotiate}
+
         def self.included(includer)
           includer.class_eval do
             # Common connectivity options
+            # TODO - renamed --ssh-user -> --ssh-password
             option :user,  # TODO - deprecate ssh_user which this replaces
               short: "-u USERNAME",
               long: "--user USERNAME",
               description: "The remote user to connect as"
 
+            # TODO - renamed --ssh-password -> --password
             option :password, # TODO - deprecate ssh_password
               short: "-P PASSWORD",
               long: "--ssh-password PASSWORD",
               description: "The password of the remote user."
 
+            # TODO - renamed --ssh-port -> --port
             option :port,
               short: "-p PORT",
               long: "--ssh-port PORT",
@@ -42,7 +50,8 @@ class Chef
             option :protocol,
               short: "-o PROTOCOL",
               long: "--protocol PROTOCOL",
-              description: "The protocol to use to connect to the target node.  Supports ssh and winrm."
+              description: "The protocol to use to connect to the target node.  Supports ssh and winrm.",
+              default: 'ssh'
 
             # TODO SSH  train gives bastion_host which seeems to map to getway/gateway_identity -
             # though not exactly.
@@ -64,9 +73,10 @@ class Chef
               proc: Proc.new { |key| Chef::Config[:knife][:ssh_gateway_identity] = key }
 
             # SSH train ssh: options[:forward_agent]
-            option :forward_agent,
+            # TODO: renamed to ssh_forward_agent from forward_agent for consistency.
+            option :ssh_forward_agent,
               short: "-A",
-              long: "--forward-agent",
+              long: "--ssh-forward-agent",
               description: "Enable SSH agent forwarding",
               boolean: true
 
@@ -77,8 +87,8 @@ class Chef
               description: "The SSH identity file used for authentication"
 
             # ssh options - train options[:verify_host_key]
-            option :host_key_verify,
-              long: "--[no-]host-key-verify",
+            option :ssh_verify_host_key,
+              long: "--ssh-[no-]verify-host-key",
               description: "Verify host key, enabled by default.",
               boolean: true,
               default: true
@@ -218,7 +228,6 @@ class Chef
                 }
 
                 # bootstrap override: url of a an installer shell script touse in place of omnitruck
-                # TODO - this replaces --msi-url out of knife windows bootstrap
                 option :bootstrap_url,
                   long: "--bootstrap-url URL",
                   description: "URL to a custom installation script",
@@ -271,88 +280,100 @@ class Chef
                     Chef::Config[:knife][:bootstrap_vault_item]
                   }
 
-                  # TODO
                   # Windows only
+
+
+                  # bootstrap template
                   option :install_as_service,
                     :long => "--install-as-service",
                     :description => "Install chef-client as a Windows service. (Windows only)",
                     :default => false
 
-                  # Windows only
-                  option :winrm_self_signed_cert,
-                    long: "--winrm-self-signed-cert",
-                    :description => "Expect a self-signed certificate when transport is 'ssl'. Defaults to false.",
-                    :default => false
+                  option :msi_url,
+                    :short => "-u URL",
+                    :long => "--msi-url URL",
+                    :description => "Location of the Chef Client MSI. The default templates will prefer to download from this location. The MSI will be downloaded from chef.io if not provided (windows).",
+                    :default => ''
+
+                  # TODO - may not need, current method works in powershell
+                  # option :winrm_codepage,
+                  #   :long => "--winrm-codepage Codepage",
+                  #   :description => "The codepage to use for the winrm cmd shell",
+                  #   :default => 65001
+
+                  # TODO - bootstrap
+                  option :winrm_ssl_peer_fingerprint,
+                    :long => "--winrm-ssl-peer-fingerprint FINGERPRINT",
+                    :description => "ssl Cert Fingerprint to bypass normal cert chain checks"
+
+                  # NOTE:removed.  winrm_port -> port
+                  # option :winrm_port,
+
+                  # NOTE: removed; was in general options for winrm,
+                  # but bootstrap previously only supported :cmd;
+                  # under train it supports only :powershell
+
+                  # option :winrm_shell
+
+                  # TODO - need to understand when this is relevant. Was not exposed
+                  #        in knife windows, but is exposed in train.
+                  # option :winrm_basic_auth_only,
+                  #   long: "--winrm-basic-auth-only",
+                  #   description: "Force Basic authentication for WinRM",
+                  #   default: false
+
+                  option :ca_trust_path,
+                    :short => "-f CA_TRUST_PATH",
+                    :long => "--ca-trust-file CA_TRUST_PATH",
+                    :description => "The Certificate Authority (CA) trust file used for SSL transport"
+
+                  option :winrm_no_verify_cert,
+                    long: "--winrm-no-verify-cert",
+                    description: "Do not verify the SSL certificate of the target node for WinRM. Defaults to true.",
+                    default: false
 
-                  option :winrm_transport,
-                    long: "--winrm-transport TRANSPORT",
-                    :description => "Specify WinRM transport. Supported values are ssl, plaintext, Defaults to 'negotiate'.",
-                    :default => "negotiate"
 
                   option :winrm_ssl,
                     long: "--winrm-ssl",
-                    :description => "Connect to WinRM over HTTPS. Defaults to false",
-                    :default => false
+                    description: "Connect to WinRM using SSL",
+                    boolean: true
 
-                  option :winrm_codepage,
-                    :long => "--winrm-codepage Codepage",
-                    :description => "The codepage to use for the winrm cmd shell",
-                    :default => 65001
-                  # TODO - bootstrap - compat in train?
-                  option :ssl_peer_fingerprint,
-                    :long => "--ssl-peer-fingerprint FINGERPRINT",
-                    :description => "ssl Cert Fingerprint to bypass normal cert chain checks"
+                  option :winrm_auth_method,
+                    :short => "-w AUTH-METHOD",
+                    :long => "--winrm-auth-method AUTH-METHOD",
+                    :description => "The WinRM authentication method to use. Valid choices are #{WINRM_AUTH_PROTOCOL_LIST}",
+                    :default => "negotiate"
+
+                  option :winrm_basic_auth_only,
+                    long: "--winrm-basic-auth-only",
+                    description: "For WinRM basic authentication when using the 'ssl' auth method",
+                    default: false,
+                    boolean: true
 
-                  option :winrm_port,
-                    :short => "-p PORT",
-                    :long => "--winrm-port PORT",
-                    :description => "The WinRM port, by default this is '5985' for 'plaintext' and '5986' for 'ssl' winrm transport",
-                    :default => '5985',
-                    :proc => Proc.new { |key| Chef::Config[:knife][:winrm_port] = key }
-
-                  option :winrm_shell, # bootstrap only works with cmd
-                    :long => "--winrm-shell SHELL",
-                    :description => "The WinRM shell type. Valid choices are [cmd, powershell, elevated]. 'elevated' runs powershell in a scheduled task",
-                    :default => :cmd,
-                    :proc => Proc.new { |shell| shell.to_sym }
-
-                  option :ca_trust_file,
-                    :short => "-f CA_TRUST_FILE",
-                    :long => "--ca-trust-file CA_TRUST_FILE",
-                    :description => "The Certificate Authority (CA) trust file used for SSL transport",
-                    :proc => Proc.new { |trust| Chef::Config[:knife][:ca_trust_file] = trust }
-
-                  option :winrm_authentication_protocol,
-                    :long => "--winrm-authentication-protocol AUTHENTICATION_PROTOCOL",
-                    :description => "The authentication protocol used during WinRM communication. The supported protocols are #{WINRM_AUTH_PROTOCOL_LIST.join(',')}. Default is 'negotiate'.",
-                    :default => "negotiate",
-                    :proc => Proc.new { |protocol| Chef::Config[:knife][:winrm_authentication_protocol] = protocol }
-                  option :winrm-ssl,
-                    winrm_transport,
-                    :short => "-w TRANSPORT",
-                    :long => "--winrm-transport TRANSPORT",
-                    :description => "The WinRM transport type. Valid choices are [ssl, plaintext]",
-                    :default => 'plaintext',
-                    :proc => Proc.new { |transport| Chef::Config[:knife][:winrm_port] = '5986' if transport == 'ssl'
-                                        Chef::Config[:knife][:winrm_transport] = transport }
-
-                  option :kerberos_keytab_file,
-                    :short => "-T KEYTAB_FILE",
-                    :long => "--keytab-file KEYTAB_FILE",
-                    :description => "The Kerberos keytab file used for authentication",
-                    :proc => Proc.new { |keytab| Chef::Config[:knife][:kerberos_keytab_file] = keytab }
+                  # This option was provided in knife bootstrap windows winrm,
+                  # but it is ignored  in knife-windows/WinrmSession.
+                  # option :kerberos_keytab_file,
+                  #   :short => "-T KEYTAB_FILE",
+                  #   :long => "--keytab-file KEYTAB_FILE",
+                  #   :description => "The Kerberos keytab file used for authentication",
+                  #   :proc => Proc.new { |keytab| Chef::Config[:knife][:kerberos_keytab_file] = keytab }
 
                   option :kerberos_realm,
                     :short => "-R KERBEROS_REALM",
                     :long => "--kerberos-realm KERBEROS_REALM",
-                    :description => "The Kerberos realm used for authentication",
-                    :proc => Proc.new { |realm| Chef::Config[:knife][:kerberos_realm] = realm }
+                    :description => "The Kerberos realm used for authentication"
 
                   option :kerberos_service,
                     :short => "-S KERBEROS_SERVICE",
                     :long => "--kerberos-service KERBEROS_SERVICE",
-                    :description => "The Kerberos service used for authentication",
-                    :proc => Proc.new { |service| Chef::Config[:knife][:kerberos_service] = service }
+                    :description => "The Kerberos service used for authentication"
+
+                    # TODO
+                  option :session_timeout,
+                    :long => "--session-timeout Minutes",
+                    :description => "The timeout for the client for the maximum length of the WinRM session",
+                    :default => 30
+
           end
         end
       end
diff --git a/lib/chef/knife/core/windows_bootstrap_context.rb b/lib/chef/knife/core/windows_bootstrap_context.rb
index 61dde28ccf..6db017ca2f 100644
--- a/lib/chef/knife/core/windows_bootstrap_context.rb
+++ b/lib/chef/knife/core/windows_bootstrap_context.rb
@@ -78,7 +78,7 @@ class Chef
           if @chef_config[:config_log_level]
             client_rb << %Q{log_level :#{@chef_config[:config_log_level]}\n}
           else
-            client_rb << "log_level        :info\n"
+            client_rb << "log_level        :auto\n"
           end
 
           client_rb << "log_location       #{get_log_location}"
@@ -300,7 +300,7 @@ class Chef
           # The default msi path has a number of url query parameters - we attempt to substitute
           # such parameters in as long as they are provided by the template.
 
-          if @config[:msi_url].nil? || @config[:msi_url].empty?
+          if @config[:install].nil? || @config[:msi_url].empty?
             url = "https://www.chef.io/chef/download?p=windows"
             url += "&pv=#{machine_os}" unless machine_os.nil?
             url += "&m=#{machine_arch}" unless machine_arch.nil?