[pve-devel] [PATCH access-control v2 2/3] auth ldap/ad: introduce connection 'mode'

[Dominik Csapak]

instead of having only a 'secure' flag which switches between
ldap/ldaps we now have a mode which also contains 'ldap+starttls'

our connection code in PVE::LDAP can handle this already (used in pmg)
so that is no problem

if we want to really remove the 'secure' flag, e.g. in 7.0
we'd either have to rewrite the config or have it as an error
in a pve6to7 script

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
changes from v1:
* refactor get_scheme_and_port

 PVE/Auth/AD.pm   |  9 ++++-----
 PVE/Auth/LDAP.pm | 25 +++++++++++++++++++++----
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
index 24b0e9f..4d64c20 100755
--- a/PVE/Auth/AD.pm
+++ b/PVE/Auth/AD.pm
@@ -27,7 +27,7 @@ sub properties {
 	    maxLength => 256,
 	},
 	secure => {
-	    description => "Use secure LDAPS protocol.",
+	    description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.",
 	    type => 'boolean',
 	    optional => 1,
 	},
@@ -93,6 +93,7 @@ sub options {
 	group_filter => { optional => 1 },
 	group_classes => { optional => 1 },
 	'sync-defaults-options' => { optional => 1 },
+	mode => { optional => 1 },
     };
 }
 
@@ -110,9 +111,7 @@ sub authenticate_user {
     my $servers = [$config->{server1}];
     push @$servers, $config->{server2} if $config->{server2};
 
-    my $default_port = $config->{secure} ? 636: 389;
-    my $port = $config->{port} // $default_port;
-    my $scheme = $config->{secure} ? 'ldaps' : 'ldap';
+    my ($scheme, $port) = $class->get_scheme_and_port($config);
 
     my %ad_args;
     if ($config->{verify}) {
@@ -130,7 +129,7 @@ sub authenticate_user {
 	$ad_args{verify} = 'none';
     }
 
-    if ($config->{secure}) {
+    if ($scheme ne 'ldap') {
 	$ad_args{sslversion} = $config->{sslversion} // 'tlsv1_2';
     }
 
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 6b6b184..64250cb 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -122,6 +122,13 @@ sub properties {
 	    format => 'realm-sync-options',
 	    optional => 1,
 	},
+	mode => {
+	    description => "LDAP protocol mode.",
+	    type => 'string',
+	    enum => [ 'ldap', 'ldaps', 'ldap+starttls'],
+	    optional => 1,
+	    default => 'ldap',
+	},
     };
 }
 
@@ -151,18 +158,28 @@ sub options {
 	group_filter => { optional => 1 },
 	group_classes => { optional => 1 },
 	'sync-defaults-options' => { optional => 1 },
+	mode => { optional => 1 },
     };
 }
 
+sub get_scheme_and_port {
+    my ($class, $config) = @_;
+
+    my $scheme = $config->{mode} // ($config->{secure} ? 'ldaps' : 'ldap');
+
+    my $default_port = $scheme eq 'ldaps' ? 636 : 389;
+    my $port = $config->{port} // $default_port;
+
+    return ($scheme, $port);
+}
+
 sub connect_and_bind {
     my ($class, $config, $realm) = @_;
 
     my $servers = [$config->{server1}];
     push @$servers, $config->{server2} if $config->{server2};
 
-    my $default_port = $config->{secure} ? 636: 389;
-    my $port = $config->{port} // $default_port;
-    my $scheme = $config->{secure} ? 'ldaps' : 'ldap';
+    my ($scheme, $port) = $class->get_scheme_and_port($config);
 
     my %ldap_args;
     if ($config->{verify}) {
@@ -180,7 +197,7 @@ sub connect_and_bind {
 	$ldap_args{verify} = 'none';
     }
 
-    if ($config->{secure}) {
+    if ($scheme ne 'ldap') {
 	$ldap_args{sslversion} = $config->{sslversion} || 'tlsv1_2';
     }
 
-- 
2.20.1