[pve-devel] [PATCH access-control v2] auth ldap/ad: make password a parameter for the api
Instead of simply requiring it to exist in /etc/pve.
Takes after the password handling of CIFS in pve-storage.
Signed-off-by: Dominik Csapak
---
changes from v1:
* use 'realm' dir instead of 'ldap' (with fallback and comment to removal)
PVE/API2/Domains.pm | 26 +++
PVE/Auth/AD.pm | 1 +
PVE/Auth/LDAP.pm| 80 -
PVE/Auth/Plugin.pm | 15 +
4 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm
index 8ae1db0..b25a5fe 100644
--- a/PVE/API2/Domains.pm
+++ b/PVE/API2/Domains.pm
@@ -88,6 +88,9 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
+ # always extract, add it with hook
+ my $password = extract_param($param, 'password');
+
PVE::Auth::Plugin::lock_domain_config(
sub {
@@ -117,6 +120,13 @@ __PACKAGE__->register_method ({
$ids->{$realm} = $config;
+ my $opts = $plugin->options();
+ if (defined($password) && !defined($opts->{password})) {
+ $password = undef;
+ warn "ignoring password parameter";
+ }
+ $plugin->on_add_hook($realm, $config, password => $password);
+
cfs_write_file($domainconfigfile, $cfg);
}, "add auth server failed");
@@ -137,6 +147,9 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
+ # always extract, update in hook
+ my $password = extract_param($param, 'password');
+
PVE::Auth::Plugin::lock_domain_config(
sub {
@@ -154,8 +167,10 @@ __PACKAGE__->register_method ({
my $delete_str = extract_param($param, 'delete');
die "no options specified\n" if !$delete_str && !scalar(keys
%$param);
+ my $delete_pw = 0;
foreach my $opt (PVE::Tools::split_list($delete_str)) {
delete $ids->{$realm}->{$opt};
+ $delete_pw = 1 if $opt eq 'password';
}
my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type});
@@ -171,6 +186,14 @@ __PACKAGE__->register_method ({
$ids->{$realm}->{$p} = $config->{$p};
}
+ my $opts = $plugin->options();
+ if ($delete_pw || ($opts->{password} && defined($password))) {
+ $plugin->on_update_hook($realm, $config, password =>
$password);
+ } else {
+ warn "ignoring password parameter" if defined($password);
+ $plugin->on_update_hook($realm, $config);
+ }
+
cfs_write_file($domainconfigfile, $cfg);
}, "update auth server failed");
@@ -238,6 +261,9 @@ __PACKAGE__->register_method ({
die "domain '$realm' does not exist\n" if !$ids->{$realm};
+ my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type});
+ $plugin->on_delete_hook($realm, $ids->{$realm});
+
delete $ids->{$realm};
cfs_write_file($domainconfigfile, $cfg);
diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
index 4f40be3..24b0e9f 100755
--- a/PVE/Auth/AD.pm
+++ b/PVE/Auth/AD.pm
@@ -83,6 +83,7 @@ sub options {
certkey => { optional => 1 },
base_dn => { optional => 1 },
bind_dn => { optional => 1 },
+ password => { optional => 1 },
user_attr => { optional => 1 },
filter => { optional => 1 },
sync_attributes => { optional => 1 },
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 905cc47..315705e 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -37,6 +37,11 @@ sub properties {
optional => 1,
maxLength => 256,
},
+ password => {
+ description => "LDAP bind password. Will be stored in
'/etc/pve/priv/realm/.pw'.",
+ type => 'string',
+ optional => 1,
+ },
verify => {
description => "Verify the server's SSL certificate",
type => 'boolean',
@@ -126,6 +131,7 @@ sub options {
server2 => { optional => 1 },
base_dn => {},
bind_dn => { optional => 1 },
+ password => { optional => 1 },
user_attr => {},
port => { optional => 1 },
secure => { optional => 1 },
@@ -185,7 +191,7 @@ sub connect_and_bind {
if ($config->{bind_dn}) {
$bind_dn = $config->{bind_dn};
- $bind_pass =
PVE::Tools::file_read_firstline("/etc/pve/priv/ldap/${realm}.pw");
+ $bind_pass = ldap_get_credentials($realm);
die "missing password for realm $realm\n" if !defined($bind_pass);
}
@@ -343,4 +349,76 @@ sub authenticate_user {
return 1;
}
+my $ldap_pw_dir = "/etc/pve/priv/realm";
+
+# check paramter should be removed and pw file should be moved with 7.0
+sub ldap_cred_filename {
+my ($realm, $check) = @_;
+
+
Re: [pve-devel] [PATCH access-control v2] auth ldap/ad: make password a parameter for the api
On 4/7/20 5:02 PM, Thomas Lamprecht wrote:
On 4/7/20 1:11 PM, Dominik Csapak wrote:
Instead of simply requiring it to exist in /etc/pve.
Takes after the password handling of CIFS in pve-storage.
Signed-off-by: Dominik Csapak
---
changes from v1:
* delete pw when given via 'delete' parameter
* do not delete pw when updating without giving 'password' parameter
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 905cc47..1b2c606 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -37,6 +37,11 @@ sub properties {
optional => 1,
maxLength => 256,
},
+ password => {
+ description => "LDAP bind password. Will be stored in
'/etc/pve/priv/ldap/.pw'.",
+ type => 'string',
+ optional => 1,
+ },
verify => {
description => "Verify the server's SSL certificate",
type => 'boolean',
@@ -185,7 +191,7 @@ sub connect_and_bind {
if ($config->{bind_dn}) {
$bind_dn = $config->{bind_dn};
- $bind_pass =
PVE::Tools::file_read_firstline("/etc/pve/priv/ldap/${realm}.pw");
+ $bind_pass = ldap_get_credentials($realm);
die "missing password for realm $realm\n" if !defined($bind_pass);
}
@@ -343,4 +349,69 @@ sub authenticate_user {
return 1;
}
+my $ldap_pw_dir = "/etc/pve/priv/ldap";
+
+sub ldap_cred_filename {
+my ($realm) = @_;
+return "${ldap_pw_dir}/${realm}.pw";
+}
+
looks mostly ok from a quick whiff, albeit I'd like to have the .pw
file in a priv/realm/ directory, ldap is "wrong" here, we also use
priv/storage/ as base directory for CIFS and PBS, not priv/cifs and
priv/pbs .. >
yeah you're right, just wanted to maintain backwards compatibility,
but we can just do it like in storage, where we look in the new place
fall back to the old place (which we can/should remove in 7.0)
i'll send a v2
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH access-control v2] auth ldap/ad: make password a parameter for the api
On 4/7/20 1:11 PM, Dominik Csapak wrote:
> Instead of simply requiring it to exist in /etc/pve.
>
> Takes after the password handling of CIFS in pve-storage.
>
> Signed-off-by: Dominik Csapak
> ---
> changes from v1:
> * delete pw when given via 'delete' parameter
> * do not delete pw when updating without giving 'password' parameter
> diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
> index 905cc47..1b2c606 100755
> --- a/PVE/Auth/LDAP.pm
> +++ b/PVE/Auth/LDAP.pm
> @@ -37,6 +37,11 @@ sub properties {
> optional => 1,
> maxLength => 256,
> },
> + password => {
> + description => "LDAP bind password. Will be stored in
> '/etc/pve/priv/ldap/.pw'.",
> + type => 'string',
> + optional => 1,
> + },
> verify => {
> description => "Verify the server's SSL certificate",
> type => 'boolean',
> @@ -185,7 +191,7 @@ sub connect_and_bind {
>
> if ($config->{bind_dn}) {
> $bind_dn = $config->{bind_dn};
> - $bind_pass =
> PVE::Tools::file_read_firstline("/etc/pve/priv/ldap/${realm}.pw");
> + $bind_pass = ldap_get_credentials($realm);
> die "missing password for realm $realm\n" if !defined($bind_pass);
> }
>
> @@ -343,4 +349,69 @@ sub authenticate_user {
> return 1;
> }
>
> +my $ldap_pw_dir = "/etc/pve/priv/ldap";
> +
> +sub ldap_cred_filename {
> +my ($realm) = @_;
> +return "${ldap_pw_dir}/${realm}.pw";
> +}
> +
looks mostly ok from a quick whiff, albeit I'd like to have the .pw
file in a priv/realm/ directory, ldap is "wrong" here, we also use
priv/storage/ as base directory for CIFS and PBS, not priv/cifs and
priv/pbs ..
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control v2] auth ldap/ad: make password a parameter for the api
Instead of simply requiring it to exist in /etc/pve.
Takes after the password handling of CIFS in pve-storage.
Signed-off-by: Dominik Csapak
---
changes from v1:
* delete pw when given via 'delete' parameter
* do not delete pw when updating without giving 'password' parameter
PVE/API2/Domains.pm | 26
PVE/Auth/AD.pm | 1 +
PVE/Auth/LDAP.pm| 73 -
PVE/Auth/Plugin.pm | 15 ++
4 files changed, 114 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm
index 8ae1db0..b25a5fe 100644
--- a/PVE/API2/Domains.pm
+++ b/PVE/API2/Domains.pm
@@ -88,6 +88,9 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
+ # always extract, add it with hook
+ my $password = extract_param($param, 'password');
+
PVE::Auth::Plugin::lock_domain_config(
sub {
@@ -117,6 +120,13 @@ __PACKAGE__->register_method ({
$ids->{$realm} = $config;
+ my $opts = $plugin->options();
+ if (defined($password) && !defined($opts->{password})) {
+ $password = undef;
+ warn "ignoring password parameter";
+ }
+ $plugin->on_add_hook($realm, $config, password => $password);
+
cfs_write_file($domainconfigfile, $cfg);
}, "add auth server failed");
@@ -137,6 +147,9 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
+ # always extract, update in hook
+ my $password = extract_param($param, 'password');
+
PVE::Auth::Plugin::lock_domain_config(
sub {
@@ -154,8 +167,10 @@ __PACKAGE__->register_method ({
my $delete_str = extract_param($param, 'delete');
die "no options specified\n" if !$delete_str && !scalar(keys
%$param);
+ my $delete_pw = 0;
foreach my $opt (PVE::Tools::split_list($delete_str)) {
delete $ids->{$realm}->{$opt};
+ $delete_pw = 1 if $opt eq 'password';
}
my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type});
@@ -171,6 +186,14 @@ __PACKAGE__->register_method ({
$ids->{$realm}->{$p} = $config->{$p};
}
+ my $opts = $plugin->options();
+ if ($delete_pw || ($opts->{password} && defined($password))) {
+ $plugin->on_update_hook($realm, $config, password =>
$password);
+ } else {
+ warn "ignoring password parameter" if defined($password);
+ $plugin->on_update_hook($realm, $config);
+ }
+
cfs_write_file($domainconfigfile, $cfg);
}, "update auth server failed");
@@ -238,6 +261,9 @@ __PACKAGE__->register_method ({
die "domain '$realm' does not exist\n" if !$ids->{$realm};
+ my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type});
+ $plugin->on_delete_hook($realm, $ids->{$realm});
+
delete $ids->{$realm};
cfs_write_file($domainconfigfile, $cfg);
diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
index 4f40be3..24b0e9f 100755
--- a/PVE/Auth/AD.pm
+++ b/PVE/Auth/AD.pm
@@ -83,6 +83,7 @@ sub options {
certkey => { optional => 1 },
base_dn => { optional => 1 },
bind_dn => { optional => 1 },
+ password => { optional => 1 },
user_attr => { optional => 1 },
filter => { optional => 1 },
sync_attributes => { optional => 1 },
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 905cc47..1b2c606 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -37,6 +37,11 @@ sub properties {
optional => 1,
maxLength => 256,
},
+ password => {
+ description => "LDAP bind password. Will be stored in
'/etc/pve/priv/ldap/.pw'.",
+ type => 'string',
+ optional => 1,
+ },
verify => {
description => "Verify the server's SSL certificate",
type => 'boolean',
@@ -126,6 +131,7 @@ sub options {
server2 => { optional => 1 },
base_dn => {},
bind_dn => { optional => 1 },
+ password => { optional => 1 },
user_attr => {},
port => { optional => 1 },
secure => { optional => 1 },
@@ -185,7 +191,7 @@ sub connect_and_bind {
if ($config->{bind_dn}) {
$bind_dn = $config->{bind_dn};
- $bind_pass =
PVE::Tools::file_read_firstline("/etc/pve/priv/ldap/${realm}.pw");
+ $bind_pass = ldap_get_credentials($realm);
die "missing password for realm $realm\n" if !defined($bind_pass);
}
@@ -343,4 +349,69 @@ sub authenticate_user {
return 1;
}
+my $ldap_pw_dir = "/etc/pve/priv/ldap";
+
+sub ldap_cred_filename {
+my ($realm) = @_;
+return "${ldap_pw_dir}/${realm}.pw";
+}
+
+
