Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Anyone using kore,-acme?
To:
ports@openbsd.org
Cc:
joris@openbsd.org, fcambus@openbsd.org
Date:
Wed, 24 Jan 2024 23:25:17 +0100

Download raw body.

Thread
  • Theo Buehler:

    Anyone using kore,-acme?

kore,-acme is the only port using the X509V3_EXT_add_alias() API, which
I am going to remove from libcrypto. This undocumented API is a gross
hack that allows abusing an X509v3 extension method for something else.

kore aliases the subject key identifier method to build the RFC 8737
ACME Identifier extension since both happen to be encoded as octet
strings. kore uses the OpenSSL config mini language to build this
extension and the SAN. This language is responsible for various holes
and is best avoided altogether.

So the below patch uses the proper types to build up the two extensions
and adds them to the certs. This is admittedly a bit more complicated
than using strings, but it avoids lots of very ugly code in libcrypto.

I also added a bit of logic to avoid adding a copy of the acme OID if it
should happen to be added to some libcrypto of the future (it would most
likely be called NID_acmeIdentifier). This will confuse libcrypto and is
therefore best avoided. I do plan on adding this OID to libcrypto, but I
do not plan on adding a proper extension method, although that would not
be hard.

I checked that the DER encoding of the two extensions is as expected,
but I did not do further testing. Unfortunately, there are no tests
shipped with the port.

Index: Makefile
===================================================================
RCS file: /cvs/ports/www/kore/Makefile,v
diff -u -p -r1.36 Makefile
--- Makefile	27 Sep 2023 19:13:02 -0000	1.36
+++ Makefile	24 Jan 2024 22:02:00 -0000
@@ -1,7 +1,7 @@
 COMMENT =	web application framework for writing scalable web APIs in C
 
 DISTNAME =	kore-4.2.3
-REVISION =	0
+REVISION =	1
 
 CATEGORIES =	www
 
Index: patches/patch-src_keymgr_openssl_c
===================================================================
RCS file: patches/patch-src_keymgr_openssl_c
diff -N patches/patch-src_keymgr_openssl_c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_keymgr_openssl_c	24 Jan 2024 22:01:48 -0000
@@ -0,0 +1,184 @@
+Index: src/keymgr_openssl.c
+--- src/keymgr_openssl.c.orig
++++ src/keymgr_openssl.c
+@@ -196,8 +196,12 @@ static int			acmeproc_ready = 0;
+ /* Renewal timer for all domains under acme control. */
+ static struct kore_timer	*acme_renewal = NULL;
+ 
++#ifndef NID_acmeIdentifier
++#define NID_acmeIdentifier -1
++#endif
++
+ /* oid for acme extension. */
+-static int			acme_oid = -1;
++static int			acme_oid = NID_acmeIdentifier;
+ 
+ struct acme_order {
+ 	int			state;
+@@ -218,9 +222,10 @@ static void	keymgr_acme_order_create(const char *);
+ static void	keymgr_acme_order_redo(void *, u_int64_t);
+ static void	keymgr_acme_order_start(void *, u_int64_t);
+ 
+-static void	keymgr_x509_ext(STACK_OF(X509_EXTENSION) *,
+-		    int, const char *, ...)
+-		    __attribute__((format (printf, 3, 4)));
++static void	keymgr_x509_ext_alt_name_dns(STACK_OF(X509_EXTENSION *),
++		    const char *);
++static void	keymgr_x509_ext_acme_id(STACK_OF(X509_EXTENSION) *,
++		    const void *data, size_t len);
+ 
+ static void	keymgr_acme_csr(const struct kore_keyreq *, struct key *);
+ static void	keymgr_acme_install_cert(const void *, size_t, struct key *);
+@@ -316,8 +321,9 @@ kore_keymgr_run(void)
+ #endif
+ 
+ #if defined(KORE_USE_ACME)
+-	acme_oid = OBJ_create(ACME_TLS_ALPN_01_OID, "acme", "acmeIdentifier");
+-	X509V3_EXT_add_alias(acme_oid, NID_subject_key_identifier);
++	if (acme_oid == -1)
++		acme_oid = OBJ_create(ACME_TLS_ALPN_01_OID, "acme",
++		    "acmeIdentifier");
+ #endif
+ 
+ 	kore_worker_started();
+@@ -1078,15 +1084,12 @@ static void
+ keymgr_acme_challenge_cert(const void *data, size_t len, struct key *key)
+ {
+ 	STACK_OF(X509_EXTENSION)	*sk;
+-	size_t				idx;
+ 	time_t				now;
+ 	X509_EXTENSION			*ext;
+ 	X509_NAME			*name;
+ 	X509				*x509;
+-	const u_int8_t			*digest;
+ 	int				slen, i;
+ 	u_int8_t			*cert, *uptr;
+-	char				hex[(SHA256_DIGEST_LENGTH * 2) + 1];
+ 
+ 	kore_log(LOG_INFO, "[%s] generating tls-alpn-01 challenge cert",
+ 	    key->dom->domain);
+@@ -1094,15 +1097,6 @@ keymgr_acme_challenge_cert(const void *data, size_t le
+ 	if (len != SHA256_DIGEST_LENGTH)
+ 		fatalx("invalid digest length of %zu bytes", len);
+ 
+-	digest = data;
+-
+-	for (idx = 0; idx < SHA256_DIGEST_LENGTH; idx++) {
+-		slen = snprintf(hex + (idx * 2), sizeof(hex) - (idx * 2),
+-		    "%02x", digest[idx]);
+-		if (slen == -1 || (size_t)slen >= sizeof(hex))
+-			fatalx("failed to convert digest to hex");
+-	}
+-
+ 	if ((x509 = X509_new()) == NULL)
+ 		fatalx("X509_new(): %s", ssl_errno_s);
+ 
+@@ -1133,8 +1127,8 @@ keymgr_acme_challenge_cert(const void *data, size_t le
+ 		fatalx("X509_set_issuer_name(): %s", ssl_errno_s);
+ 
+ 	sk = sk_X509_EXTENSION_new_null();
+-	keymgr_x509_ext(sk, acme_oid, "critical,%s", hex);
+-	keymgr_x509_ext(sk, NID_subject_alt_name, "DNS:%s", key->dom->domain);
++	keymgr_x509_ext_acme_id(sk, data, len);
++	keymgr_x509_ext_alt_name_dns(sk, key->dom->domain);
+ 
+ 	for (i = 0; i < sk_X509_EXTENSION_num(sk); i++) {
+ 		ext = sk_X509_EXTENSION_value(sk, i);
+@@ -1190,7 +1184,7 @@ keymgr_acme_csr(const struct kore_keyreq *req, struct 
+ 		fatalx("X509_NAME_add_entry_by_txt(): %s", ssl_errno_s);
+ 
+ 	sk = sk_X509_EXTENSION_new_null();
+-	keymgr_x509_ext(sk, NID_subject_alt_name, "DNS:%s", key->dom->domain);
++	keymgr_x509_ext_alt_name_dns(sk, key->dom->domain);
+ 
+ 	if (!X509_REQ_add_extensions(csr, sk))
+ 		fatalx("X509_REQ_add_extensions(): %s", ssl_errno_s);
+@@ -1217,26 +1211,74 @@ keymgr_acme_csr(const struct kore_keyreq *req, struct 
+ }
+ 
+ static void
+-keymgr_x509_ext(STACK_OF(X509_EXTENSION) *sk, int extnid, const char *fmt, ...)
++keymgr_x509_ext_alt_name_dns(STACK_OF(X509_EXTENSION) *sk,
++    const char *dns_name)
+ {
+-	int			len;
+-	va_list			args;
++	ASN1_IA5STRING		*ia5;
++	GENERAL_NAME		*gen;
++	GENERAL_NAMES		*gens;
+ 	X509_EXTENSION		*ext;
+-	char			buf[1024];
+ 
+-	va_start(args, fmt);
+-	len = vsnprintf(buf, sizeof(buf), fmt, args);
+-	va_end(args);
++	ia5 = ASN1_IA5STRING_new();
++	if (ia5 == NULL)
++		fatalx("ASN1_IA5STRING_new(): %s", ssl_errno_s);
++	if (!ASN1_STRING_set(ia5, dns_name, -1))
++		fatalx("ASN1_STRING_set(): %s", ssl_errno_s);
+ 
+-	if (len == -1 || (size_t)len >= sizeof(buf))
+-		fatalx("failed to create buffer for extension %d", extnid);
++	gen = GENERAL_NAME_new();
++	if (gen == NULL)
++		fatalx("GENERAL_NAME_new(): %s", ssl_errno_s);
++	GENERAL_NAME_set0_value(gen, GEN_DNS, ia5);
+ 
+-	if ((ext = X509V3_EXT_conf_nid(NULL, NULL, extnid, buf)) == NULL) {
+-		fatalx("X509V3_EXT_conf_nid(%d, %s): %s",
+-		    extnid, buf, ssl_errno_s);
+-	}
++	gens = GENERAL_NAMES_new();
++	if (gens == NULL)
++		fatalx("GENERAL_NAMES_new(): %s", ssl_errno_s);
++	if (!sk_GENERAL_NAME_push(gens, gen))
++		fatalx("sk_GENERAL_NAME_push(): %s", ssl_errno_s);
+ 
+-	sk_X509_EXTENSION_push(sk, ext);
++	ext = X509V3_EXT_i2d(NID_subject_alt_name, 0, gens);
++	if (ext == NULL)
++		fatalx("X509V3_EXT_i2d(): %s", ssl_errno_s);
++
++	GENERAL_NAMES_free(gens);
++
++	if (!sk_X509_EXTENSION_push(sk, ext))
++		fatalx("sk_X509_EXTENSION_push(): %s", ssl_errno_s);
++}
++
++static void
++keymgr_x509_ext_acme_id(STACK_OF(X509_EXTENSION) *sk, const void *data,
++    size_t len)
++{
++	ASN1_OCTET_STRING	*aos;
++	X509_EXTENSION		*ext;
++	unsigned char		*der = NULL;
++	int			 der_len;
++
++	if (len != SHA256_DIGEST_LENGTH)
++		fatalx("invalid digest length of %zu bytes", len);
++
++	aos = ASN1_OCTET_STRING_new();
++	if (aos == NULL)
++		fatalx("ASN1_OCTET_STRING_new(): %s", ssl_errno_s);
++	if (!ASN1_STRING_set(aos, data, len))
++		fatalx("ASN1_STRING_set(): %s", ssl_errno_s);
++
++	der_len = i2d_ASN1_OCTET_STRING(aos, &der);
++	if (der_len <= 0)
++		fatalx("i2d_ASN1_OCTET_STRING(): %s", ssl_errno_s);
++	if (!ASN1_STRING_set(aos, der, der_len))
++		fatalx("ASN1_STRING_set(): %s", ssl_errno_s);
++	free(der);
++
++	ext = X509_EXTENSION_create_by_NID(NULL, acme_oid, 1, aos);
++	if (ext == NULL)
++		fatalx("X509_EXTENSION_create_by_NID(): %s", ssl_errno_s);
++
++	ASN1_OCTET_STRING_free(aos);
++
++	if (!sk_X509_EXTENSION_push(sk, ext))
++		fatalx("sk_X509_EXTENSION_push(): %s", ssl_errno_s);
+ }
+ 
+ static char *