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

FEAT: Allow client side TLS for Docker hosts #2852

Merged
merged 3 commits into from Aug 4, 2023

Conversation

marekful
Copy link
Contributor

Inlcude client side TLS certificate in HTTPS requests to Docker hosts when certificate files are locally available to Kuma for the given host.

  • I have read and understand the pull request rules.

Description

Fixes #2353

Currently it's not possible to add a remote docker daemon that is secured with TLS because in this case the client needs to include the client side TLS certificate, issued by the docker host's CA, in every request.

This PR adds the ability to include client side certificate in HTTPS requests to docker hosts by looking for the certificate files at a specified location in the file system and including them in requests. Certificates for multiple docker hosts can be made available to Kuma as described below.

The base path where certificates are looked for can be set with the DOCKER_TLS_DIR_PATH environmental variable or defaults to data/docker-tls/.

If a directory in this path exists with a name matching the FQDN of the docker host (e.g. the FQDN of https://example.com:2376 is example.com so the directory data/docker-tls/example.com/ would be searched for certificate files), then ca.pem, key.pem and cert.pem files are loaded and included in the agent options. File names can also be overridden via DOCKER_TLS_FILE_NAME_(CA|KEY|CERT).

Type of change

Please delete any options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screen capture

Screen.Recording.2023-02-27.at.14.57.10.mp4

Inlcude TLS certificate in HTTPS requests when certificate
files are locally available to Kuma for a host.
@chakflying
Copy link
Collaborator

IMO uploading the certificate in the UI (in the docker host config) would be better for usability.

@marekful
Copy link
Contributor Author

marekful commented Mar 6, 2023

IMO uploading the certificate in the UI (in the docker host config) would be better for usability.

I focused on achieving the goal in the simplest acceptable way which results in ~50 lines of code. Your suggestion is absolutely valid and could be a nice addition but that would require much more effort. Storing certificates in database, providing UI for managing certificates and assigning them to hosts, etc. - probably in the order of a few thousand lines.

Which is not to say I wouldn't be open to that, should this PR receive some attention...

@flikites
Copy link

Would love to see this merged.

@Galmior Galmior mentioned this pull request Apr 13, 2023
2 tasks
@louislam louislam added this to the 1.23.0 milestone May 15, 2023

class DockerHost {

static CertificateBasePath = process.env.DOCKER_TLS_DIR_PATH || "data/docker-tls/";
Copy link
Owner

@louislam louislam Jul 9, 2023

Choose a reason for hiding this comment

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

Thanks for the pr.

Since data/ can be changed by the env var DATA_DIR, data/ cannot be hardcoded here.

But actually, in my opinion, these 4 env vars are not necessary, just hardcode:

  • Database.dataDir + "docker-tls/"
  • ca.pem
  • cert.pem
  • key.pem

Other sub-directories are created in this function.

Database.path = Database.dataDir + "kuma.db";
if (! fs.existsSync(Database.dataDir)) {
fs.mkdirSync(Database.dataDir, { recursive: true });
}
Database.uploadDir = Database.dataDir + "upload/";
if (! fs.existsSync(Database.uploadDir)) {
fs.mkdirSync(Database.uploadDir, { recursive: true });
}
// Create screenshot dir
Database.screenshotDir = Database.dataDir + "screenshots/";
if (! fs.existsSync(Database.screenshotDir)) {
fs.mkdirSync(Database.screenshotDir, { recursive: true });
}

@louislam louislam added the question Further information is requested label Jul 9, 2023
@louislam
Copy link
Owner

louislam commented Aug 4, 2023

Unable to push, I merge this first.

! [remote rejected]   feat/docker-host-client-side-tls -> feat/docker-host-client-side-tls (permission denied)

@ghnp5
Copy link

ghnp5 commented Sep 4, 2023

Hey! Happy to know that this is possible, at least! But it would be awesome if we could just upload the files through the UI. It's not clear to new Uptime Kuma users that we need to setup the folder structure. This is my first day using Uptime Kuma, and I was already thinking that it wouldn't serve the purpose to me, if I couldn't connect to Docker servers using TLS !! :-)
Thanks!!

@CommanderStorm
Copy link
Collaborator

It's not clear to new Uptime Kuma users that we need to setup the folder structure.

Feel free to improve the wiki for this.
I think this already mentones tls => no action required

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

Successfully merging this pull request may close these issues.

How to setup remote docker hosts with a TLS certificate?
6 participants