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

Added support for custom SSL certificate #969

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+55 −3
Open

Added support for custom SSL certificate #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81d96bd
Added support for custom SSL certificate
kpumuk Sep 24, 2024
ff32dcb
SSL is only supported on a single server or with custom SSL certificates
kpumuk Oct 14, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about this part a bit more:

ideally we wouldn't need to pass paths around and kamal-proxy should be able to infer the location of the certificate ...
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.

I think it may be better to not have default paths for this on the proxy side, and instead always make it explicit by passing the paths as part of the deploy command. Although that makes the deploy commands longer, it has a few advantages:

  • Only Kamal needs to be aware of the file structure it's using, and we're free to evolve that later on the Kamal side if we need to. Whereas if we lean on defaults, both Kamal & Kamal Proxy need to agree on what those are, so that Kamal can put the files in the places where Kamal Proxy expects to find them. I think it'll simplify things if we make it clear who owns the decision of where the files go, and who owns the files themselves.
  • This is perhaps a special case of the same point, but, I'm currently looking at ways to add path-based routing to the proxy (Add Support for Path-Based Routing in Kamal kamal-proxy#48), and one of the ramifications of that is TLS settings could straddle multiple services. The TLS settings are really domain-specific, but with path-based routing the same domain can cover multiple services. I'm not sure yet exactly how that will end up looking, but it does mean that mapping cert files to the service name may no longer be reliable (especially as services are added/updated/removed over time).
  • Similarly, if we have default paths for the certs, how do we specify when they should be used? If there was an old cert in that location but you wanted to switch to using automatic TLS instead, how does the proxy know what to do? I suppose we could add an additional flag like --tls-use-custom but at that point maybe passing the file paths is just as clear.

So my preference would be that Kamal implements a scheme like you described, but that we're explicit about paths when interacting with the proxy so it can remain agnostic about where we put things. What do you think?

"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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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