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

Added support for custom SSL certificate #969

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
11 changes: 11 additions & 0 deletions lib/kamal/configuration/docs/proxy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ proxy:
# Defaults to `false`:
ssl: true

# Custom SSL certificate
#
# In scenarios where Let's Encrypt is not an option, or you already have your own
# certificates from a different Certificate Authority, you can configure kamal-proxy
# to load the certificate and the corresponding private key from disk.
#
# The certificate must be in PEM format and contain the full chain. The private key
# must also be in PEM format.
ssl_certificate_path: /data/cert/foo.example.com/fullchain.pem
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should read these directly from the Kamal secrets. Then here we can set the name of the secret rather than specifying a path.

We can also nest the pems under the ssl key. A hash for the ssl key would imply ssl: true.

E.g:

ssl:
  certificate_pem: CERTIFICATE_PEM
  private_key_pem: PRIVATE_KEY_PEM

Then in your secrets file:

# .kamal/secrets
CERTIFICATE_PEM=$(kamal secrets extract ...)
PRIVATE_KEY_PEM=$(kamal secrets extract ...)

ssl_private_key_path: /data/cert/foo.example.com/privkey.pem

# Response timeout
#
# How long to wait for requests to complete before timing out, defaults to 30 seconds:
Expand Down
6 changes: 6 additions & 0 deletions lib/kamal/configuration/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def ssl?
proxy_config.fetch("ssl", false)
end

def custom_ssl_certificate?
proxy_config["ssl_certificate_path"].present?
end

def hosts
proxy_config["hosts"] || proxy_config["host"]&.split(",") || []
end
Expand All @@ -30,6 +34,8 @@ def deploy_options
{
host: hosts,
tls: proxy_config["ssl"].presence,
"tls-certificate-path": proxy_config["ssl_certificate_path"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should mount a single volume into the proxy when we boot it.

This avoids having to set application level configuration for proxy volumes, which we want to avoid because multiple applications can share the same proxy.

We can then re-use that volume for any other features that need to provide files to the proxy (such as custom error pages for maintenance mode).

Also ideally we wouldn't need to pass paths around and kamal-proxy should be able to infer the location of the certificate.

I think it would mean that kamal-proxy would need to assume that app supplied files live in a different subdirectory to /home/kamal-proxy/.config/kamal-proxy so we can map the volume in separately - e.g. $(PWD)/.kamal/proxy/apps:/home/kamal-proxy/.config/kamal-proxy-apps.

/home/kamal-proxy/.config/kamal-proxy is then writable for the proxy, while /home/kamal-proxy/.config/kamal-proxy-apps is read only.

Then we can have /home/kamal-proxy/.config/kamal-proxy-apps/#{service}-#{role}-#{destination}/tls/ and store the certs in there. #{service}-#{role}-#{destination} will match the service name passed to the proxy, so kamal-proxy can infer where the certs are stored.

What to do think @kevinmcconnell?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll also need to update kamal app remove to scrub the /home/kamal-proxy/.config/kamal-proxy-apps/#{service}-#{role}-#{destination} directory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The kamal-proxy-apps plan sounds good to me. The proxy won't really care how it's organized, as we can give it absolute paths for the files that it needs (certs, error pages, etc). But as a scheme for organizing the per-app resources I think it makes sense 👍

"tls-private-key-path": proxy_config["ssl_private_key_path"],
"deploy-timeout": seconds_duration(config.deploy_timeout),
"drain-timeout": seconds_duration(config.drain_timeout),
"health-check-interval": seconds_duration(proxy_config.dig("healthcheck", "interval")),
Expand Down
4 changes: 2 additions & 2 deletions lib/kamal/configuration/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ def asset_volume_directory(version = config.version)
end

def ensure_one_host_for_ssl
if running_proxy? && proxy.ssl? && hosts.size > 1
raise Kamal::ConfigurationError, "SSL is only supported on a single server, found #{hosts.size} servers for role #{name}"
if running_proxy? && proxy.ssl? && hosts.size > 1 && !proxy.custom_ssl_certificate?
raise Kamal::ConfigurationError, "SSL is only supported on a single server or with custom SSL certificates, found #{hosts.size} servers for role #{name}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe unless you provide custom certificates?

end
end

Expand Down
8 changes: 8 additions & 0 deletions lib/kamal/configuration/validator/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ def validate!
if (config.keys & [ "host", "hosts" ]).size > 1
error "Specify one of 'host' or 'hosts', not both"
end

if config["ssl_certificate_path"].present? && config["ssl_private_key_path"].blank?
error "Must set a private key path to use a custom SSL certificate"
end

if config["ssl_private_key_path"].present? && config["ssl_certificate_path"].blank?
error "Must set a certificate path to use a custom SSL private key"
end
end
end
end
8 changes: 8 additions & 0 deletions test/commands/app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ class CommandsAppTest < ActiveSupport::TestCase
new_command.deploy(target: "172.1.0.2").join(" ")
end

test "deploy with custom SSL certificate" do
@config[:proxy] = { "ssl" => true, "host" => "example.com", "ssl_certificate_path" => "/path/to/cert.pem", "ssl_private_key_path" => "/path/to/key.pem" }

assert_equal \
"docker exec kamal-proxy kamal-proxy deploy app-web --target=\"172.1.0.2:80\" --host=\"example.com\" --tls --tls-certificate-path=\"/path/to/cert.pem\" --tls-private-key-path=\"/path/to/key.pem\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
new_command.deploy(target: "172.1.0.2").join(" ")
end

test "remove" do
assert_equal \
"docker exec kamal-proxy kamal-proxy remove app-web",
Expand Down
10 changes: 10 additions & 0 deletions test/configuration/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ class ConfigurationProxyTest < ActiveSupport::TestCase
assert_not config.proxy.ssl?
end

test "ssl with certificate path and no private key path" do
@deploy[:proxy] = { "ssl" => true, "ssl_certificate_path" => "/path/to/cert.pem" }
assert_raises(Kamal::ConfigurationError) { config.proxy.ssl? }
end

test "ssl with private key path and no certificate path" do
@deploy[:proxy] = { "ssl" => true, "ssl_private_key_path" => "/path/to/key.pem" }
assert_raises(Kamal::ConfigurationError) { config.proxy.ssl? }
end

private
def config
Kamal::Configuration.new(@deploy)
Expand Down
11 changes: 10 additions & 1 deletion test/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,16 @@ class ConfigurationTest < ActiveSupport::TestCase
Kamal::Configuration.new(@deploy_with_roles)
end

assert_equal "SSL is only supported on a single server, found 2 servers for role workers", exception.message
assert_equal "SSL is only supported on a single server or with custom SSL certificates, found 2 servers for role workers", exception.message
end

test "proxy ssl roles with multiple servers and a custom SSL certificate" do
@deploy_with_roles[:servers]["workers"]["proxy"] = { "ssl" => true, "host" => "foo.example.com", "ssl_certificate_path" => "/path/to/cert.pem", "ssl_private_key_path" => "/path/to/key.pem" }

config = Kamal::Configuration.new(@deploy_with_roles)

assert config.role(:workers).running_proxy?
assert config.role(:workers).ssl?
end

test "two proxy ssl roles with same host" do
Expand Down