cockpit-tls: enable loading multiple TLS certificates (for post-quantum) by allisonkarlitskaya · Pull Request #22831 · cockpit-project/cockpit

allisonkarlitskaya · 2026-01-30T16:27:18Z

(incomplete)

The Credentials type wraps gnutls_certificate_credentials_t, which can hold multiple certificate/key pairs. Renaming to Credentials better reflects this capability and aligns with GnuTLS terminology. This is preparation for supporting multiple certificates (e.g., PQC with RSA fallback). Assisted-by: Claude <noreply@anthropic.com>

Instead of having one "cert" and one "key" file in the runtime directory, we support any number of files named like "n.crt" and "n.key" where n is an integer starting from 0. cockpit-tls reads files until it sees ENOENT. Meanwhile in cockpit-certificate-ensure, we change the names of the files that we output to "0.crt" and "0.key". We still don't support multiple keys yet, but that will come next.

Add tests to verify that when multiple certificates (RSA and ECDSA) are configured, the server correctly selects the certificate based on the client's signature algorithm preferences. Assisted-by: Claude <noreply@anthropic.com>

martinpitt · 2026-02-19T14:37:15Z

@allisonkarlitskaya this failure feels relevant? cockpit-certificate-ensure --for-cockpit-tls failed, so the unit failed.

cockpit-certificate-ensure[103587]: cockpit-certificate-ensure: /etc/cockpit/ws-certs.d/0-self-signed.cert/.key: Base64 decoding error.

And then the unit tries to auto-restart, and eventually barfs.

the other failure is a flake obviously.

martinpitt

Thanks! Code looks good to me. I'll ask Copilot for a second opinion, it finds different stuff than us humans.

martinpitt · 2026-02-19T14:38:53Z

src/tls/cockpit-certificate-ensure.c

 #include <common/cockpitwebcertificate.h>
 #include <common/cockpitmemory.h>

-#include "certificate.h"


Hmm, is this really not using anything from credentials.h? Or is that some weird "transient #include" effect?

martinpitt · 2026-02-19T14:43:59Z

src/tls/credentials.h


 #pragma once

+#include <stdbool.h>


This commit does not add anything bool-ish here to the include. Leftover?

Copilot

Pull request overview

This PR updates cockpit-tls to support loading multiple TLS certificate/key pairs (to enable post-quantum and other multi-certificate deployments) by switching from fixed certificate filenames to a directory-based, numbered certificate set.

Changes:

Replace single certificate/key loading with directory-based loading of sequential N.crt/N.key pairs.
Update certificate ensure/helper outputs and tests to use 0.crt/0.key (and .source symlinks) naming.
Add TLS tests that verify certificate selection (RSA vs ECDSA) via GnuTLS priority configuration.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/verify/check-connection	Adjust expected `.source` symlink name to `0.key.source`.
src/tls/test-server.c	Extend test fixtures to provision multiple certs and validate selected server cert algorithm.
src/tls/test-cockpit-certificate-ensure.c	Update assertions and file loading to use `0.crt`/`0.key`.
src/tls/main.c	Open certificate directory, initialize TLS from dirfd, and delete numbered cert/key files after loading.
src/tls/credentials.h	Introduce new credentials API header for multi-cert loading.
src/tls/credentials.c	Implement directory-based loading of sequential `N.crt`/`N.key` into GnuTLS credentials.
src/tls/connection.h	Update `connection_crypto_init()` signature to accept a cert directory fd.
src/tls/connection.c	Switch from `Certificate` to `Credentials` and initialize TLS from directory-loaded credentials.
src/tls/cockpit-certificate-ensure.c	Write `0.crt`/`0.key` and corresponding `.source` symlinks instead of legacy names.
src/tls/certificate.h	Removed legacy single-certificate API.
src/tls/certificate.c	Removed legacy single-certificate implementation.
src/tls/Makefile-tls.am	Replace certificate sources with credentials sources in the TLS library build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T14:50:51Z

src/tls/credentials.c

+      if (!load_file (dirfd, crt_name, &crt_data))
+        break;
+
+      debug (SERVER, "Adding certificate %s", cert_name);


cert_name is not defined here; this should likely log crt_name. As-is, this is a build-breaking compile error in credentials_load_directory().

Suggested change

debug (SERVER, "Adding certificate %s", cert_name);

debug (SERVER, "Adding certificate %s", crt_name);

Copilot · 2026-02-19T14:50:52Z

src/tls/credentials.c

+ *
+ * Returns true on success, false if the file doesn't exist.
+ * Exits on any other failure.
+ * On success, data->data must be freed with gnutls_free().


The comment says data->data must be freed with gnutls_free(), but this function allocates via mallocx() and later frees with free(). Please update the comment to match the actual ownership/deallocator (or switch to gnutls allocators if that’s the intent).

Suggested change

* On success, data->data must be freed with gnutls_free().

* On success, data->data must be freed with free().

Copilot · 2026-02-19T14:50:52Z

src/tls/credentials.h

+credentials_load (const char *certificate_filename,
+                  const char *key_filename);
+
+Credentials *
+credentials_new_empty (void);
+
+Credentials *


credentials.h declares credentials_load() and credentials_new_empty(), but there are no definitions in src/tls/credentials.c (and no other definitions in the repo). This advertises APIs that don’t exist and will cause link failures if any code starts using them; please either implement them or remove the declarations.

Suggested change

credentials_load (const char *certificate_filename,

const char *key_filename);

Credentials *

credentials_new_empty (void);

Credentials *

Copilot · 2026-02-19T14:50:52Z

src/tls/connection.c

+ * Loads all certificate/key pairs from the directory. Files ending in
+ * ".crt" or ".cert" are treated as certificates, with corresponding
+ * ".key" files for private keys.


The doc comment describes loading arbitrary *.crt/*.cert + *.key pairs from the directory, but credentials_load_directory() actually only looks for sequentially numbered %d.crt and %d.key. Please update this comment (and parameter docs if needed) to reflect the numbered file convention actually required.

Copilot · 2026-02-19T14:50:52Z

src/tls/main.c

+      /* Clean up certificate files after loading */
+      for (int i = 0; ; i++)
+        {
+          char cert_name[16], key_name[16];
+          snprintf (cert_name, sizeof cert_name, "%d.crt", i);
+          snprintf (key_name, sizeof key_name, "%d.key", i);

-      /* There's absolutely no need to keep these around */
-      if (unlink ("/run/cockpit/tls/server/cert") != 0)
-        err (EXIT_FAILURE, "unlink: /run/cockpit/tls/server/cert");
+          if (unlinkat (cert_dirfd, cert_name, 0) != 0)
+            break;
+          unlinkat (cert_dirfd, key_name, 0);
+        }


The cleanup loop breaks on any unlinkat(cert) error. That can silently ignore unexpected failures (e.g. EACCES/EPERM/EBUSY) and leave partially cleaned state. Consider breaking only on errno == ENOENT and treating other errors as fatal (and likewise check/report failures for the key unlink if cleanup is intended to be strict).

Copilot · 2026-02-19T14:50:53Z

src/tls/main.c

-                              allow_unencrypted, client_cert_mode);
+      int cert_dirfd = open (cert_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
+      if (cert_dirfd == -1)
+        err (EXIT_FAILURE, "open: %s", cert_dir);


err(EXIT_FAILURE, ...) already appends the strerror(errno) text; using the format string "open: %s" makes the message a bit redundant/confusing. Consider err(EXIT_FAILURE, "%s", cert_dir) or a clearer message like "Failed to open %s".

Suggested change

err (EXIT_FAILURE, "open: %s", cert_dir);

err (EXIT_FAILURE, "Failed to open %s", cert_dir);

allisonkarlitskaya force-pushed the pqc branch from a87bab1 to ae0aeb1 Compare February 5, 2026 15:09

allisonkarlitskaya added 2 commits February 12, 2026 13:38

allisonkarlitskaya force-pushed the pqc branch from ae0aeb1 to 2cb0d7e Compare February 19, 2026 07:38

allisonkarlitskaya force-pushed the pqc branch from 2cb0d7e to af5d513 Compare February 19, 2026 13:18

allisonkarlitskaya requested a review from martinpitt February 19, 2026 14:01

martinpitt reviewed Feb 19, 2026

View reviewed changes

martinpitt requested a review from Copilot February 19, 2026 14:46

Copilot started reviewing on behalf of martinpitt February 19, 2026 14:47 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cockpit-tls: enable loading multiple TLS certificates (for post-quantum)#22831

cockpit-tls: enable loading multiple TLS certificates (for post-quantum)#22831
allisonkarlitskaya wants to merge 3 commits intocockpit-project:mainfrom
allisonkarlitskaya:pqc

allisonkarlitskaya commented Jan 30, 2026

Uh oh!

martinpitt commented Feb 19, 2026

Uh oh!

martinpitt left a comment

Uh oh!

martinpitt Feb 19, 2026

Uh oh!

martinpitt Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	debug (SERVER, "Adding certificate %s", cert_name);
	debug (SERVER, "Adding certificate %s", crt_name);

	* On success, data->data must be freed with gnutls_free().
	* On success, data->data must be freed with free().

	err (EXIT_FAILURE, "open: %s", cert_dir);
	err (EXIT_FAILURE, "Failed to open %s", cert_dir);

Conversation

allisonkarlitskaya commented Jan 30, 2026

Uh oh!

martinpitt commented Feb 19, 2026

Uh oh!

martinpitt left a comment

Choose a reason for hiding this comment

Uh oh!

martinpitt Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

martinpitt Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants