Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Messaging Hostname Validation #254

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion lib/manageiq/appliance_console/message_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def ask_questions
show_parameters
return false unless agree("\nProceed? (Y/N): ")

return false unless host_reachable?(message_server_host, "Message Server Host:")
return false unless host_resolvable?(message_server_host) && host_reachable?(message_server_host, "Message Server Host:")

true
end
Expand Down Expand Up @@ -190,6 +190,29 @@ def host_reachable?(host, what)
true
end

def host_resolvable?(host)
require 'ipaddr'
require 'resolv'

agrare marked this conversation as resolved.
Show resolved Hide resolved
say("Checking if #{host} is resolvable ... ")
begin
ip_address = Resolv.getaddress(host)

if IPAddr.new("127.0.0.1/8").include?(ip_address) || IPAddr.new("::1/128").include?(ip_address)
say("Failed.\nThe hostname must not resolve to a link-local address")

return false
end
rescue Resolv::ResolvError => e
say("Failed.\nHostname #{host} is not resolvable: #{e.message}")

return false
end

say("Succeeded.")
true
end

def unconfigure
remove_installed_files
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def configure
def ask_for_parameters
say("\nMessage Client Parameters:\n\n")

@message_server_host = ask_for_string("Message Server Hostname or IP address")
@message_server_host = ask_for_messaging_hostname("Message Server Hostname")
@message_server_port = ask_for_integer("Message Server Port number", (1..65_535), 9_093).to_i
@message_server_username = ask_for_string("Message Server Username", message_server_username)
@message_server_password = ask_for_password("Message Server Password")
Expand Down
15 changes: 3 additions & 12 deletions lib/manageiq/appliance_console/message_configuration_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ def restart_services
def ask_for_parameters
say("\nMessage Server Parameters:\n\n")

@message_server_host = ask_for_string("Message Server Hostname or IP address", message_server_host)

# SSL Validation for Kafka does not work for hostnames containing "localhost"
# Therefore we replace with the equivalent IP "127.0.0.1" if a /localhost*/ hostname was entered
@message_server_host = "127.0.0.1" if @message_server_host.include?("localhost")
@message_server_host = ask_for_messaging_hostname("Message Server Hostname", message_server_host)

@message_keystore_username = ask_for_string("Message Keystore Username", message_keystore_username)
@message_keystore_password = ask_for_new_password("Message Keystore Password")
Expand Down Expand Up @@ -301,13 +297,8 @@ def assemble_keystore_params
"-genkey" => nil,
"-keyalg" => "RSA"}

if message_server_host.ipaddress?
keystore_params["-alias"] = "localhost"
keystore_params["-ext"] = "san=ip:#{message_server_host}"
else
keystore_params["-alias"] = message_server_host
keystore_params["-ext"] = "san=dns:#{message_server_host}"
end
keystore_params["-alias"] = message_server_host
keystore_params["-ext"] = "san=dns:#{message_server_host}"

keystore_params["-dname"] = "cn=#{keystore_params["-alias"]}"

Expand Down
6 changes: 6 additions & 0 deletions lib/manageiq/appliance_console/prompts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Prompts
INT_REGEXP = /^[0-9]+$/
NONE_REGEXP = /^('?NONE'?)?$/i.freeze
HOSTNAME_REGEXP = /^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$/.freeze
MESSAGING_HOSTNAME_REGEXP = /^(?!.*localhost)(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$/.freeze

def ask_for_uri(prompt, expected_scheme, opts = {})
require 'uri'
Expand Down Expand Up @@ -71,6 +72,11 @@ def ask_for_hostname(prompt, default = nil, validate = HOSTNAME_REGEXP, error_te
just_ask(prompt, default, validate, error_text, &block)
end

def ask_for_messaging_hostname(prompt, default = nil, error_text = "a valid Messaging Hostname (not an IP or localhost)", &block)
validation = ->(h) { h =~ MESSAGING_HOSTNAME_REGEXP && h !~ IP_REGEXP }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I wonder if instead of mostly duplicating the hostname regex we just say h =~ HOSTNAME_REGEXP && h !~ IP_REGEXP && h != "localhost" here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a bad idea, the only difference I see is that the regex detects instances of localhost anywhere in the string. i.e. a hostname that has .localhost at the end (or anywhere), another example is the infamous localhost.localadmin

just_ask(prompt, default, validation, error_text, &block)
end

def ask_for_ip_or_hostname(prompt, default = nil)
validation = ->(h) { (h =~ HOSTNAME_REGEXP || h =~ IP_REGEXP) && h.length > 0 }
ask_for_ip(prompt, default, validation, "a valid Hostname or IP Address.")
Expand Down
5 changes: 3 additions & 2 deletions spec/message_configuration_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@
describe "#ask_questions" do
before do
allow(subject).to receive(:agree).and_return(true)
allow(subject).to receive(:host_resolvable?).and_return(true)
allow(subject).to receive(:host_reachable?).and_return(true)
allow(subject).to receive(:message_client_configured?).and_return(false)
end

it "should prompt for message_keystore_username and message_keystore_password" do
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_integer).with("Message Server Port number", (1..65_535), 9_093).and_return("9093")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
expect(subject).to receive(:ask_for_password).with("Message Keystore Password").and_return("top_secret")
Expand All @@ -61,7 +62,7 @@
end

it "should display Server Hostname and Key Username" do
allow(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address").and_return("my-kafka-server.example.com")
allow(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname").and_return("my-kafka-server.example.com")
allow(subject).to receive(:ask_for_integer).with("Message Server Port number", (1..65_535), 9_093).and_return("9093")
allow(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
allow(subject).to receive(:ask_for_password).with("Message Keystore Password").and_return("top_secret")
Expand Down
54 changes: 5 additions & 49 deletions spec/message_configuration_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
describe "#ask_questions" do
before do
allow(subject).to receive(:agree).and_return(true)
allow(subject).to receive(:host_resolvable?).and_return(true)
allow(subject).to receive(:host_reachable?).and_return(true)
allow(subject).to receive(:message_server_configured?).and_return(false)
end
Expand All @@ -45,7 +46,7 @@
end

it "should prompt for message_keystore_username and message_keystore_password" do
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")

Expand All @@ -55,7 +56,7 @@
end

it "should re-prompt when an empty message_keystore_password is given" do
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).and_return("")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")
Expand All @@ -67,7 +68,7 @@
end

it "should display Server Hostname and Keystore Username" do
allow(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", "my-host-name.example.com").and_return("my-host-name.example.com")
allow(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname", "my-host-name.example.com").and_return("my-host-name.example.com")
allow(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")

Expand All @@ -88,7 +89,7 @@

it "should prompt for message_keystore_username, message_keystore_password and persistent disk" do
message_persistent_disk = LinuxAdmin::Disk.new(:path => "/tmp/disk")
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_messaging_hostname).with("Message Server Hostname", "my-host-name.example.com").and_return("my-host-name.example.com")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", message_keystore_username).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")
expect(subject).to receive(:ask_for_disk).with("Persistent disk").and_return(message_persistent_disk)
Expand Down Expand Up @@ -215,14 +216,6 @@
expect(subject).to receive(:say).with("Configure Keystore")
end

context "with IP address" do
let(:ks_alias) { "localhost" }
let(:message_server_host) { "192.0.2.0" }
let(:ext) { "san=ip:#{message_server_host}" }

include_examples "configure keystore"
end

context "with hostname" do
let(:ks_alias) { "my-host-name.example.com" }
let(:message_server_host) { ks_alias }
Expand Down Expand Up @@ -283,13 +276,6 @@
end
end

context "with IP address" do
let(:ident_algorithm) { "" }
let(:client_auth) { "none" }
let(:message_server_host) { "192.0.2.0" }
include_examples "service properties file"
end

context "with hostname" do
let(:ident_algorithm) { "HTTPS" }
let(:client_auth) { "required" }
Expand Down Expand Up @@ -366,35 +352,5 @@
expect(subject.message_server_host).to eq("192.0.2.1")
end
end

context "when --message-server-host is specified as localhost*" do
before do
allow(subject).to receive(:agree).and_return(true)
allow(subject).to receive(:host_reachable?).and_return(true)
allow(subject).to receive(:message_server_configured?).and_return(true)
end

it "replaces localhost with 127.0.0.1" do
expect(subject).to receive(:say).with(/Message Server Parameters/)
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", anything).and_return("localhost")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", anything).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")
expect(subject).to receive(:ask_for_disk).with("Persistent disk").and_return("/tmp/disk")

subject.ask_for_parameters
expect(subject.message_server_host).to eq("127.0.0.1")
end

it "replaces localhost.localadmin with 127.0.0.1" do
expect(subject).to receive(:say).with(/Message Server Parameters/)
expect(subject).to receive(:ask_for_string).with("Message Server Hostname or IP address", anything).and_return("localhost.localadmin")
expect(subject).to receive(:ask_for_string).with("Message Keystore Username", anything).and_return("admin")
expect(subject).to receive(:just_ask).with(/Message Keystore Password/i, anything).twice.and_return("top_secret")
expect(subject).to receive(:ask_for_disk).with("Persistent disk").and_return("/tmp/disk")

subject.ask_for_parameters
expect(subject.message_server_host).to eq("127.0.0.1")
end
end
end
end