Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: relayd: support explicit paths for keypair
To:
Rafael Sadowski <rafael@sizeofvoid.org>
Cc:
tech@openbsd.org, Theo Buehler <tb@theobuehler.org>
Date:
Sat, 09 May 2026 18:13:05 +0200

Download raw body.

Thread
On Sat, 09 May 2026 14:59:45 +0200,
Rafael Sadowski <rafael@sizeofvoid.org> wrote:
> 
> 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 <rafael@sizeofvoid.org> 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?
>

Probably I do something wrong. I've tried config:

table <httpd> { 127.0.0.1 }

http protocol https {
        tls keypair xxx cert /etc/ssl/test.crt key /etc/ssl/private/test.key
}

relay https {
        listen on egress port 443 tls
        protocol https

        forward to <httpd> port 80
}

and it fails as:

relayd.conf:4: syntax error
relayd.conf:9: no such protocol: https
no actions, nothing to do


> 
> 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);
> 

-- 
wbr, Kirill