Skip to content

Commit a87bab1

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 83b07b6 commit a87bab1

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
@@ -219,15 +219,15 @@ certificate_and_key_write (const CertificateKeyPair *self,
219219
if (fchown (fd, buf.st_uid, buf.st_gid) != 0)
220220
err (EXIT_FAILURE, "%s: fchown", directory);
221221

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

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

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

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

232232
close (dirfd);
233233
close (fd);

src/tls/connection.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -846,19 +846,23 @@ connection_thread_main (int fd)
846846
* support for connections. If this function is not called, the server
847847
* will only be able to handle http requests.
848848
*
849-
* The certificate file must either contain the key as well, or end with
850-
* "*.crt" or "*.cert" and have a corresponding "*.key" file.
849+
* Loads all certificate/key pairs from the directory. Files ending in
850+
* ".crt" or ".cert" are treated as certificates, with corresponding
851+
* ".key" files for private keys.
851852
*
852-
* @certfile: Server TLS certificate file; cannot be %NULL
853+
* @cert_dirfd: Directory fd containing certificate/key files
854+
* @allow_unencrypted: Whether to allow plain HTTP connections
853855
* @request_mode: Whether to ask for client certificates
854856
*/
855857
void
856-
connection_crypto_init (const char *certificate_filename,
857-
const char *key_filename,
858+
connection_crypto_init (int cert_dirfd,
858859
bool allow_unencrypted,
859860
gnutls_certificate_request_t request_mode)
860861
{
861-
parameters.credentials = credentials_load (certificate_filename, key_filename);
862+
parameters.credentials = credentials_load_directory (cert_dirfd);
863+
if (parameters.credentials == NULL)
864+
errx (EXIT_FAILURE, "Failed to load certificates from directory");
865+
862866
parameters.request_mode = request_mode;
863867
/* If we aren't called, then require_https is false */
864868
parameters.require_https = !allow_unencrypted;

src/tls/connection.h

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

3232
void
33-
connection_crypto_init (const char *certificate_filename,
34-
const char *key_filename,
33+
connection_crypto_init (int cert_dirfd,
3534
bool allow_unencrypted,
3635
gnutls_certificate_request_t request_mode);
3736

src/tls/credentials.c

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

2424
#include <assert.h>
2525
#include <err.h>
26+
#include <errno.h>
27+
#include <fcntl.h>
28+
#include <stdio.h>
2629
#include <stdlib.h>
2730
#include <string.h>
31+
#include <sys/stat.h>
32+
#include <unistd.h>
2833

2934
#include <gnutls/x509.h>
3035

@@ -73,24 +78,111 @@ credentials_get (Credentials *self)
7378
return self->creds;
7479
}
7580

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

83-
debug (SERVER, "Using certificate %s", certificate_filename);
84-
85144
ret = gnutls_certificate_allocate_credentials (&creds);
86145
assert (ret == GNUTLS_E_SUCCESS);
87146

88-
ret = gnutls_certificate_set_x509_key_file (creds,
89-
certificate_filename, key_filename,
90-
GNUTLS_X509_FMT_PEM);
147+
Credentials *self = credentials_new(creds);
148+
149+
/* Load files sequentially 0.{crt,key}, 1.{crt,key}, 2.{crt,key}, etc... */
150+
int i;
151+
for (i = 0; ; i++)
152+
{
153+
char crt_name[32];
154+
snprintf (crt_name, sizeof crt_name, "%d.crt", i);
155+
156+
gnutls_datum_t crt_data;
157+
if (!load_file (dirfd, crt_name, &crt_data))
158+
break;
159+
160+
debug (SERVER, "Adding certificate %s", cert_name);
161+
162+
char key_name[32];
163+
snprintf (key_name, sizeof key_name, "%d.key", i);
164+
165+
gnutls_datum_t key_data;
166+
if (!load_file (dirfd, key_name, &key_data))
167+
errx (EXIT_FAILURE, "Certificate '%s' exists but key '%s' is missing",
168+
crt_name, key_name);
169+
170+
int ret = gnutls_certificate_set_x509_key_mem2 (self->creds,
171+
&crt_data, &key_data,
172+
GNUTLS_X509_FMT_PEM,
173+
NULL, 0);
174+
if (ret < 0)
175+
errx (EXIT_FAILURE, "Failed to load keypair %s/%s: %s",
176+
crt_name, key_name, gnutls_strerror (ret));
177+
178+
gnutls_memset (key_data.data, 0, key_data.size);
179+
free (key_data.data);
180+
free (crt_data.data);
181+
}
91182

92-
if (ret != GNUTLS_E_SUCCESS)
93-
errx (EXIT_FAILURE, "Failed to initialize server certificate: %s", gnutls_strerror (ret));
183+
if (i == 0)
184+
errx (EXIT_FAILURE, "No certificates found in directory");
94185

95-
return credentials_new (creds);
186+
debug (SERVER, "Loaded %d certificate(s)", i);
187+
return self;
96188
}

src/tls/credentials.h

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

2020
#pragma once
2121

22+
#include <stdbool.h>
23+
2224
#include <gnutls/gnutls.h>
2325

2426
typedef struct _Credentials Credentials;
@@ -35,3 +37,9 @@ credentials_get (Credentials *self);
3537
Credentials *
3638
credentials_load (const char *certificate_filename,
3739
const char *key_filename);
40+
41+
Credentials *
42+
credentials_new_empty (void);
43+
44+
Credentials *
45+
credentials_load_directory (int dirfd);

src/tls/main.c

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

2222
#include <argp.h>
2323
#include <err.h>
24+
#include <fcntl.h>
2425
#include <stddef.h>
26+
#include <stdio.h>
2527
#include <stdlib.h>
2628
#include <unistd.h>
2729

@@ -110,26 +112,32 @@ main (int argc, char **argv)
110112

111113
if (!arguments.no_tls)
112114
{
113-
char *error = NULL;
114-
115-
if (error)
116-
errx (EXIT_FAILURE, "Could not locate server certificate: %s", error);
115+
static const char cert_dir[] = "/run/cockpit/certs.d";
117116

118117
if (cockpit_conf_bool ("WebService", "ClientCertAuthentication", false))
119118
client_cert_mode = GNUTLS_CERT_REQUEST;
120119

121120
bool allow_unencrypted = cockpit_conf_bool ("WebService", "AllowUnencrypted", false);
122121

123-
connection_crypto_init ("/run/cockpit/tls/server/cert",
124-
"/run/cockpit/tls/server/key",
125-
allow_unencrypted, client_cert_mode);
122+
int cert_dirfd = open (cert_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
123+
if (cert_dirfd == -1)
124+
err (EXIT_FAILURE, "open: %s", cert_dir);
125+
126+
connection_crypto_init (cert_dirfd, allow_unencrypted, client_cert_mode);
127+
128+
/* Clean up certificate files after loading */
129+
for (int i = 0; ; i++)
130+
{
131+
char cert_name[16], key_name[16];
132+
snprintf (cert_name, sizeof cert_name, "%d.crt", i);
133+
snprintf (key_name, sizeof key_name, "%d.key", i);
126134

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

131-
if (unlink ("/run/cockpit/tls/server/key") != 0)
132-
err (EXIT_FAILURE, "unlink: /run/cockpit/tls/server/key");
140+
close (cert_dirfd);
133141
}
134142

135143
server_run ();

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

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

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

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

232232
if (tc->copy_exit == EXIT_SUCCESS)
@@ -239,8 +239,8 @@ test_copy (Fixture *fixture,
239239
g_autoptr(GTlsCertificate) input = g_tls_certificate_new_from_pem (input_data->str, input_data->len, &error);
240240
g_assert_no_error (error);
241241

242-
g_autofree gchar *certfile = g_build_filename (fixture->runtime_dir, "server", "cert", NULL);
243-
g_autofree gchar *keyfile = g_build_filename (fixture->runtime_dir, "server", "key", NULL);
242+
g_autofree gchar *certfile = g_build_filename (fixture->runtime_dir, "server", "0.crt", NULL);
243+
g_autofree gchar *keyfile = g_build_filename (fixture->runtime_dir, "server", "0.key", NULL);
244244
g_autoptr(GTlsCertificate) output = g_tls_certificate_new_from_files (certfile, keyfile, &error);
245245
#if !GLIB_CHECK_VERSION(2,58,0)
246246
/* 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
@@ -448,7 +448,20 @@ setup (TestCase *tc, gconstpointer data)
448448
server_init (tc->ws_socket_dir, tc->runtime_dir, fixture ? fixture->idle_timeout : 0, 0);
449449

450450
if (fixture && fixture->certfile)
451-
connection_crypto_init (fixture->certfile, fixture->keyfile, false, fixture->cert_request_mode);
451+
{
452+
/* Set up certs.d directory with 0.cert and 0.key */
453+
g_autofree gchar *certs_dir = g_build_filename (tc->runtime_dir, "certs.d", NULL);
454+
g_assert_cmpint (g_mkdir (certs_dir, 0700), ==, 0);
455+
g_autofree gchar *cert_link = g_build_filename (certs_dir, "0.crt", NULL);
456+
g_autofree gchar *key_link = g_build_filename (certs_dir, "0.key", NULL);
457+
g_assert_cmpint (symlink (fixture->certfile, cert_link), ==, 0);
458+
g_assert_cmpint (symlink (fixture->keyfile, key_link), ==, 0);
459+
460+
int cert_dirfd = open (certs_dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
461+
g_assert_cmpint (cert_dirfd, >=, 0);
462+
connection_crypto_init (cert_dirfd, false, fixture->cert_request_mode);
463+
close (cert_dirfd);
464+
}
452465

453466
/* Figure out the socket address we ought to connect to */
454467
socklen_t addrlen = sizeof tc->server_addr;
@@ -495,6 +508,17 @@ teardown (TestCase *tc, gconstpointer data)
495508
g_assert_cmpint (g_rmdir (tc->clients_dir), ==, 0);
496509
g_free (tc->clients_dir);
497510

511+
/* Clean up certs.d if it exists */
512+
g_autofree gchar *certs_dir = g_build_filename (tc->runtime_dir, "certs.d", NULL);
513+
if (g_file_test (certs_dir, G_FILE_TEST_IS_DIR))
514+
{
515+
g_autofree gchar *cert_file = g_build_filename (certs_dir, "0.crt", NULL);
516+
g_autofree gchar *key_file = g_build_filename (certs_dir, "0.key", NULL);
517+
g_unlink (cert_file);
518+
g_unlink (key_file);
519+
g_assert_cmpint (g_rmdir (certs_dir), ==, 0);
520+
}
521+
498522
g_assert_cmpint (g_rmdir (tc->runtime_dir), ==, 0);
499523
g_free (tc->runtime_dir);
500524

test/verify/check-connection

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

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

0 commit comments

Comments
 (0)