Re: ypldap TLS by default

2022-10-13 Thread Klemens Nanni
On Thu, Oct 13, 2022 at 10:11:39PM +1000, Jonathan Matthew wrote:
> While working on ypconnect(2), Theo suggested that ypldap(8) should
> not default to plaintext LDAP connections, since the data it's dealing with
> is pretty important to the security of the system.  Here's a straightforward
> diff implementing that, defaulting to what was previously called 'tls'
> (STARTTLS on port 389), and adding a 'notls' option for plaintext.
> 
> ok?  other opinions on what this option should be called?

Given TLS and STARTTLS, "plain" might be a tad clearer as "notls" could
still be understood as no-TLS-but-try-STARTTLS.

Given the existing "tls" and "ldaps" keywords, I thnk "ldap" would be
most clear and consistent.


> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/ypldap/parse.y,v
> retrieving revision 1.36
> diff -u -p -u -p -r1.36 parse.y
> --- parse.y   13 Oct 2022 04:55:33 -  1.36
> +++ parse.y   13 Oct 2022 08:13:21 -
> @@ -107,7 +107,7 @@ typedef struct {
>  %token   SERVER FILTER ATTRIBUTE BASEDN BINDDN GROUPDN BINDCRED MAPS 
> CHANGE DOMAIN PROVIDE
>  %token   USER GROUP TO EXPIRE HOME SHELL GECOS UID GID INTERVAL
>  %token   PASSWD NAME FIXED LIST GROUPNAME GROUPPASSWD GROUPGID MAP
> -%token   INCLUDE DIRECTORY CLASS PORT ERROR GROUPMEMBERS LDAPS TLS CAFILE
> +%token   INCLUDE DIRECTORY CLASS PORT ERROR GROUPMEMBERS LDAPS TLS NOTLS 
> CAFILE
>  %token   BIND LOCAL PORTMAP BINDEXT CERTFILE KEYFILE
>  %token STRING
>  %token NUMBER
> @@ -366,9 +366,10 @@ diropt   : BINDDN STRING 
> {
>   }
>   ;
>  
> -ssl  : /* empty */   { $$ = 0; }
> +ssl  : /* empty */   { $$ = F_STARTTLS; }
>   | LDAPS { $$ = F_SSL; }
>   | TLS   { $$ = F_STARTTLS; }
> + | NOTLS { $$ = 0; }
>   ;
>  
>  directory: DIRECTORY STRING port ssl {
> @@ -556,6 +557,7 @@ lookup(char *s)
>   { "map",MAP },
>   { "maps",   MAPS },
>   { "name",   NAME },
> + { "notls",  NOTLS },
>   { "passwd", PASSWD },
>   { "port",   PORT },
>   { "portmap",PORTMAP },
> Index: ypldap.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ypldap/ypldap.conf.5,v
> retrieving revision 1.28
> diff -u -p -u -p -r1.28 ypldap.conf.5
> --- ypldap.conf.5 13 Oct 2022 04:55:33 -  1.28
> +++ ypldap.conf.5 13 Oct 2022 08:13:21 -
> @@ -119,15 +119,19 @@ directory are used to construct YP map e
>  .Bl -tag -width Ds
>  .It Ic directory Ar hostname Oo Ic port Ar port Oc Oo tls Oc Brq ...
>  Defines a directory by hostname and optionally port number.
> -If the
> +The
>  .Ar tls
> -argument is not specified, no transport-level security will be used.
> +argument specifies the transport-level security used for the connection.
>  Valid options are:
>  .Bl -tag -width Ds
>  .It Ic tls
> -Use STARTTLS to negotiate TLS, by default on port 389.
> +Use STARTTLS to negotiate TLS on port 389 unless an alternate port is
> +specified.
> +This is the default.
>  .It Ic ldaps
> -Connect with TLS enabled, by default on port 636.
> +Connect with TLS enabled on port 636 unless an alternate port is specified.
> +.It Ic notls
> +Connect with no transport-level security.
>  .El
>  .El
>  .Pp
> 



ypldap TLS by default

2022-10-13 Thread Jonathan Matthew
While working on ypconnect(2), Theo suggested that ypldap(8) should
not default to plaintext LDAP connections, since the data it's dealing with
is pretty important to the security of the system.  Here's a straightforward
diff implementing that, defaulting to what was previously called 'tls'
(STARTTLS on port 389), and adding a 'notls' option for plaintext.

ok?  other opinions on what this option should be called?

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ypldap/parse.y,v
retrieving revision 1.36
diff -u -p -u -p -r1.36 parse.y
--- parse.y 13 Oct 2022 04:55:33 -  1.36
+++ parse.y 13 Oct 2022 08:13:21 -
@@ -107,7 +107,7 @@ typedef struct {
 %token SERVER FILTER ATTRIBUTE BASEDN BINDDN GROUPDN BINDCRED MAPS CHANGE 
DOMAIN PROVIDE
 %token USER GROUP TO EXPIRE HOME SHELL GECOS UID GID INTERVAL
 %token PASSWD NAME FIXED LIST GROUPNAME GROUPPASSWD GROUPGID MAP
-%token INCLUDE DIRECTORY CLASS PORT ERROR GROUPMEMBERS LDAPS TLS CAFILE
+%token INCLUDE DIRECTORY CLASS PORT ERROR GROUPMEMBERS LDAPS TLS NOTLS CAFILE
 %token BIND LOCAL PORTMAP BINDEXT CERTFILE KEYFILE
 %token   STRING
 %token   NUMBER
@@ -366,9 +366,10 @@ diropt : BINDDN STRING 
{
}
;
 
-ssl: /* empty */   { $$ = 0; }
+ssl: /* empty */   { $$ = F_STARTTLS; }
| LDAPS { $$ = F_SSL; }
| TLS   { $$ = F_STARTTLS; }
+   | NOTLS { $$ = 0; }
;
 
 directory  : DIRECTORY STRING port ssl {
@@ -556,6 +557,7 @@ lookup(char *s)
{ "map",MAP },
{ "maps",   MAPS },
{ "name",   NAME },
+   { "notls",  NOTLS },
{ "passwd", PASSWD },
{ "port",   PORT },
{ "portmap",PORTMAP },
Index: ypldap.conf.5
===
RCS file: /cvs/src/usr.sbin/ypldap/ypldap.conf.5,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 ypldap.conf.5
--- ypldap.conf.5   13 Oct 2022 04:55:33 -  1.28
+++ ypldap.conf.5   13 Oct 2022 08:13:21 -
@@ -119,15 +119,19 @@ directory are used to construct YP map e
 .Bl -tag -width Ds
 .It Ic directory Ar hostname Oo Ic port Ar port Oc Oo tls Oc Brq ...
 Defines a directory by hostname and optionally port number.
-If the
+The
 .Ar tls
-argument is not specified, no transport-level security will be used.
+argument specifies the transport-level security used for the connection.
 Valid options are:
 .Bl -tag -width Ds
 .It Ic tls
-Use STARTTLS to negotiate TLS, by default on port 389.
+Use STARTTLS to negotiate TLS on port 389 unless an alternate port is
+specified.
+This is the default.
 .It Ic ldaps
-Connect with TLS enabled, by default on port 636.
+Connect with TLS enabled on port 636 unless an alternate port is specified.
+.It Ic notls
+Connect with no transport-level security.
 .El
 .El
 .Pp