From: Rafael Sadowski Subject: Re: relayd: support explicit paths for keypair To: tech@openbsd.org Cc: Theo Buehler , "Kirill A. Korinsky" Date: Sat, 9 May 2026 14:59:45 +0200 On Sun Mar 29, 2026 at 03:31:40PM +0200, Kirill A. Korinsky wrote: > On Wed, 18 Mar 2026 17:47:42 +0100, > Rafael Sadowski wrote: > > > > On Thu Feb 19, 2026 at 09:28:17PM +0100, Rafael Sadowski wrote: > > > The following diff extends the keypair keyword in relayd.conf to allow > > > explicit path specifications for certificates, private keys, and OCSP > > > staple files. > > > > > > Currently, relayd relies on a fixed lookup logic, searching for TLS > > > crt/key in /etc/ssl and /etc/ssl/private based on the keypair name and > > > port. > > > > > > That has always annoyed me, since all other applications must comply > > > with the naming convention of relayd. > > > > > > The idea is simple, the keypair statement now supports optional > > > certificate, key, and ocsp keywords followed by a path: > > > > > > keypair name [certificate path [key path [ocsp path]]]. > > > > Is anyone interested? > > > > I think this is worth to be introduced. We already has in smptd.conf: > > pki pkiname cert certfile > > pki pkiname key keyfile > > And this is the way to somehow stay alligned. > > Personally, I think we need also support of keypair keyword in smtpd.conf, > but it is different diff. > > Not sure that keypair here is good name, maybe smtpd.conf way is cleaner. > Here is a rework of my latest diff after feedback from tb@ (Thanks!). - KNF and syntax fixes - Added PATH_MAX check in proto_keyname - Added missing free($*) - Added strlcpy truncation checks - Use secure snprintf idiom from manpage OK? diff --git a/usr.sbin/relayd/parse.y b/usr.sbin/relayd/parse.y index 6c3d538ed4e..5282afda091 100644 --- a/usr.sbin/relayd/parse.y +++ b/usr.sbin/relayd/parse.y @@ -140,6 +140,7 @@ int relay_id(struct relay *); struct relay *relay_inherit(struct relay *, struct relay *); int getservice(char *); int is_if_in_group(const char *, const char *); +static struct keyname *proto_keyname(char *); typedef struct { union { @@ -1342,20 +1343,93 @@ tlsflags : SESSION TICKETS { proto->tickets = 1; } free($3); } | KEYPAIR STRING { - struct keyname *name; + struct keyname *kname = NULL; - if (strlen($2) >= PATH_MAX) { - yyerror("keypair name too long"); + if ((kname = proto_keyname($2)) == NULL) { free($2); YYERROR; } - if ((name = calloc(1, sizeof(*name))) == NULL) { - yyerror("calloc"); + free($2); + } + | KEYPAIR STRING CERTIFICATE STRING { + struct keyname *kname = NULL; + + if ((kname = proto_keyname($2)) == NULL) { + free($2); + free($4); + YYERROR; + } + + if (strlen($4) >= PATH_MAX) { + yyerror("keypair cert too long"); free($2); + free($4); + YYERROR; + } + if (strlcpy(kname->certificate, $4, + sizeof(kname->certificate)) >= + sizeof(kname->certificate)) { + yyerror("keypair certificate truncated"); + free($2); + free($4); YYERROR; } - name->name = $2; - TAILQ_INSERT_TAIL(&proto->tlscerts, name, entry); + free($2); + free($4); + } + | KEYPAIR STRING KEY STRING { + struct keyname *kname = NULL; + + if ((kname = proto_keyname($2)) == NULL) { + free($2); + free($4); + YYERROR; + } + + if (strlen($4) >= PATH_MAX) { + yyerror("keypair certificate key too long"); + free($2); + free($4); + YYERROR; + } + + if (strlcpy(kname->key, $4, + sizeof(kname->key)) >= + sizeof(kname->key)) { + yyerror("keypair certificate key truncated"); + free($2); + free($4); + YYERROR; + } + free($2); + free($4); + } + | KEYPAIR STRING OCSP STRING { + struct keyname *kname = NULL; + + if ((kname = proto_keyname($2)) == NULL) { + free($2); + free($4); + YYERROR; + } + + if (strlen($4) >= PATH_MAX) { + yyerror("keypair ocsp file too long"); + free($2); + free($4); + YYERROR; + } + + if (strlcpy(kname->ocsp, $4, + sizeof(kname->ocsp)) >= + sizeof(kname->ocsp)) { + yyerror("ocsp truncated"); + free($2); + free($4); + YYERROR; + } + free($2); + free($4); } | CLIENT CA STRING { if (strlcpy(proto->tlsclientca, $3, @@ -1850,7 +1924,7 @@ relay : RELAY STRING { } '{' optnl relayopts_l '}' { struct relay *r; struct relay_config *rlconf = &rlay->rl_conf; - struct keyname *name; + struct keyname *kname; if (relay_findbyname(conf, rlconf->name) != NULL || relay_findbyaddr(conf, rlconf) != NULL) { @@ -1888,11 +1962,11 @@ relay : RELAY STRING { rlay->rl_conf.name); YYERROR; } - TAILQ_FOREACH(name, &rlay->rl_proto->tlscerts, entry) { + TAILQ_FOREACH(kname, &rlay->rl_proto->tlscerts, entry) { if (relay_load_certfiles(conf, - rlay, name->name) == -1) { + rlay, kname) == -1) { yyerror("cannot load keypair %s" - " for relay %s", name->name, + " for relay %s", kname->name, rlay->rl_conf.name); YYERROR; } @@ -3452,7 +3526,7 @@ relay_inherit(struct relay *ra, struct relay *rb) goto err; } TAILQ_FOREACH(name, &rb->rl_proto->tlscerts, entry) { - if (relay_load_certfiles(conf, rb, name->name) == -1) { + if (relay_load_certfiles(conf, rb, name) == -1) { yyerror("cannot load keypair %s for relay %s", name->name, rb->rl_conf.name); goto err; @@ -3551,3 +3625,31 @@ end: close(s); return (ret); } + +struct keyname* +proto_keyname(char *name) +{ + struct keyname *kname = NULL, *key; + + if (strlen(name) >= PATH_MAX) { + yyerror("keypair name too long"); + return NULL; + } + + + TAILQ_FOREACH(key, &proto->tlscerts, entry) { + if (strcmp(key->name, name) == 0) + return key; + } + + if ((kname = calloc(1, sizeof(*kname))) == NULL) { + return NULL; + } + + if ((kname->name = strdup(name)) == NULL) { + free(kname); + return NULL; + } + TAILQ_INSERT_TAIL(&proto->tlscerts, kname, entry); + return kname; +} diff --git a/usr.sbin/relayd/relayd.c b/usr.sbin/relayd/relayd.c index 9eb1b452d81..e3667a15dae 100644 --- a/usr.sbin/relayd/relayd.c +++ b/usr.sbin/relayd/relayd.c @@ -1335,14 +1335,14 @@ relay_load_fd(int fd, off_t *len) } int -relay_load_certfiles(struct relayd *env, struct relay *rlay, const char *name) +relay_load_certfiles(struct relayd *env, struct relay *rlay, const struct keyname *name) { char certfile[PATH_MAX]; char hbuf[PATH_MAX]; struct protocol *proto = rlay->rl_proto; struct relay_cert *cert; int useport = htons(rlay->rl_conf.port); - int cert_fd = -1, key_fd = -1, ocsp_fd = -1; + int cert_fd = -1, key_fd = -1, ocsp_fd = -1, ret = 0; if (rlay->rl_conf.flags & F_TLSCLIENT) { if (strlen(proto->tlsca) && rlay->rl_tls_ca_fd == -1) { @@ -1385,15 +1385,29 @@ relay_load_certfiles(struct relayd *env, struct relay *rlay, const char *name) print_host(&rlay->rl_conf.ss, hbuf, sizeof(hbuf)) == NULL) goto fail; else if (name != NULL && - strlcpy(hbuf, name, sizeof(hbuf)) >= sizeof(hbuf)) + strlcpy(hbuf, name->name, sizeof(hbuf)) >= sizeof(hbuf)) goto fail; - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/%s:%u.crt", hbuf, useport) == -1) - goto fail; + if (name != NULL && strcmp(name->certificate, "") != 0) { + if (strlcpy(certfile, name->certificate, sizeof(certfile)) + >= sizeof(certfile)) { + log_warnx("certificate truncated"); + goto fail; + } + } + else { + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/%s:%u.crt", hbuf, useport); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) + goto fail; + } if ((cert_fd = open(certfile, O_RDONLY)) == -1) { - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/%s.crt", hbuf) == -1) + + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/%s.crt", hbuf); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) goto fail; if ((cert_fd = open(certfile, O_RDONLY)) == -1) goto fail; @@ -1401,27 +1415,56 @@ relay_load_certfiles(struct relayd *env, struct relay *rlay, const char *name) } log_debug("%s: using certificate %s", __func__, certfile); - if (useport) { - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/private/%s:%u.key", hbuf, useport) == -1) - goto fail; - } else { - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/private/%s.key", hbuf) == -1) + if (name != NULL && strcmp(name->key, "") != 0) { + if (strlcpy(certfile, name->key, sizeof(certfile)) + >= sizeof(certfile)) { + log_warnx("certificate key truncated"); goto fail; + } + } + else { + if (useport) { + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/private/%s:%u.key", + hbuf, useport); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) + goto fail; + } else { + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/private/%s.key", hbuf); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) + goto fail; + } } if ((key_fd = open(certfile, O_RDONLY)) == -1) goto fail; log_debug("%s: using private key %s", __func__, certfile); - if (useport) { - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/%s:%u.ocsp", hbuf, useport) == -1) - goto fail; - } else { - if (snprintf(certfile, sizeof(certfile), - "/etc/ssl/%s.ocsp", hbuf) == -1) + if (name != NULL && strcmp(name->ocsp, "") != 0) { + if (strlcpy(certfile, name->ocsp, sizeof(certfile)) + >= sizeof(certfile)) { + log_warnx("certificate ocsp truncated"); goto fail; + } + + } + else { + if (useport) { + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/%s:%u.ocsp", + hbuf, useport); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) + goto fail; + } else { + ret = snprintf(certfile, sizeof(certfile), + "/etc/ssl/%s.ocsp", hbuf); + + if (ret < 0 || (size_t)ret >= sizeof(certfile)) + goto fail; + } } if ((ocsp_fd = open(certfile, O_RDONLY)) != -1) log_debug("%s: using OCSP staple file %s", __func__, certfile); diff --git a/usr.sbin/relayd/relayd.conf.5 b/usr.sbin/relayd/relayd.conf.5 index a63dd4e3a41..8803b57c73b 100644 --- a/usr.sbin/relayd/relayd.conf.5 +++ b/usr.sbin/relayd/relayd.conf.5 @@ -997,8 +997,10 @@ is omitted, is used. The default is .Ic no edh . -.It Ic keypair Ar name -The relay will attempt to look up a private key in +.It Ic keypair Ar name Op Ic cert Ar path Op Ic key Ar path Op Ic ocsp Ar path +The relay will attempt to look up the TLS assets associated with +.Ar name . +By default, it searches for a private key in .Pa /etc/ssl/private/name:port.key and a public certificate in .Pa /etc/ssl/name:port.crt , @@ -1009,6 +1011,16 @@ If these files are not present, the relay will continue to look in .Pa /etc/ssl/private/name.key and .Pa /etc/ssl/name.crt . +.Pp +If the +.Ic cert , +.Ic key , +or +.Ic ocsp +keywords are followed by an explicit +.Ar path , +that file will be used instead of the default location. +.Pp This option can be specified multiple times for TLS Server Name Indication. If not specified, a keypair will be loaded using the specified IP address of the relay as @@ -1017,8 +1029,10 @@ See .Xr ssl 8 for details about TLS server certificates. .Pp -An optional OCSP staple file will be used during TLS handshakes with -this server if it is found as a non-empty file in +An optional OCSP staple file will be used during TLS handshakes. +If no explicit +.Ic ocsp Ar path +is given, it will be searched as a non-empty file in .Pa /etc/ssl/name:port.ocsp or .Pa /etc/ssl/name.ocsp . diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h index a5363989f4b..1756d9aec7a 100644 --- a/usr.sbin/relayd/relayd.h +++ b/usr.sbin/relayd/relayd.h @@ -726,6 +726,9 @@ struct relay_ticket_key { struct keyname { TAILQ_ENTRY(keyname) entry; char *name; + char certificate[PATH_MAX]; + char key[PATH_MAX]; + char ocsp[PATH_MAX]; }; TAILQ_HEAD(keynamelist, keyname); @@ -1323,7 +1326,7 @@ struct relay_cert *cert_add(struct relayd *, objid_t); struct relay_cert *cert_find(struct relayd *, objid_t); char *relay_load_fd(int, off_t *); int relay_load_certfiles(struct relayd *, struct relay *, - const char *); + const struct keyname *); int expand_string(char *, size_t, const char *, const char *); void translate_string(char *); void purge_key(char **, off_t);