From b40f6e5fdae1113518e9c92f96dfcd364b646ff9 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 14:54:27 -0700 Subject: [PATCH 1/7] Add ed25519 insecure private key Add a new ed25519 insecure private key. Update the README with information about the two insecure keys. Retain the RSA key in the `vagrant` file and duplicate it into `vagrant.key.rsa`. Append the ed25519 public key to the `vagrant.pub` file. --- keys/README.md | 15 ++++++++++++++- keys/vagrant.key.ed25519 | 7 +++++++ keys/vagrant.key.rsa | 27 +++++++++++++++++++++++++++ keys/vagrant.pub | 1 + keys/vagrant.pub.ed25519 | 1 + keys/vagrant.pub.rsa | 1 + 6 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 keys/vagrant.key.ed25519 create mode 100644 keys/vagrant.key.rsa create mode 100644 keys/vagrant.pub.ed25519 create mode 100644 keys/vagrant.pub.rsa diff --git a/keys/README.md b/keys/README.md index c900682a050..de81d83bfc7 100644 --- a/keys/README.md +++ b/keys/README.md @@ -1,9 +1,22 @@ -# Insecure Keypair +# Insecure Keypairs These keys are the "insecure" public/private keypair we offer to [base box creators](https://www.vagrantup.com/docs/boxes/base.html) for use in their base boxes so that vagrant installations can automatically SSH into the boxes. +# Vagrant Keypairs + +There are currently two "insecure" public/private keypairs for +Vagrant. One keypair was generated using the older RSA algorithm +and the other keypair was generated using the more recent ED25519 +algorithm. + +The `vagrant.pub` file includes the public key for both keypairs. It +is important for box creators to include both keypairs as versions of +Vagrant prior to 2.3.8 will only use the RSA private key. + +# Custom Keys + If you're working with a team or company or with a custom box and you want more secure SSH, you should create your own keypair and configure the private key in the Vagrantfile with diff --git a/keys/vagrant.key.ed25519 b/keys/vagrant.key.ed25519 new file mode 100644 index 00000000000..73d365e7f47 --- /dev/null +++ b/keys/vagrant.key.ed25519 @@ -0,0 +1,7 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACDdWHcQaTZc8Q6nycsP0CqMNRfsLxvYVxqKosrHyTp+WAAAAJj2TBMT9kwT +EwAAAAtzc2gtZWQyNTUxOQAAACDdWHcQaTZc8Q6nycsP0CqMNRfsLxvYVxqKosrHyTp+WA +AAAEAveRHRHSCjIxbNKHDRzezD0U3R3UEEmS7R33fzvPQAD91YdxBpNlzxDqfJyw/QKow1 +F+wvG9hXGoqiysfJOn5YAAAAEHNwb3hAdmFncmFudC1kZXYBAgMEBQ== +-----END OPENSSH PRIVATE KEY----- diff --git a/keys/vagrant.key.rsa b/keys/vagrant.key.rsa new file mode 100644 index 00000000000..7d6a083909e --- /dev/null +++ b/keys/vagrant.key.rsa @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzI +w+niNltGEFHzD8+v1I2YJ6oXevct1YeS0o9HZyN1Q9qgCgzUFtdOKLv6IedplqoP +kcmF0aYet2PkEDo3MlTBckFXPITAMzF8dJSIFo9D8HfdOV0IAdx4O7PtixWKn5y2 +hMNG0zQPyUecp4pzC6kivAIhyfHilFR61RGL+GPXQ2MWZWFYbAGjyiYJnAmCP3NO +Td0jMZEnDkbUvxhMmBYSdETk1rRgm+R4LOzFUGaHqHDLKLX+FIPKcF96hrucXzcW +yLbIbEgE98OHlnVYCzRdK8jlqm8tehUc9c9WhQIBIwKCAQEA4iqWPJXtzZA68mKd +ELs4jJsdyky+ewdZeNds5tjcnHU5zUYE25K+ffJED9qUWICcLZDc81TGWjHyAqD1 +Bw7XpgUwFgeUJwUlzQurAv+/ySnxiwuaGJfhFM1CaQHzfXphgVml+fZUvnJUTvzf +TK2Lg6EdbUE9TarUlBf/xPfuEhMSlIE5keb/Zz3/LUlRg8yDqz5w+QWVJ4utnKnK +iqwZN0mwpwU7YSyJhlT4YV1F3n4YjLswM5wJs2oqm0jssQu/BT0tyEXNDYBLEF4A +sClaWuSJ2kjq7KhrrYXzagqhnSei9ODYFShJu8UWVec3Ihb5ZXlzO6vdNQ1J9Xsf +4m+2ywKBgQD6qFxx/Rv9CNN96l/4rb14HKirC2o/orApiHmHDsURs5rUKDx0f9iP +cXN7S1uePXuJRK/5hsubaOCx3Owd2u9gD6Oq0CsMkE4CUSiJcYrMANtx54cGH7Rk +EjFZxK8xAv1ldELEyxrFqkbE4BKd8QOt414qjvTGyAK+OLD3M2QdCQKBgQDtx8pN +CAxR7yhHbIWT1AH66+XWN8bXq7l3RO/ukeaci98JfkbkxURZhtxV/HHuvUhnPLdX +3TwygPBYZFNo4pzVEhzWoTtnEtrFueKxyc3+LjZpuo+mBlQ6ORtfgkr9gBVphXZG +YEzkCD3lVdl8L4cw9BVpKrJCs1c5taGjDgdInQKBgHm/fVvv96bJxc9x1tffXAcj +3OVdUN0UgXNCSaf/3A/phbeBQe9xS+3mpc4r6qvx+iy69mNBeNZ0xOitIjpjBo2+ +dBEjSBwLk5q5tJqHmy/jKMJL4n9ROlx93XS+njxgibTvU6Fp9w+NOFD/HvxB3Tcz +6+jJF85D5BNAG3DBMKBjAoGBAOAxZvgsKN+JuENXsST7F89Tck2iTcQIT8g5rwWC +P9Vt74yboe2kDT531w8+egz7nAmRBKNM751U/95P9t88EDacDI/Z2OwnuFQHCPDF +llYOUI+SpLJ6/vURRbHSnnn8a/XG+nzedGH5JGqEJNQsz+xT2axM0/W/CRknmGaJ +kda/AoGANWrLCz708y7VYgAtW2Uf1DPOIYMdvo6fxIB5i9ZfISgcJ/bbCUkFrhoH ++vq/5CIWxCPp0f85R4qxxQ5ihxJ0YDQT9Jpx4TMss4PSavPaBH3RXow5Ohe+bYoQ +NE5OgEXk2wVfZczCZpigBKbKZHNYcelXtTt/nP3rsCuGcM4h53s= +-----END RSA PRIVATE KEY----- diff --git a/keys/vagrant.pub b/keys/vagrant.pub index 18a9c00fd56..78a8ccf5188 100644 --- a/keys/vagrant.pub +++ b/keys/vagrant.pub @@ -1 +1,2 @@ ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzIw+niNltGEFHzD8+v1I2YJ6oXevct1YeS0o9HZyN1Q9qgCgzUFtdOKLv6IedplqoPkcmF0aYet2PkEDo3MlTBckFXPITAMzF8dJSIFo9D8HfdOV0IAdx4O7PtixWKn5y2hMNG0zQPyUecp4pzC6kivAIhyfHilFR61RGL+GPXQ2MWZWFYbAGjyiYJnAmCP3NOTd0jMZEnDkbUvxhMmBYSdETk1rRgm+R4LOzFUGaHqHDLKLX+FIPKcF96hrucXzcWyLbIbEgE98OHlnVYCzRdK8jlqm8tehUc9c9WhQ== vagrant insecure public key +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIN1YdxBpNlzxDqfJyw/QKow1F+wvG9hXGoqiysfJOn5Y vagrant insecure public key diff --git a/keys/vagrant.pub.ed25519 b/keys/vagrant.pub.ed25519 new file mode 100644 index 00000000000..ef65a946c86 --- /dev/null +++ b/keys/vagrant.pub.ed25519 @@ -0,0 +1 @@ +ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIN1YdxBpNlzxDqfJyw/QKow1F+wvG9hXGoqiysfJOn5Y vagrant insecure public key diff --git a/keys/vagrant.pub.rsa b/keys/vagrant.pub.rsa new file mode 100644 index 00000000000..18a9c00fd56 --- /dev/null +++ b/keys/vagrant.pub.rsa @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6NF8iallvQVp22WDkTkyrtvp9eWW6A8YVr+kz4TjGYe7gHzIw+niNltGEFHzD8+v1I2YJ6oXevct1YeS0o9HZyN1Q9qgCgzUFtdOKLv6IedplqoPkcmF0aYet2PkEDo3MlTBckFXPITAMzF8dJSIFo9D8HfdOV0IAdx4O7PtixWKn5y2hMNG0zQPyUecp4pzC6kivAIhyfHilFR61RGL+GPXQ2MWZWFYbAGjyiYJnAmCP3NOTd0jMZEnDkbUvxhMmBYSdETk1rRgm+R4LOzFUGaHqHDLKLX+FIPKcF96hrucXzcWyLbIbEgE98OHlnVYCzRdK8jlqm8tehUc9c9WhQ== vagrant insecure public key From 67562588c97fb6a29e017a36aaec3240e4ada85a Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 09:19:47 -0700 Subject: [PATCH 2/7] Add ed25519 keypair support Introduce keypair support for ed25519. Default keypair type when generating without specifying type is rsa to maintain existing behavior. --- lib/vagrant/util/keypair.rb | 171 ++++++++++++++++++++++++++++-------- 1 file changed, 135 insertions(+), 36 deletions(-) diff --git a/lib/vagrant/util/keypair.rb b/lib/vagrant/util/keypair.rb index 5e3d10ea6ce..31665af29f5 100644 --- a/lib/vagrant/util/keypair.rb +++ b/lib/vagrant/util/keypair.rb @@ -1,55 +1,154 @@ require "base64" -require "openssl" +require "ed25519" +require "securerandom" require "vagrant/util/retryable" module Vagrant module Util class Keypair - extend Retryable - - # Creates an SSH keypair and returns it. - # - # @param [String] password Password for the key, or nil for no password. - # @return [Array] PEM-encoded public and private key, - # respectively. The final element is the OpenSSH encoded public - # key. - def self.create(password=nil) - # This sometimes fails with RSAError. It is inconsistent and strangely - # sleeps seem to fix it. We just retry this a few times. See GH-5056 - rsa_key = nil - retryable(on: OpenSSL::PKey::RSAError, sleep: 2, tries: 5) do - rsa_key = OpenSSL::PKey::RSA.new(2048) + class Ed25519 + # Magic string header + AUTH_MAGIC = "openssh-key-v1".freeze + # Key type identifier + KEY_TYPE = "ssh-ed25519".freeze + # Header of private key file content + PRIVATE_KEY_START = "-----BEGIN OPENSSH PRIVATE KEY-----\n".freeze + # Footer of private key file content + PRIVATE_KEY_END = "-----END OPENSSH PRIVATE KEY-----".freeze + + # Encodes given string + # + # @param [String] s String to encode + # @return [String] + def self.string(s) + [s.length].pack("N") + s + end + + # Encodes given string with padding to block size + # + # @param [String] s String to encode + # @param [Integer] blocksize Defined block size + # @return [String] + def self.padded_string(s, blocksize) + pad = blocksize - (s.length % blocksize) + string(s + Array(1..pad).pack("c*")) end - public_key = rsa_key.public_key - private_key = rsa_key.to_pem + # Creates an ed25519 SSH key pair + # @return [Array] Public key, openssh private key, openssh public key with comment + # @note Password support was not included as it's not actively used anywhere. If it ends up being + # something that's needed, it can be revisited + def self.create(password=nil) + if password + raise NotImplementedError, + "Ed25519 key pair generation does not support passwords" + end + + # Generate the key + base_key = ::Ed25519::SigningKey.generate + # Define the comment used for the key + comment = "vagrant" + + # Grab the raw public key + public_key = base_key.verify_key.to_bytes + # Encode the public key for use building the openssh private key + encoded_public_key = string(KEY_TYPE) + string(public_key) + # Format the public key into the openssh public key format for writing + openssh_public_key = "#{KEY_TYPE} #{Base64.encode64(encoded_public_key).gsub("\n", "")} #{comment}" - if password - cipher = OpenSSL::Cipher.new('des3') - private_key = rsa_key.to_pem(cipher, password) + # Agent encoded private key is used when building the openssh private key + # (https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent#section-4.2.3) + # (https://dnaeon.github.io/openssh-private-key-binary-format/) + agent_private_key = [ + ([SecureRandom.random_number((2**32)-1)] * 2).pack("NN"), # checkint, random uint32 value, twice (used for encryption verification) + encoded_public_key, # includes the key type and public key + string(base_key.seed + public_key), # private key with public key concatenated + string(comment), # comment for the key + ].join + + # Build openssh private key data (https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.key) + private_key = [ + AUTH_MAGIC + "\0", # Magic string + string("none"), # cipher name, no encryption, so none + string("none"), # kdf name, no encryption, so none + string(""), # kdf options/data, no encryption, so empty string + [1].pack("N"), # Number of keys (just one) + string(encoded_public_key), # The public key + padded_string(agent_private_key, 8) # Private key encoded with agent rules, padded for 8 byte block size + ].join + + # Create the openssh private key content + openssh_private_key = [ + PRIVATE_KEY_START, + Base64.encode64(private_key), + PRIVATE_KEY_END, + ].join + + return [public_key, openssh_private_key, openssh_public_key] end + end + + class Rsa + extend Retryable - # Generate the binary necessary for the OpenSSH public key. - binary = [7].pack("N") - binary += "ssh-rsa" - ["e", "n"].each do |m| - val = public_key.send(m) - data = val.to_s(2) - - first_byte = data[0,1].unpack("c").first - if val < 0 - data[0] = [0x80 & first_byte].pack("c") - elsif first_byte < 0 - data = 0.chr + data + # Creates an SSH keypair and returns it. + # + # @param [String] password Password for the key, or nil for no password. + # @return [Array] PEM-encoded public and private key, + # respectively. The final element is the OpenSSH encoded public + # key. + def self.create(password=nil) + # This sometimes fails with RSAError. It is inconsistent and strangely + # sleeps seem to fix it. We just retry this a few times. See GH-5056 + rsa_key = nil + retryable(on: OpenSSL::PKey::RSAError, sleep: 2, tries: 5) do + rsa_key = OpenSSL::PKey::RSA.new(2048) end - binary += [data.length].pack("N") + data + public_key = rsa_key.public_key + private_key = rsa_key.to_pem + + if password + cipher = OpenSSL::Cipher.new('des3') + private_key = rsa_key.to_pem(cipher, password) + end + + # Generate the binary necessary for the OpenSSH public key. + binary = [7].pack("N") + binary += "ssh-rsa" + ["e", "n"].each do |m| + val = public_key.send(m) + data = val.to_s(2) + + first_byte = data[0,1].unpack("c").first + if val < 0 + data[0] = [0x80 & first_byte].pack("c") + elsif first_byte < 0 + data = 0.chr + data + end + + binary += [data.length].pack("N") + data + end + + openssh_key = "ssh-rsa #{Base64.encode64(binary).gsub("\n", "")} vagrant" + public_key = public_key.to_pem + return [public_key, private_key, openssh_key] + end + end + + # Supported key types. + VALID_TYPES = {ed25519: Ed25519, rsa: Rsa}.freeze + # Ordered mapping of openssh key type name to lookup name + PREFER_KEY_TYPES = {"ssh-ed25519".freeze => :ed25519, "ssh-rsa".freeze => :rsa}.freeze + + def self.create(password=nil, type: :rsa) + if !VALID_TYPES.key?(type) + raise ArgumentError, + "Invalid key type requested (supported types: #{VALID_TYPES.keys.map(&:inspect)})" end - openssh_key = "ssh-rsa #{Base64.encode64(binary).gsub("\n", "")} vagrant" - public_key = public_key.to_pem - return [public_key, private_key, openssh_key] + VALID_TYPES[type].create(password) end end end From cebfb7a63b0d7306bd65585cefbcb61730385f31 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 15:07:58 -0700 Subject: [PATCH 3/7] Use key type supported by server if possible Check the key types supported by the server. If the data is not available, default to the previous behavior which is using the rsa key type. Update insecure key check to match against any key files located within the keys directory. For now, this effectively allows matching either rsa or ed25519 insecure private keys. --- plugins/communicators/ssh/communicator.rb | 58 +++++++++++++++++++++-- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/plugins/communicators/ssh/communicator.rb b/plugins/communicators/ssh/communicator.rb index 68d36e44d77..e8667165186 100644 --- a/plugins/communicators/ssh/communicator.rb +++ b/plugins/communicators/ssh/communicator.rb @@ -185,7 +185,29 @@ def ready? @machine.guest.capability?(:remove_public_key) raise Vagrant::Errors::SSHInsertKeyUnsupported if !cap - _pub, priv, openssh = Vagrant::Util::Keypair.create + # Check for supported key type + key_type = catch(:key_type) do + begin + Vagrant::Util::Keypair::PREFER_KEY_TYPES.each do |type_name, type| + throw :key_type, type if supports_key_type?(type_name) + end + nil + rescue => err + @logger.warn("Failed to check key types server supports: #{err}") + nil + end + end + + @logger.debug("Detected key type for new private key: #{key_type}") + + # If no key type was discovered, default to rsa + if key_type.nil? + @logger.debug("Failed to detect supported key type, defaulting to rsa") + key_type = :rsa + end + + @logger.info("Creating new ssh keypair (type: #{key_type.inspect})") + _pub, priv, openssh = Vagrant::Util::Keypair.create(type: key_type) @logger.info("Inserting key to avoid password: #{openssh}") @machine.ui.detail("\n"+I18n.t("vagrant.inserting_random_key")) @@ -748,8 +770,9 @@ def scp_connect def insecure_key?(path) return false if !path return false if !File.file?(path) - source_path = Vagrant.source_root.join("keys", "vagrant") - return File.read(path).chomp == source_path.read.chomp + Dir.glob(Vagrant.source_root.join("keys", "vagrant.key.*")).any? do |source_path| + File.read(path).chomp == File.read(source_path).chomp + end end def create_remote_directory(dir) @@ -759,6 +782,35 @@ def create_remote_directory(dir) def machine_config_ssh @machine.config.ssh end + + protected + + # Check if server supports given key type + # + # @param [String, Symbol] type Key type + # @return [Boolean] + # @note This does not use a stable API and may be subject + # to unexpected breakage on net-ssh updates + def supports_key_type?(type) + if @connection.nil? + raise Vagrant::Errors::SSHNotReady + end + server_data = @connection. + transport&. + algorithms&. + instance_variable_get(:@server_data) + if server_data.nil? + @logger.warn("No server data available for key type support check") + return false + end + if !server_data.is_a?(Hash) + @logger.warn("Server data is not expected type (expecting Hash, got #{server_data.class})") + return false + end + + @logger.debug("server data used for host key support check: #{server_data.inspect}") + server_data[:host_key].include?(type.to_s) + end end end end From 380afe5fac713d4ad930c147940beb9237c18379 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 15:43:25 -0700 Subject: [PATCH 4/7] Define directory and paths for insecure private keys Within the environment, add a new directory value which points to the directory containing the valid insecure private keys. A new default private key paths value contains an array of all the insecure private keys which are available for initial authentication. --- lib/vagrant/environment.rb | 56 +++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 8b2531182ca..fcac4d44800 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -68,8 +68,11 @@ class Environment # The path where the plugins are stored (gems) attr_reader :gems_path - # The path to the default private key - attr_reader :default_private_key_path + # The path to the default private keys directory + attr_reader :default_private_keys_directory + + # The paths for each of the default private keys + attr_reader :default_private_key_paths # Initializes a new environment with the given options. The options # is a hash where the main available key is `cwd`, which defines where @@ -174,7 +177,12 @@ def initialize(opts=nil) # Setup the default private key @default_private_key_path = @home_path.join("insecure_private_key") - copy_insecure_private_key + @default_private_keys_directory = @home_path.join("insecure_private_keys") + if !@default_private_keys_directory.directory? + @default_private_keys_directory.mkdir + end + @default_private_key_paths = [] + copy_insecure_private_keys # Initialize localized plugins plugins = Vagrant::Plugin::Manager.instance.localize!(self) @@ -196,6 +204,13 @@ def initialize(opts=nil) hook(:environment_load, runner: Action::PrimaryRunner.new(env: self)) end + # The path to the default private key + # NOTE: deprecated, used default_private_keys_directory instead + def default_private_key_path + # TODO(spox): Add deprecation warning + @default_private_key_path + end + # Return a human-friendly string for pretty printed or inspected # instances. # @@ -1053,14 +1068,18 @@ def process_configured_plugins end end - # This method copies the private key into the home directory if it - # doesn't already exist. + # This method copies the private keys into the home directory if they + # do not already exist. The `default_private_key_path` references the + # original rsa based private key and is retained for compatibility. The + # `default_private_keys_directory` contains the list of valid private + # keys supported by Vagrant. # - # This must be done because `ssh` requires that the key is chmod + # NOTE: The keys are copied because `ssh` requires that the key is chmod # 0600, but if Vagrant is installed as a separate user, then the # effective uid won't be able to read the key. So the key is copied # to the home directory and chmod 0600. - def copy_insecure_private_key + def copy_insecure_private_keys + # First setup the deprecated single key path if !@default_private_key_path.exist? @logger.info("Copying private key to home directory") @@ -1084,6 +1103,29 @@ def copy_insecure_private_key @default_private_key_path.chmod(0600) end end + + # Now setup the key directory + Dir.glob(File.expand_path("keys/vagrant.key.*", Vagrant.source_root)).each do |source| + destination = default_private_keys_directory.join(File.basename(source)) + default_private_key_paths << destination + next if File.exist?(destination) + begin + FileUtils.cp(source, destination) + rescue Errno::EACCES + raise Errors::CopyPrivateKeyFailed, + source: source, + destination: destination + end + end + + if !Util::Platform.windows? + default_private_key_paths.each do |key_path| + if Util::FileMode.from_octal(key_path.stat.mode) != "600" + @logger.info("Changing permissions on private key (#{key_path}) to 0600") + key_path.chmod(0600) + end + end + end end # Finds the Vagrantfile in the given directory. From e0dbbcc04ce626c851eeba122ca45c66bfbf1342 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 15:46:15 -0700 Subject: [PATCH 5/7] Use all insecure private key paths When constructing the ssh information, use all available insecure key paths for authentication. --- lib/vagrant/machine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index 5335f3a27e6..be3f749eccb 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -502,7 +502,7 @@ def ssh_info if @config.ssh.private_key_path info[:private_key_path] = @config.ssh.private_key_path else - info[:private_key_path] = @env.default_private_key_path + info[:private_key_path] = @env.default_private_key_paths end end From 602d42bbc883bc8d7d9bb3f40b082e27cded4abc Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 15:47:32 -0700 Subject: [PATCH 6/7] Add and update tests for insecure private keys Updates existing test coverage to use insecure private key collection and adds testing for behavior changes within the communicator and the keypair utility. --- .../communicators/ssh/communicator_test.rb | 92 +++++++++++++++++++ test/unit/vagrant/machine_test.rb | 8 +- test/unit/vagrant/util/keypair_test.rb | 58 ++++++++---- 3 files changed, 139 insertions(+), 19 deletions(-) diff --git a/test/unit/plugins/communicators/ssh/communicator_test.rb b/test/unit/plugins/communicators/ssh/communicator_test.rb index 314bceb66d6..a40f7331c8f 100644 --- a/test/unit/plugins/communicators/ssh/communicator_test.rb +++ b/test/unit/plugins/communicators/ssh/communicator_test.rb @@ -242,6 +242,10 @@ let(:openssh){ :openssh } let(:private_key_file){ double("private_key_file") } let(:path_joiner){ double("path_joiner") } + let(:algorithms) { double(:algorithms) } + let(:transport) { double(:transport, algorithms: algorithms) } + let(:valid_key_types) { [] } + let(:server_data) { { host_key: valid_key_types} } before do allow(Vagrant::Util::Keypair).to receive(:create). @@ -255,6 +259,8 @@ allow(path_joiner).to receive(:join).and_return(private_key_file) allow(guest).to receive(:capability).with(:insert_public_key) allow(guest).to receive(:capability).with(:remove_public_key) + allow(connection).to receive(:transport).and_return(transport) + allow(algorithms).to receive(:instance_variable_get).with(:@server_data).and_return(server_data) end after{ communicator.ready? } @@ -280,6 +286,53 @@ it "should remove the default public key" do expect(guest).to receive(:capability).with(:remove_public_key, any_args) end + + context "with server algorithm support data" do + context "when no key type matches are found" do + it "should default to rsa type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :rsa).and_call_original + end + end + + context "when rsa is the only match" do + let(:valid_key_types) { ["ssh-edsca", "ssh-rsa"] } + + it "should use rsa type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :rsa).and_call_original + end + end + + context "when ed25519 and rsa are both available" do + let(:valid_key_types) { ["ssh-ed25519", "ssh-rsa"] } + + it "should use ed25519 type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :ed25519).and_call_original + end + end + + context "when ed25519 is the only match" do + let(:valid_key_types) { ["ssh-edsca", "ssh-ed25519"] } + + it "should use ed25519 type" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :ed25519).and_call_original + end + end + end + + context "when an error is encountered getting server data" do + before do + expect(connection).to receive(:transport).and_raise(StandardError) + end + + it "should default to rsa key" do + expect(Vagrant::Util::Keypair).to receive(:create). + with(type: :rsa).and_call_original + end + end end end end @@ -929,6 +982,45 @@ end end + describe ".insecure_key?" do + let(:key_data) { "" } + let(:key_file) { + if !@key_file + f = Tempfile.new + f.write(key_data) + f.close + @key_file = f.path + end + @key_file + } + + after { File.delete(key_file) } + + context "when using rsa private key" do + let(:key_data) { File.read(Vagrant.source_root.join("keys", "vagrant.key.rsa")) } + + it "should match as insecure key" do + expect(communicator.send(:insecure_key?, key_file)).to be_truthy + end + end + + context "when using ed25519 private key" do + let(:key_data) { File.read(Vagrant.source_root.join("keys", "vagrant.key.ed25519")) } + + it "should match as insecure key" do + expect(communicator.send(:insecure_key?, key_file)).to be_truthy + end + end + + context "when using unknown private key" do + let(:key_data) { "invalid data" } + + it "should not match as insecure key" do + expect(communicator.send(:insecure_key?, key_file)).to be_falsey + end + end + end + describe ".generate_environment_export" do it "should generate bourne shell compatible export" do expect(communicator.send(:generate_environment_export, "TEST", "value")).to eq("export TEST=\"value\"\n") diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index 6b4dd15310c..be160b2350a 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -729,7 +729,9 @@ def detect?(machine) provider_ssh_info[:private_key_path] = nil instance.config.ssh.private_key_path = nil - expect(ssh_klass).to receive(:check_key_permissions).once.with(Pathname.new(instance.env.default_private_key_path.to_s)) + instance.env.default_private_key_paths.each do |key_path| + expect(ssh_klass).to receive(:check_key_permissions).once.with(Pathname.new(key_path.to_s)) + end instance.ssh_info end @@ -773,7 +775,7 @@ def detect?(machine) instance.config.ssh.private_key_path = nil expect(instance.ssh_info[:private_key_path]).to eq( - [instance.env.default_private_key_path.to_s] + instance.env.default_private_key_paths ) end @@ -783,7 +785,7 @@ def detect?(machine) instance.config.ssh.keys_only = false expect(instance.ssh_info[:private_key_path]).to eq( - [instance.env.default_private_key_path.to_s] + instance.env.default_private_key_paths ) end diff --git a/test/unit/vagrant/util/keypair_test.rb b/test/unit/vagrant/util/keypair_test.rb index 30ab76eba6f..97213182363 100644 --- a/test/unit/vagrant/util/keypair_test.rb +++ b/test/unit/vagrant/util/keypair_test.rb @@ -1,34 +1,60 @@ require "openssl" +require "ed25519" +require "net/ssh" require File.expand_path("../../../base", __FILE__) require "vagrant/util/keypair" describe Vagrant::Util::Keypair do - describe ".create" do - it "generates a usable keypair with no password" do - # I don't know how to validate the final return value yet... - pubkey, privkey, _ = described_class.create + describe Vagrant::Util::Keypair::Rsa do + describe ".create" do + it "generates a usable keypair with no password" do + # I don't know how to validate the final return value yet... + pubkey, privkey, _ = described_class.create - pubkey = OpenSSL::PKey::RSA.new(pubkey) - privkey = OpenSSL::PKey::RSA.new(privkey) + pubkey = OpenSSL::PKey::RSA.new(pubkey) + privkey = OpenSSL::PKey::RSA.new(privkey) - encrypted = pubkey.public_encrypt("foo") - decrypted = privkey.private_decrypt(encrypted) + encrypted = pubkey.public_encrypt("foo") + decrypted = privkey.private_decrypt(encrypted) - expect(decrypted).to eq("foo") + expect(decrypted).to eq("foo") + end + + it "generates a keypair that requires a password" do + pubkey, privkey, _ = described_class.create("password") + + pubkey = OpenSSL::PKey::RSA.new(pubkey) + privkey = OpenSSL::PKey::RSA.new(privkey, "password") + + encrypted = pubkey.public_encrypt("foo") + decrypted = privkey.private_decrypt(encrypted) + + expect(decrypted).to eq("foo") + end end + end + + describe Vagrant::Util::Keypair::Ed25519 do + describe ".create" do + it "generates a usable keypair with no password" do + pubkey, ossh_privkey, _ = described_class.create - it "generates a keypair that requires a password" do - pubkey, privkey, _ = described_class.create("password") - pubkey = OpenSSL::PKey::RSA.new(pubkey) - privkey = OpenSSL::PKey::RSA.new(privkey, "password") + privkey = Net::SSH::Authentication::ED25519::PrivKey.read(ossh_privkey, "").sign_key + pubkey = Ed25519::VerifyKey.new(pubkey) - encrypted = pubkey.public_encrypt("foo") - decrypted = privkey.private_decrypt(encrypted) + message = "vagrant test" + signature = privkey.sign(message) + expect(pubkey.verify(signature, message)).to be_truthy + end - expect(decrypted).to eq("foo") + it "does not generate a keypair that requires a password" do + expect { + described_class.create("my password") + }.to raise_error(NotImplementedError) + end end end end From 2e95d08309dbb2dda005fed2fcb93891a4db372e Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 26 Jun 2023 15:48:53 -0700 Subject: [PATCH 7/7] Update box documentation about keys --- website/content/docs/boxes/base.mdx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/website/content/docs/boxes/base.mdx b/website/content/docs/boxes/base.mdx index c32f50b5e2b..95757d81583 100644 --- a/website/content/docs/boxes/base.mdx +++ b/website/content/docs/boxes/base.mdx @@ -115,15 +115,16 @@ users, passwords, private keys, etc.). By default, Vagrant expects a "vagrant" user to SSH into the machine as. This user should be setup with the -[insecure keypair](https://github.com/hashicorp/vagrant/tree/main/keys) +[insecure keypairs](https://github.com/hashicorp/vagrant/tree/main/keys) that Vagrant uses as a default to attempt to SSH. It should belong to a group named "vagrant". Also, even though Vagrant uses key-based authentication by default, it is a general convention to set the password for the "vagrant" user to "vagrant". This lets people login as that user manually if they need to. -To configure SSH access with the insecure keypair, place the public -key into the `~/.ssh/authorized_keys` file for the "vagrant" user. Note +To configure SSH access with the insecure keypair, place the [public +keys](https://github.com/hashicorp/vagrant/tree/main/keys/vagrant.pub) +into the `~/.ssh/authorized_keys` file for the "vagrant" user. Note that OpenSSH is very picky about file permissions. Therefore, make sure that `~/.ssh` has `0700` permissions and the authorized keys file has `0600` permissions.