Skip to content

Commit 1ce3504

Browse files
tls: support multiple certificates in runtime dir
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.
1 parent d2e7190 commit 1ce3504

File tree

9 files changed

+177
-42
lines changed

9 files changed

+177
-42
lines changed

src/tls/cockpit-certificate-ensure.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,15 +207,15 @@ certificate_and_key_write (const CertificateKeyPair *self,
207207
if (fchown (fd, buf.st_uid, buf.st_gid) != 0)
208208
err (EXIT_FAILURE, "%s: fchown", directory);
209209

210-
if (symlinkat (self->certificate_filename, fd, "cert.source") != 0)
211-
err (EXIT_FAILURE, "%s/%s: symlinkat", directory, "certificate.source");
210+
if (symlinkat (self->certificate_filename, fd, "0.crt.source") != 0)
211+
err (EXIT_FAILURE, "%s/%s: symlinkat", directory, "0.crt.source");
212212

213-
if (symlinkat (self->key_filename, fd, "key.source") != 0)
214-
err (EXIT_FAILURE, "%s/%s: symlinkat", directory, "key.source");
213+
if (symlinkat (self->key_filename, fd, "0.key.source") != 0)
214+
err (EXIT_FAILURE, "%s/%s: symlinkat", directory, "0.key.source");
215215

216-
write_file (fd, directory, "cert", &self->certificate, buf.st_uid, buf.st_gid);
216+
write_file (fd, directory, "0.crt", &self->certificate, buf.st_uid, buf.st_gid);
217217

218-
write_file (fd, directory, "key", &self->key, buf.st_uid, buf.st_gid);
218+
write_file (fd, directory, "0.key", &self->key, buf.st_uid, buf.st_gid);
219219

220220
close (dirfd);
221221
close (fd);

src/tls/connection.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -832,19 +832,23 @@ connection_thread_main (int fd)
832832
* support for connections. If this function is not called, the server
833833
* will only be able to handle http requests.
834834
*
835-
* The certificate file must either contain the key as well, or end with
836-
* "*.crt" or "*.cert" and have a corresponding "*.key" file.
835+
* Loads all certificate/key pairs from the directory. Files ending in
836+
* ".crt" or ".cert" are treated as certificates, with corresponding
837+
* ".key" files for private keys.
837838
*
838-
* @certfile: Server TLS certificate file; cannot be %NULL
839+
* @cert_dirfd: Directory fd containing certificate/key files
840+
* @allow_unencrypted: Whether to allow plain HTTP connections
839841
* @request_mode: Whether to ask for client certificates
840842
*/
841843
void
842-
connection_crypto_init (const char *certificate_filename,
843-
const char *key_filename,
844+
connection_crypto_init (int cert_dirfd,
844845
bool allow_unencrypted,
845846
gnutls_certificate_request_t request_mode)
846847
{
847-
parameters.credentials = credentials_load (certificate_filename, key_filename);
848+
parameters.credentials = credentials_load_directory (cert_dirfd);
849+
if (parameters.credentials == NULL)
850+
errx (EXIT_FAILURE, "Failed to load certificates from directory");
851+
848852
parameters.request_mode = request_mode;
849853
/* If we aren't called, then require_https is false */
850854
parameters.require_https = !allow_unencrypted;

src/tls/connection.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ connection_set_directories (const char *wsinstance_sockdir,
1616
const char *runtime_directory);
1717

1818
void
19-
connection_crypto_init (const char *certificate_filename,
20-
const char *key_filename,
19+
connection_crypto_init (int cert_dirfd,
2120
bool allow_unencrypted,
2221
gnutls_certificate_request_t request_mode);
2322

src/tls/credentials.c

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,13 @@
99

1010
#include <assert.h>
1111
#include <err.h>
12+
#include <errno.h>
13+
#include <fcntl.h>
14+
#include <stdio.h>
1215
#include <stdlib.h>
1316
#include <string.h>
17+
#include <sys/stat.h>
18+
#include <unistd.h>
1419

1520
#include <gnutls/x509.h>
1621

@@ -59,24 +64,111 @@ credentials_get (Credentials *self)
5964
return self->creds;
6065
}
6166

67+
/* Load a file into a gnutls_datum_t.
68+
*
69+
* Returns true on success, false if the file doesn't exist.
70+
* Exits on any other failure.
71+
* On success, data->data must be freed with gnutls_free().
72+
*/
73+
static bool
74+
load_file (int dirfd, const char *filename, gnutls_datum_t *data)
75+
{
76+
int fd = openat (dirfd, filename, O_RDONLY | O_CLOEXEC | O_NOCTTY);
77+
if (fd < 0)
78+
{
79+
if (errno == ENOENT)
80+
return false;
81+
err (EXIT_FAILURE, "Failed to open '%s'", filename);
82+
}
83+
84+
struct stat st;
85+
if (fstat (fd, &st) < 0)
86+
err (EXIT_FAILURE, "Failed to stat '%s'", filename);
87+
88+
if (!S_ISREG (st.st_mode))
89+
errx (EXIT_FAILURE, "'%s' is not a regular file", filename);
90+
91+
if (st.st_size <= 0)
92+
errx (EXIT_FAILURE, "'%s' is empty", filename);
93+
94+
if (st.st_size > 640 * 1024) /* ought to be enough for anybody! */
95+
errx (EXIT_FAILURE, "'%s' is too large", filename);
96+
97+
size_t file_size = (size_t) st.st_size;
98+
99+
unsigned char *buffer = mallocx (file_size + 1);
100+
if (buffer == NULL)
101+
errx (EXIT_FAILURE, "Failed to allocate memory for '%s'", filename);
102+
103+
ssize_t n;
104+
do
105+
n = read (fd, buffer, file_size);
106+
while (n < 0 && errno == EINTR);
107+
108+
if (n < 0)
109+
err (EXIT_FAILURE, "Failed to read '%s'", filename);
110+
111+
if (n != (ssize_t) file_size)
112+
errx (EXIT_FAILURE, "Failed to read '%s': expected %zu bytes, got %zd", filename, file_size, n);
113+
114+
close (fd);
115+
116+
buffer[file_size] = '\0';
117+
118+
data->data = buffer;
119+
data->size = (unsigned int) file_size; /* <= 640k */
120+
121+
return true;
122+
}
123+
62124
Credentials *
63-
credentials_load (const char *certificate_filename,
64-
const char *key_filename)
125+
credentials_load_directory (int dirfd)
65126
{
66127
gnutls_certificate_credentials_t creds;
67128
int ret;
68129

69-
debug (SERVER, "Using certificate %s", certificate_filename);
70-
71130
ret = gnutls_certificate_allocate_credentials (&creds);
72131
assert (ret == GNUTLS_E_SUCCESS);
73132

74-
ret = gnutls_certificate_set_x509_key_file (creds,
75-
certificate_filename, key_filename,
76-
GNUTLS_X509_FMT_PEM);
133+
Credentials *self = credentials_new(creds);
134+
135+
/* Load files sequentially 0.{crt,key}, 1.{crt,key}, 2.{crt,key}, etc... */
136+
int i;
137+
for (i = 0; ; i++)
138+
{
139+
char crt_name[32];
140+
snprintf (crt_name, sizeof crt_name, "%d.crt", i);
141+
142+
gnutls_datum_t crt_data;
143+
if (!load_file (dirfd, crt_name, &crt_data))
144+
break;
145+
146+
debug (SERVER, "Adding certificate %s", cert_name);
147+
148+
char key_name[32];
149+
snprintf (key_name, sizeof key_name, "%d.key", i);
150+
151+
gnutls_datum_t key_data;
152+
if (!load_file (dirfd, key_name, &key_data))
153+
errx (EXIT_FAILURE, "Certificate '%s' exists but key '%s' is missing",
154+
crt_name, key_name);
155+
156+
int ret = gnutls_certificate_set_x509_key_mem2 (self->creds,
157+
&crt_data, &key_data,
158+
GNUTLS_X509_FMT_PEM,
159+
NULL, 0);
160+
if (ret < 0)
161+
errx (EXIT_FAILURE, "Failed to load keypair %s/%s: %s",
162+
crt_name, key_name, gnutls_strerror (ret));
163+
164+
gnutls_memset (key_data.data, 0, key_data.size);
165+
free (key_data.data);
166+
free (crt_data.data);
167+
}
77168

78-
if (ret != GNUTLS_E_SUCCESS)
79-
errx (EXIT_FAILURE, "Failed to initialize server certificate: %s", gnutls_strerror (ret));
169+
if (i == 0)
170+
errx (EXIT_FAILURE, "No certificates found in directory");
80171

81-
return credentials_new (creds);
172+
debug (SERVER, "Loaded %d certificate(s)", i);
173+
return self;
82174
}

src/tls/credentials.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
#pragma once
77

8+
#include <stdbool.h>
9+
810
#include <gnutls/gnutls.h>
911

1012
typedef struct _Credentials Credentials;
@@ -21,3 +23,9 @@ credentials_get (Credentials *self);
2123
Credentials *
2224
credentials_load (const char *certificate_filename,
2325
const char *key_filename);
26+
27+
Credentials *
28+
credentials_new_empty (void);
29+
30+
Credentials *
31+
credentials_load_directory (int dirfd);

src/tls/main.c

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77

88
#include <argp.h>
99
#include <err.h>
10+
#include <fcntl.h>
1011
#include <stddef.h>
12+
#include <stdio.h>
1113
#include <stdlib.h>
1214
#include <unistd.h>
1315

@@ -96,26 +98,32 @@ main (int argc, char **argv)
9698

9799
if (!arguments.no_tls)
98100
{
99-
char *error = NULL;
100-
101-
if (error)
102-
errx (EXIT_FAILURE, "Could not locate server certificate: %s", error);
101+
static const char cert_dir[] = "/run/cockpit/tls/server";
103102

104103
if (cockpit_conf_bool ("WebService", "ClientCertAuthentication", false))
105104
client_cert_mode = GNUTLS_CERT_REQUEST;
106105

107106
bool allow_unencrypted = cockpit_conf_bool ("WebService", "AllowUnencrypted", false);
108107

109-
connection_crypto_init ("/run/cockpit/tls/server/cert",
110-
"/run/cockpit/tls/server/key",
111-
allow_unencrypted, client_cert_mode);
108+
int cert_dirfd = open (cert_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
109+
if (cert_dirfd == -1)
110+
err (EXIT_FAILURE, "open: %s", cert_dir);
111+
112+
connection_crypto_init (cert_dirfd, allow_unencrypted, client_cert_mode);
113+
114+
/* Clean up certificate files after loading */
115+
for (int i = 0; ; i++)
116+
{
117+
char cert_name[16], key_name[16];
118+
snprintf (cert_name, sizeof cert_name, "%d.crt", i);
119+
snprintf (key_name, sizeof key_name, "%d.key", i);
112120

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

117-
if (unlink ("/run/cockpit/tls/server/key") != 0)
118-
err (EXIT_FAILURE, "unlink: /run/cockpit/tls/server/key");
126+
close (cert_dirfd);
119127
}
120128

121129
server_run ();

src/tls/test-cockpit-certificate-ensure.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,10 @@ test_copy (Fixture *fixture,
209209
cockpit_assert_strmatch (stderr_str, tc->copy_stderr);
210210
g_assert_cmpint (g_subprocess_get_exit_status (helper), ==, tc->copy_exit);
211211

212-
g_autofree gchar *cert_source = areadlinkat (fixture->runtime_dir_fd, "server/cert.source");
212+
g_autofree gchar *cert_source = areadlinkat (fixture->runtime_dir_fd, "server/0.crt.source");
213213
cockpit_assert_strmatch (cert_source, tc->cert_source);
214214

215-
g_autofree gchar *key_source = areadlinkat (fixture->runtime_dir_fd, "server/key.source");
215+
g_autofree gchar *key_source = areadlinkat (fixture->runtime_dir_fd, "server/0.key.source");
216216
cockpit_assert_strmatch (key_source, tc->key_source);
217217

218218
if (tc->copy_exit == EXIT_SUCCESS)
@@ -225,8 +225,8 @@ test_copy (Fixture *fixture,
225225
g_autoptr(GTlsCertificate) input = g_tls_certificate_new_from_pem (input_data->str, input_data->len, &error);
226226
g_assert_no_error (error);
227227

228-
g_autofree gchar *certfile = g_build_filename (fixture->runtime_dir, "server", "cert", NULL);
229-
g_autofree gchar *keyfile = g_build_filename (fixture->runtime_dir, "server", "key", NULL);
228+
g_autofree gchar *certfile = g_build_filename (fixture->runtime_dir, "server", "0.crt", NULL);
229+
g_autofree gchar *keyfile = g_build_filename (fixture->runtime_dir, "server", "0.key", NULL);
230230
g_autoptr(GTlsCertificate) output = g_tls_certificate_new_from_files (certfile, keyfile, &error);
231231
#if !GLIB_CHECK_VERSION(2,58,0)
232232
/* Older GLib (RHEL 8) doesn't know how to find EC private keys */

src/tls/test-server.c

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,20 @@ setup (TestCase *tc, gconstpointer data)
434434
server_init (tc->ws_socket_dir, tc->runtime_dir, fixture ? fixture->idle_timeout : 0, 0);
435435

436436
if (fixture && fixture->certfile)
437-
connection_crypto_init (fixture->certfile, fixture->keyfile, false, fixture->cert_request_mode);
437+
{
438+
/* Set up certs directory with 0.cert and 0.key */
439+
g_autofree gchar *certs_dir = g_build_filename (tc->runtime_dir, "tls/server", NULL);
440+
g_assert_cmpint (g_mkdir (certs_dir, 0700), ==, 0);
441+
g_autofree gchar *cert_link = g_build_filename (certs_dir, "0.crt", NULL);
442+
g_autofree gchar *key_link = g_build_filename (certs_dir, "0.key", NULL);
443+
g_assert_cmpint (symlink (fixture->certfile, cert_link), ==, 0);
444+
g_assert_cmpint (symlink (fixture->keyfile, key_link), ==, 0);
445+
446+
int cert_dirfd = open (certs_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
447+
g_assert_cmpint (cert_dirfd, >=, 0);
448+
connection_crypto_init (cert_dirfd, false, fixture->cert_request_mode);
449+
close (cert_dirfd);
450+
}
438451

439452
/* Figure out the socket address we ought to connect to */
440453
socklen_t addrlen = sizeof tc->server_addr;
@@ -481,6 +494,17 @@ teardown (TestCase *tc, gconstpointer data)
481494
g_assert_cmpint (g_rmdir (tc->clients_dir), ==, 0);
482495
g_free (tc->clients_dir);
483496

497+
/* Clean up certs.d if it exists */
498+
g_autofree gchar *certs_dir = g_build_filename (tc->runtime_dir, "certs.d", NULL);
499+
if (g_file_test (certs_dir, G_FILE_TEST_IS_DIR))
500+
{
501+
g_autofree gchar *cert_file = g_build_filename (certs_dir, "0.crt", NULL);
502+
g_autofree gchar *key_file = g_build_filename (certs_dir, "0.key", NULL);
503+
g_unlink (cert_file);
504+
g_unlink (key_file);
505+
g_assert_cmpint (g_rmdir (certs_dir), ==, 0);
506+
}
507+
484508
g_assert_cmpint (g_rmdir (tc->runtime_dir), ==, 0);
485509
g_free (tc->runtime_dir);
486510

test/verify/check-connection

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1451,7 +1451,7 @@ Command={self.libexecdir}/cockpit-session
14511451
b.wait_text("#server-name", "ExDeeGee")
14521452

14531453
# uses the expected certs
1454-
self.assertEqual(m.execute("readlink /run/cockpit/tls/server/key.source").strip(),
1454+
self.assertEqual(m.execute("readlink /run/cockpit/tls/server/0.key.source").strip(),
14551455
"/etc/test-xdg/cockpit/ws-certs.d/0-self-signed.key")
14561456
# and did not generate new ones
14571457
self.assertNotIn("ws-certs.d", m.execute("ls /etc/cockpit"))

0 commit comments

Comments
 (0)