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.

Sign up for GitHub

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

Fixes #34667 - Add SSL support when connecting to mqtt broker #75

Merged
merged 2 commits into from
Apr 6, 2022
Merged

Fixes #34667 - Add SSL support when connecting to mqtt broker #75

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54e0452
Fixes #34667 - Add SSL support when connecting to mqtt broker
adamruzicka Mar 23, 2022
b874a3f
Fixes #34667 - Use client certs
adamruzicka Mar 31, 2022
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
4 changes: 4 additions & 0 deletions lib/smart_proxy_remote_execution_ssh.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def validate_mqtt_settings!

raise 'mqtt_broker has to be set when pull-mqtt mode is used' if Plugin.settings.mqtt_broker.nil?
raise 'mqtt_port has to be set when pull-mqtt mode is used' if Plugin.settings.mqtt_port.nil?

if Plugin.settings.mqtt_tls.nil?
Plugin.settings.mqtt_tls = [:ssl_certificate, :ssl_private_key, :ssl_ca_file].all? { |key| ::Proxy::SETTINGS[key] }
Copy link
Member

Choose a reason for hiding this comment

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

I think this line needs updating with change to client certificates.

end
end

def validate_ssh_log_level!
Expand Down
11 changes: 10 additions & 1 deletion lib/smart_proxy_remote_execution_ssh/actions/pull_script.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,20 @@ def mqtt_start(otp_password)
end

def mqtt_notify(payload)
MQTT::Client.connect(settings.mqtt_broker, settings.mqtt_port) do |c|
with_mqtt_client do |c|
c.publish(mqtt_topic, JSON.dump(payload), false, 1)
end
end

def with_mqtt_client(&block)
MQTT::Client.connect(settings.mqtt_broker, settings.mqtt_port,
:ssl => settings.mqtt_tls,
:cert_file => ::Proxy::SETTINGS.ssl_certificate,
:key_file => ::Proxy::SETTINGS.ssl_private_key,
:ca_file => ::Proxy::SETTINGS.ssl_ca_file,
Copy link
Member

Choose a reason for hiding this comment

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

I have been thinking about certificates and this raises a good question: should we use the smart-proxy server certificates or the foreman client certificates (:foreman_ssl_ca, :foreman_ssl_cert, :foreman_ssl_key) to talk to mosquitto?

A few things to consider with this.

  1. In a standard Foreman installation, this should be all Puppet certificates so it doesn't really matter.
  2. In a standard Katello installation, this should be all certificates from the default CA so it doesn't really matter.

And then there is the dragon scenarios:

  1. If the user provides custom server certificates in the Katello scenario:
    a) the smart-proxy server certificates will be the custom certificates and mosquitto will need to be configured with the CA of those certificates to accept them.
    b) the smart-proxy Foreman client certificates will be from the default CA and if used, mosquitto would need to be configured with the default CA

That leads in to what certificates should we configure mosquitto with: ones derived only from the default CA or the same server certificates running the smart-proxy?

Foreman has a similar scenario if the user is using Puppet certificates but has opted to provide the smart-proxy with server certificates from a different CA.

@ekohl @evgeni your thoughts are appreciated, apologies for any headaches this may cause

Copy link
Member

Choose a reason for hiding this comment

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

This is one of the headaches I already had (thanks to a very special tree).
Either Mosquitto or Go's TLS stack was very picky about which certificate was used where, based on flags present. So using the non-client cert didn't work and "solved" the selection for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to client certs

&block)
end

def host_name
alternative_names = input.fetch(:alternative_names, {})

Expand Down
1 change: 1 addition & 0 deletions lib/smart_proxy_remote_execution_ssh/plugin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class Plugin < Proxy::Plugin
:cleanup_working_dirs => true,
# :mqtt_broker => nil,
# :mqtt_port => nil,
# :mqtt_tls => nil,
:mode => :ssh

plugin :ssh, Proxy::RemoteExecution::Ssh::VERSION
Expand Down
5 changes: 5 additions & 0 deletions settings.d/remote_execution_ssh.yml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,8 @@
# MQTT configuration, need to be set if mode is set to pull-mqtt
# :mqtt_broker: localhost
# :mqtt_port: 1883

# Use of SSL can be forced either way by explicitly setting mqtt_tls setting. If
# unset, SSL gets used if smart-proxy's ssl_certificate, ssl_private_key and
# ssl_ca_file settings are set available.
Copy link
Member

Choose a reason for hiding this comment

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

Does this also need updating if using client certificates?

# :mqtt_tls: