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

Conversation

kpumuk
Copy link
Contributor

@kpumuk kpumuk commented Sep 24, 2024

Following the change in kamal-proxy, this MR introduces a configuration option to load custom SSL certificate and the corresponding private key from disk:

proxy:
  ssl: true
  ssl_certificate_path: /data/cert/foo.example.com/fullchain.pem
  ssl_private_key_path: /data/cert/foo.example.com/privkey.pem

Documentation preview:

image

@yogeshjain999
Copy link

I'm a newbie to kamal (and docker) and was wondering what's the recommended way to get those custom PEM files on server.

Copy using kamal+Dockerfile or do it manually ? Maybe better alternative would be to load the values via ENV variables (given kamal-proxy could support it) ?

@kpumuk
Copy link
Contributor Author

kpumuk commented Sep 30, 2024

I'm a newbie to kamal (and docker) and was wondering what's the recommended way to get those custom PEM files on server.

The easiest way would be to use pre-proxy-reboot. For example, if you use 1Password to manage secrets:

  1. Put both cert.pem and key.pem under the item you use for secrets
  2. Create .kamal/hooks/pre-proxy-reboot:
    #!/bin/sh
    
    set -euo pipefail
    
    KAMAL_PROXY_TLS_CERT=$(op read "op://Private/Kamal Demo/cert.pem")
    KAMAL_PROXY_TLS_PRIVATE_KEY=$(op read "op://Private/Kamal Demo/key.pem")
    
    for ip in ${KAMAL_HOSTS//,/ }; do
      ssh -q -T -o BatchMode=yes ubuntu@"${ip}" bash --noprofile <<-EOF
        mkdir -p .kamal/apps/${KAMAL_SERVICE}/tls
        echo '${KAMAL_PROXY_TLS_CERT}' > .kamal/apps/${KAMAL_SERVICE}/tls/cert.pem
        echo "${KAMAL_PROXY_TLS_PRIVATE_KEY}" > .kamal/apps/${KAMAL_SERVICE}/tls/key.pem
    EOF
    done
  3. Edit config/deploy.yml to mount TLS certificates to Kamal's image and then enable them:
    proxy:
      ssl: true
      host: app.example.com
      ssl_certificate_path: /home/kamal-proxy/.config/certs/cert.pem
      ssl_private_key_path: /home/kamal-proxy/.config/certs/key.pem
      volumes:
        - "/home/ubuntu/.kamal/apps/demo/certs:/home/kamal-proxy/.config/certs"
  4. Run kamal proxy reboot to deploy

Copy using kamal+Dockerfile or do it manually ? Maybe better alternative would be to load the values via ENV variables (given kamal-proxy could support it) ?

kamal-proxy does not support environment variables.

TODO: Add volumes support to proxy.

@yogeshjain999
Copy link

Cool, setting it up via pre-proxy-reboot sounds good approach. Thanks!!

@kpumuk
Copy link
Contributor Author

kpumuk commented Oct 2, 2024

@djmb to follow up on the thread in kamal-proxy, here is the documentation update + support for the recent custom TLS cert changes.

@agu-z
Copy link

agu-z commented Oct 9, 2024

Is it possible to specify a client certificate too? I need this in order to enable CloudFlare's Authenticated Origin Pulls

@mtmckenna
Copy link

With this change, would it be possible to remove the ensure_one_host_for_ssl requirement when providing a cert and key?

Context: I'm looking for a way to have end-to-end in-transit encryption in a regulated environment that requires TLS between the load balancer and server node. I'd also like to be able to use multiple app servers.

By providing my own cert to kamal-proxy, I was thinking I should be able to have the load balancer terminate SSL and then re-encrypt the traffic to kamal-proxy, which would use my supplied cert. wdyt?

Thank you!

@kpumuk
Copy link
Contributor Author

kpumuk commented Oct 14, 2024

By providing my own cert to kamal-proxy, I was thinking I should be able to have the load balancer terminate SSL and then re-encrypt the traffic to kamal-proxy, which would use my supplied cert. wdyt?

Yep, that's exactly how it would work with custom certificates, and it removes the limitation of one host behind the load balancer.

@mtmckenna
Copy link

Yep, that's exactly how it would work with custom certificates, and it removes the limitation of one host behind the load balancer.

That's great! Would ensure_one_host_for_ssl be updated in a separate PR?

def ensure_one_host_for_ssl

Thank you!

@kpumuk
Copy link
Contributor Author

kpumuk commented Oct 14, 2024

That's great! Would ensure_one_host_for_ssl be updated in a separate PR?

Great catch, I missed this change in the upstream :-) Added a commit to this MR

@qinmingyuan
Copy link

qinmingyuan commented Oct 17, 2024

I have used with wildcard domain and multi hosts, Works fine. Please merge this PR.

@dhh dhh requested a review from djmb October 18, 2024 22:17
@christo-ph
Copy link

I am looking forward to this feature. Could you kindly explain how I can test this without being merged and released?

@qinmingyuan
Copy link

qinmingyuan commented Oct 22, 2024

I am looking forward to this feature. Could you kindly explain how I can test this without being merged and released?

I have override the kamal command like this, and add the github repo to Gemfile:

Thread.report_on_exception = false
require_relative '../config/boot'
require 'kamal'
require 'kamal_override'

begin
  Kamal::Cli::Main.start(ARGV)
rescue SSHKit::Runner::ExecuteError => e
  puts "  \e[31mERROR (#{e.cause.class}): #{e.message}\e[0m"
  puts e.cause.backtrace if ENV["VERBOSE"]
  exit 1
rescue => e
  puts "  \e[31mERROR (#{e.class}): #{e.message}\e[0m"
  puts e.backtrace if ENV["VERBOSE"]
  exit 1
end

Copy link
Collaborator

@djmb djmb left a comment

Choose a reason for hiding this comment

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

Hi @kpumuk!

Thanks for putting this together and sorry for the delay in responding - I've been away.

I've added some comments on this - the main things are:

  • providing a single way to map files into the proxy container
  • read the certs from the Kamal secrets
  • get kamal-proxy to infer the cert locations

I'll need to tackle the volume/file mounting work as part of the maintenance mode work, so it might be easier to wait until that's out and then this can piggy back on that.

#
# 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 ...)

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?

@@ -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 👍

@Rukamakama
Copy link

Looking forward to having this functionality available, we are deploying multiple nodes behind a load balancer and would like to secure the transactions between the load balancer and application servers.
Thanks !

@junket
Copy link
Contributor

junket commented Nov 26, 2024

This seems really close! Thank you @kpumuk ❤️ Anything you need to get this over the finish line?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants