On Sat, 2020-11-28 at 23:08 +0100, Theo Buehler wrote:
> > "If the certificate name is an absolute path, a .crt and .key
> > extension are appended to form the certificate path and key path
> > respectively."
> > This part does not seem to work at all.
> > Neither it tries to search certificates using the absolute path nor
> > it tries to append .crt or .key extension to the absolute path when no
> > extension is used in config.
> >
> > Or I do it completely wrong?
>
> It's a bug. If the certificate path is absolute, faulty short-circuiting
> logic would result in first correctly appending ".crt" to the path, then
> incorrectly prepending "/etc/ldap/cert".
>
> You can see the problem with a config containing
>
> listen on lo0 port 6636 tls certificate "/bogus/lo0"
>
> $ ldapd -vv -f ldapd.conf -n
> ...
> loading certificate file /etc/ldap/certs//bogus/lo0.crt
> ldapd.conf:5: cannot load certificate: /bogus/lo0
> ...
>
> The diff below avoids calling bsnprintf() twice for an absolute
> certificate path.
>
Wouldn't it be more future idiot proof if we were a little more verbose?
But if you prefer, your diff also looks good to me.
martijn@
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/parse.y,v
retrieving revision 1.36
diff -u -p -r1.36 parse.y
--- parse.y 24 Jun 2020 07:20:47 -0000 1.36
+++ parse.y 28 Nov 2020 22:54:42 -0000
@@ -1279,12 +1279,17 @@ load_certfile(struct ldapd_config *env,
goto err;
}
- if ((name[0] == '/' &&
- !bsnprintf(certfile, sizeof(certfile), "%s.crt", name)) ||
- !bsnprintf(certfile, sizeof(certfile), "/etc/ldap/certs/%s.crt",
- name)) {
- log_warn("load_certfile: path truncated");
- goto err;
+ if (name[0] == '/') {
+ if (!bsnprintf(certfile, sizeof(certfile), "%s.crt", name)) {
+ log_warn("load_certfile: path truncated");
+ goto err;
+ }
+ } else {
+ if (!bsnprintf(certfile, sizeof(certfile),
+ "/etc/ldap/certs/%s.crt", name)) {
+ log_warn("load_certfile: path truncated");
+ goto err;
+ }
}
log_debug("loading certificate file %s", certfile);
@@ -1298,12 +1303,17 @@ load_certfile(struct ldapd_config *env,
goto err;
}
- if ((name[0] == '/' &&
- !bsnprintf(certfile, sizeof(certfile), "%s.key", name)) ||
- !bsnprintf(certfile, sizeof(certfile), "/etc/ldap/certs/%s.key",
- name)) {
- log_warn("load_certfile: path truncated");
- goto err;
+ if (name[0] == '/') {
+ if (!bsnprintf(certfile, sizeof(certfile), "%s.key", name)) {
+ log_warn("load_certfile: path truncated");
+ goto err;
+ }
+ } else {
+ if (!bsnprintf(certfile, sizeof(certfile),
+ "/etc/ldap/certs/%s.key", name)) {
+ log_warn("load_certfile: path truncated");
+ goto err;
+ }
}
log_debug("loading key file %s", certfile);
