Using the "ldaps" or "tls" keywords in ldapd.conf currently enables all

protocols and ciphers. So you get a TLS server speaking TLSv1.0 and
supporting cipher suites with RC4 and 3DES encryption, all of which should
be considered broken. There is no way of disabling TLSv1.0 and TLSv1.1 in
ldapd. All this is also not very clearly called out in the documentation.

This commit switches the defaults to using the libtls defaults for both
protocols and ciphers. If compatibility with the insecure legacy protocols
and ciphers is needed, use the "legacy" keyword before "tls" or "ldaps" in
ldapd.conf.

tested by abieber.
inoguchi agrees with the direction.

ok beck

3 files changed, 40 insertions, 14 deletions

diff --git a/usr.sbin/ldapd/ldapd.conf.5 b/usr.sbin/ldapd/ldapd.conf.5
index 3da0137e137..5e4e69dbf79 100644
--- a/usr.sbin/ldapd/ldapd.conf.5
+++ b/usr.sbin/ldapd/ldapd.conf.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ldapd.conf.5,v 1.26 2020/02/10 13:18:21 schwarze Exp $
+.\"	$OpenBSD: ldapd.conf.5,v 1.27 2020/06/24 07:20:47 tb Exp $
 .\"
 .\" Copyright (c) 2009, 2010 Martin Hedenfalk <martin@bzero.se>
 .\" Copyright (c) 2008 Janne Johansson <jj@openbsd.org>
@@ -17,7 +17,7 @@
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
 .\"
-.Dd $Mdocdate: February 10 2020 $
+.Dd $Mdocdate: June 24 2020 $
 .Dt LDAPD.CONF 5
 .Os
 .Sh NAME
@@ -79,6 +79,7 @@ is described below.
 .It Xo
 .Ic listen on Ar interface
 .Op Ic port Ar port
+.Op Ic legacy
 .Op Ic tls | ldaps | secure
 .Op Ic certificate Ar name
 .Xc
@@ -97,6 +98,15 @@ by default on port 389,
 or LDAPS
 .Pq Ic ldaps ,
 by default on port 636.
+.Ic tls
+and
+.Ic ldaps
+connections will use the defaults from libtls.
+If compatibility with the insecure TLSv1.0 and TLSv1.1
+protocols and ciphers is required,
+they can be enabled with the
+.Ic legacy
+keyword.
 Creation of certificates is documented in
 .Xr starttls 8 .
 If no certificate
diff --git a/usr.sbin/ldapd/ldapd.h b/usr.sbin/ldapd/ldapd.h
index 3f995d184b4..235c228f389 100644
--- a/usr.sbin/ldapd/ldapd.h
+++ b/usr.sbin/ldapd/ldapd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ldapd.h,v 1.31 2018/07/31 11:01:00 claudio Exp $ */
+/*	$OpenBSD: ldapd.h,v 1.32 2020/06/24 07:20:47 tb Exp $ */
 
 /*
  * Copyright (c) 2009, 2010 Martin Hedenfalk <martin@bzero.se>
@@ -53,6 +53,7 @@
 #define F_SSL			(F_LDAPS|F_STARTTLS)
 
 #define F_SECURE		 0x04
+#define F_LEGACY		 0x08
 
 #define F_SCERT			 0x01
 
diff --git a/usr.sbin/ldapd/parse.y b/usr.sbin/ldapd/parse.y
index bad9bc63040..bf27aa7a256 100644
--- a/usr.sbin/ldapd/parse.y
+++ b/usr.sbin/ldapd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.35 2019/02/13 22:57:08 deraadt Exp $ */
+/*	$OpenBSD: parse.y,v 1.36 2020/06/24 07:20:47 tb Exp $ */
 
 /*
  * Copyright (c) 2009, 2010 Martin Hedenfalk <martinh@openbsd.org>
@@ -83,7 +83,7 @@ int		 host(const char *, const char *,
 		    struct listenerlist *, int, in_port_t, u_int8_t);
 int		 interface(const char *, const char *,
 		    struct listenerlist *, int, in_port_t, u_int8_t);
-int		 load_certfile(struct ldapd_config *, const char *, u_int8_t);
+int		 load_certfile(struct ldapd_config *, const char *, u_int8_t, u_int8_t);
 
 TAILQ_HEAD(symhead, sym)	 symhead = TAILQ_HEAD_INITIALIZER(symhead);
 struct sym {
@@ -116,14 +116,14 @@ static struct namespace *current_ns = NULL;
 
 %}
 
-%token	ERROR LISTEN ON TLS LDAPS PORT NAMESPACE ROOTDN ROOTPW INDEX
+%token	ERROR LISTEN ON LEGACY TLS LDAPS PORT NAMESPACE ROOTDN ROOTPW INDEX
 %token	SECURE RELAX STRICT SCHEMA USE COMPRESSION LEVEL
 %token	INCLUDE CERTIFICATE FSYNC CACHE_SIZE INDEX_CACHE_SIZE
 %token	DENY ALLOW READ WRITE BIND ACCESS TO ROOT REFERRAL
 %token	ANY CHILDREN OF ATTRIBUTE IN SUBTREE BY SELF
 %token	<v.string>	STRING
 %token  <v.number>	NUMBER
-%type	<v.number>	port ssl boolean comp_level
+%type	<v.number>	port ssl boolean comp_level legacy protocol
 %type	<v.number>	aci_type aci_access aci_rights aci_right aci_scope
 %type	<v.string>	aci_target aci_attr aci_subject certname
 %type	<v.aci>		aci
@@ -143,12 +143,19 @@ grammar		: /* empty */
 		| grammar schema '\n'
 		;
 
-ssl		: /* empty */			{ $$ = 0; }
+legacy		: /* empty */			{ $$ = 0; }
+		| LEGACY			{ $$ = F_LEGACY; }
+		;
+
+protocol	: /* empty */			{ $$ = 0; }
 		| TLS				{ $$ = F_STARTTLS; }
 		| LDAPS				{ $$ = F_LDAPS; }
 		| SECURE			{ $$ = F_SECURE; }
 		;
 
+ssl		: legacy protocol		{ $$ = $1 | $2; }
+		;
+
 certname	: /* empty */			{ $$ = NULL; }
 		| CERTIFICATE STRING		{ $$ = $2; }
 		;
@@ -181,7 +188,7 @@ conf_main	: LISTEN ON STRING port ssl certname	{
 			char			*cert;
 
 			if ($4 == 0) {
-				if ($5 == F_LDAPS)
+				if ($5 & F_LDAPS)
 					$4 = htons(LDAPS_PORT);
 				else
 					$4 = htons(LDAP_PORT);
@@ -189,8 +196,8 @@ conf_main	: LISTEN ON STRING port ssl certname	{
 
 			cert = ($6 != NULL) ? $6 : $3;
 
-			if (($5 == F_STARTTLS || $5 == F_LDAPS) &&
-			    load_certfile(conf, cert, F_SCERT) < 0) {
+			if (($5 & F_SSL) &&
+			    load_certfile(conf, cert, F_SCERT, $5) < 0) {
 				yyerror("cannot load certificate: %s", cert);
 				free($6);
 				free($3);
@@ -448,6 +455,7 @@ lookup(char *s)
 		{ "index",		INDEX },
 		{ "index-cache-size",	INDEX_CACHE_SIZE },
 		{ "ldaps",		LDAPS },
+		{ "legacy",		LEGACY },
 		{ "level",		LEVEL },
 		{ "listen",		LISTEN },
 		{ "namespace",		NAMESPACE },
@@ -1225,11 +1233,14 @@ ssl_cmp(struct ssl *s1, struct ssl *s2)
 }
 
 int
-load_certfile(struct ldapd_config *env, const char *name, u_int8_t flags)
+load_certfile(struct ldapd_config *env, const char *name, u_int8_t flags,
+    u_int8_t protocol)
 {
 	struct ssl	*s;
 	struct ssl	 key;
 	char		 certfile[PATH_MAX];
+	uint32_t	 tls_protocols = TLS_PROTOCOLS_DEFAULT;
+	const char	*tls_ciphers = "default";
 
 	if (strlcpy(key.ssl_name, name, sizeof(key.ssl_name))
 	    >= sizeof(key.ssl_name)) {
@@ -1253,12 +1264,16 @@ load_certfile(struct ldapd_config *env, const char *name, u_int8_t flags)
 	if (s->config == NULL)
 		goto err;
 
-	if (tls_config_set_protocols(s->config, TLS_PROTOCOLS_ALL) != 0) {
+	if (protocol & F_LEGACY) {
+		tls_protocols = TLS_PROTOCOLS_ALL;
+		tls_ciphers = "all";
+	}
+	if (tls_config_set_protocols(s->config, tls_protocols) != 0) {
 		log_warn("load_certfile: failed to set tls protocols: %s",
 		    tls_config_error(s->config));
 		goto err;
 	}
-	if (tls_config_set_ciphers(s->config, "all")) {
+	if (tls_config_set_ciphers(s->config, tls_ciphers)) {
 		log_warn("load_certfile: failed to set tls ciphers: %s",
 		    tls_config_error(s->config));
 		goto err;