[pve-devel] [PATCH pve-access-control 3/4] api: implement openid API
Fabian Grünbichler
f.gruenbichler at proxmox.com
Tue Jun 29 10:22:26 CEST 2021
On June 24, 2021 10:18 am, Dietmar Maurer wrote:
> This moves compute_api_permission() into RPCEnvironment.pm.
> ---
> src/PVE/API2/AccessControl.pm | 60 ++--------
> src/PVE/API2/Makefile | 3 +-
> src/PVE/API2/OpenId.pm | 214 ++++++++++++++++++++++++++++++++++
> src/PVE/RPCEnvironment.pm | 49 ++++++++
> 4 files changed, 273 insertions(+), 53 deletions(-)
> create mode 100644 src/PVE/API2/OpenId.pm
>
> diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
> index a77694b..6dec66c 100644
> --- a/src/PVE/API2/AccessControl.pm
> +++ b/src/PVE/API2/AccessControl.pm
> @@ -19,9 +19,9 @@ use PVE::API2::User;
> use PVE::API2::Group;
> use PVE::API2::Role;
> use PVE::API2::ACL;
> +use PVE::API2::OpenId;
> use PVE::Auth::Plugin;
> use PVE::OTP;
> -use PVE::Tools;
>
> my $u2f_available = 0;
> eval {
> @@ -56,6 +56,11 @@ __PACKAGE__->register_method ({
> path => 'domains',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PVE::API2::OpenId",
> + path => 'openid',
> +});
> +
> __PACKAGE__->register_method ({
> name => 'index',
> path => '',
> @@ -165,55 +170,6 @@ my $create_ticket = sub {
> };
> };
>
> -my $compute_api_permission = sub {
> - my ($rpcenv, $authuser) = @_;
> -
> - my $usercfg = $rpcenv->{user_cfg};
> -
> - my $res = {};
> - my $priv_re_map = {
> - vms => qr/VM\.|Permissions\.Modify/,
> - access => qr/(User|Group)\.|Permissions\.Modify/,
> - storage => qr/Datastore\.|Permissions\.Modify/,
> - nodes => qr/Sys\.|Permissions\.Modify/,
> - sdn => qr/SDN\.|Permissions\.Modify/,
> - dc => qr/Sys\.Audit|SDN\./,
> - };
> - map { $res->{$_} = {} } keys %$priv_re_map;
> -
> - my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
> -
> - my $checked_paths = {};
> - foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
> - next if $checked_paths->{$path};
> - $checked_paths->{$path} = 1;
> -
> - my $path_perm = $rpcenv->permissions($authuser, $path);
> -
> - my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
> - if ($toplevel eq 'pool') {
> - foreach my $priv (keys %$path_perm) {
> - if ($priv =~ m/^VM\./) {
> - $res->{vms}->{$priv} = 1;
> - } elsif ($priv =~ m/^Datastore\./) {
> - $res->{storage}->{$priv} = 1;
> - } elsif ($priv eq 'Permissions.Modify') {
> - $res->{storage}->{$priv} = 1;
> - $res->{vms}->{$priv} = 1;
> - }
> - }
> - } else {
> - my $priv_regex = $priv_re_map->{$toplevel} // next;
> - foreach my $priv (keys %$path_perm) {
> - next if $priv !~ m/^($priv_regex)/;
> - $res->{$toplevel}->{$priv} = 1;
> - }
> - }
> - }
> -
> - return $res;
> -};
> -
> __PACKAGE__->register_method ({
> name => 'get_ticket',
> path => 'ticket',
> @@ -314,7 +270,7 @@ __PACKAGE__->register_method ({
> die PVE::Exception->new("authentication failure\n", code => 401);
> }
>
> - $res->{cap} = &$compute_api_permission($rpcenv, $username)
> + $res->{cap} = $rpcenv->compute_api_permission($username)
> if !defined($res->{NeedTFA});
>
> my $clinfo = PVE::Cluster::get_clinfo();
> @@ -659,7 +615,7 @@ __PACKAGE__->register_method({
>
> return {
> ticket => PVE::AccessControl::assemble_ticket($authuser),
> - cap => &$compute_api_permission($rpcenv, $authuser),
> + cap => $rpcenv->compute_api_permission($authuser),
> }
> }});
>
> diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
> index 1bf8c05..4e49037 100644
> --- a/src/PVE/API2/Makefile
> +++ b/src/PVE/API2/Makefile
> @@ -5,7 +5,8 @@ API2_SOURCES= \
> ACL.pm \
> Role.pm \
> Group.pm \
> - User.pm
> + User.pm \
> + OpenId.pm
>
> .PHONY: install
> install:
> diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
> new file mode 100644
> index 0000000..db9f9eb
> --- /dev/null
> +++ b/src/PVE/API2/OpenId.pm
> @@ -0,0 +1,214 @@
> +package PVE::API2::OpenId;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools qw(extract_param);
> +use PVE::RS::OpenId;
> +
> +use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
> +use PVE::SafeSyslog;
> +use PVE::RPCEnvironment;
> +use PVE::Cluster qw(cfs_read_file);
> +use PVE::AccessControl;
> +use PVE::JSONSchema qw(get_standard_option);
> +
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +my $openid_state_path = "/var/lib/pve-manager";
> +
> +my $lookup_openid_auth = sub {
> + my ($realm, $redirect_url) = @_;
> +
> + my $cfg = cfs_read_file('domains.cfg');
> + my $ids = $cfg->{ids};
> +
> + die "authentication domain '$realm' does not exist\n" if !$ids->{$realm};
> +
> + my $config = $ids->{$realm};
> + die "wrong realm type ($config->{type} != openid)" if $config->{type} ne "openid";
missing \n
> +
> + my $openid_config = {
> + issuer_url => $config->{'issuer-url'},
> + client_id => $config->{'client-id'},
> + client_key => $config->{'client-key'},
> + };
> +
> + my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
this also potentially dies and does not have the newline appended, maybe
wrap in an eval? (there are probably quite a few of those I missed in
the endpoints below)
also, most likely an error here happens when either of the three
parameters is wrong / not matching what the provider expects. when
testing I got 404 here because I entered the full issuer-url instead of
just the https://FQDN part, maybe we could improve the error handling
story in proxmox-openid-connect-rs by being more explicit which steps
fail, or even detecting common pitfalls like that..
> + return ($config, $openid);
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "Directory index.",
> + permissions => {
> + user => 'all',
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + subdir => { type => 'string' },
> + },
> + },
> + links => [ { rel => 'child', href => "{subdir}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + return [
> + { subdir => 'auth-url' },
> + { subdir => 'login' },
> + ];
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'auth_url',
> + path => 'auth-url',
> + method => 'POST',
> + protected => 1,
> + description => "Get the OpenId Authorization Url for the specified realm.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + realm => get_standard_option('realm'),
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + type => "string",
> + description => "Redirection URL.",
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $realm = extract_param($param, 'realm');
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
> + my $url = $openid->authorize_url($openid_state_path , $realm);
> +
> + return $url;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'login',
> + path => 'login',
> + method => 'POST',
> + protected => 1,
> + description => " Verify OpenID authorization code and create a ticket.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + 'state' => {
> + description => "OpenId state.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + code => {
> + description => "OpenId authorization code.",
> + type => 'string',
> + maxLength => 1024,
> + },
> + 'redirect-url' => {
> + description => "Redirection Url. The client should set this to the used server url (location.origin).",
> + type => 'string',
> + maxLength => 255,
> + },
> + },
> + },
> + returns => {
> + properties => {
> + username => { type => 'string' },
> + ticket => { type => 'string' },
> + CSRFPreventionToken => { type => 'string' },
> + cap => { type => 'object' }, # computed api permissions
> + clustername => { type => 'string', optional => 1 },
> + },
> + },
> + permissions => { user => 'world' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> +
> + my $res;
> + eval {
> + use Data::Dumper;
> +
> + # fixme /tmp
leftover fixme? this is now in /var/lib/pve-manager AFAICT..
> + my ($realm, $private_auth_state) = PVE::RS::OpenId::verify_public_auth_state(
> + $openid_state_path, $param->{'state'});
> +
> + my $redirect_url = extract_param($param, 'redirect-url');
> +
> + my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
> +
> + my $info = $openid->verify_authorization_code($param->{code}, $private_auth_state);
> + my $subject = $info->{'sub'};
> +
> + die "missing openid claim 'sub'" if !defined($subject);
missing \n
> +
> + my $unique_name = $subject; # default
> + if (defined(my $user_attr = $config->{'user-attr'})) {
> + if ($user_attr eq 'subject') {
> + $unique_name = $subject;
> + } elsif ($user_attr eq 'username') {
> + my $username = $info->{'preferred_username'};
> + die "missing claim 'preferred_username'" if !defined($username);
same
> + $unique_name = $username;
> + } elsif ($user_attr eq 'email') {
> + my $email = $info->{'email'};
> + die "missing claim 'email'" if !defined($email);
same
> + $unique_name = $email;
> + } else {
> + die "got unexpected value for user_attr '${user_attr}'";
same
> + }
> + }
> +
> + my $username = "${unique_name}\@${realm}";
> +
> + # test if user exists and is enabled
> + $rpcenv->check_user_enabled($username);
missing check for user being expired
> +
> + my $ticket = PVE::AccessControl::assemble_ticket($username);
> + my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
> + my $cap = $rpcenv->compute_api_permission($username);
> +
> + $res = {
> + ticket => $ticket,
> + username => $username,
> + CSRFPreventionToken => $csrftoken,
> + cap => $cap,
> + };
> +
> + my $clinfo = PVE::Cluster::get_clinfo();
> + if ($clinfo->{cluster}->{name} && $rpcenv->check($username, '/', ['Sys.Audit'], 1)) {
> + $res->{clustername} = $clinfo->{cluster}->{name};
> + }
> + };
> + if (my $err = $@) {
> + my $clientip = $rpcenv->get_client_ip() || '';
> + syslog('err', "openid authentication failure; rhost=$clientip msg=$err");
> + # do not return any info to prevent user enumeration attacks
> + die PVE::Exception->new("authentication failure\n", code => 401);
> + }
> +
> + PVE::Cluster::log_msg('info', 'root at pam', "successful openid auth for user '$res->{username}'");
> +
> + return $res;
> + }});
> diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
> index 2e5a33b..8aae094 100644
> --- a/src/PVE/RPCEnvironment.pm
> +++ b/src/PVE/RPCEnvironment.pm
> @@ -126,6 +126,55 @@ sub permissions {
> return &$compile_acl_path($self, $user, $path);
> }
>
> +sub compute_api_permission {
> + my ($self, $authuser) = @_;
> +
> + my $usercfg = $self->{user_cfg};
> +
> + my $res = {};
> + my $priv_re_map = {
> + vms => qr/VM\.|Permissions\.Modify/,
> + access => qr/(User|Group)\.|Permissions\.Modify/,
> + storage => qr/Datastore\.|Permissions\.Modify/,
> + nodes => qr/Sys\.|Permissions\.Modify/,
> + sdn => qr/SDN\.|Permissions\.Modify/,
> + dc => qr/Sys\.Audit|SDN\./,
> + };
> + map { $res->{$_} = {} } keys %$priv_re_map;
> +
> + my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
> +
> + my $checked_paths = {};
> + foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
> + next if $checked_paths->{$path};
> + $checked_paths->{$path} = 1;
> +
> + my $path_perm = $self->permissions($authuser, $path);
> +
> + my $toplevel = ($path =~ /^\/(\w+)/) ? $1 : 'dc';
> + if ($toplevel eq 'pool') {
> + foreach my $priv (keys %$path_perm) {
> + if ($priv =~ m/^VM\./) {
> + $res->{vms}->{$priv} = 1;
> + } elsif ($priv =~ m/^Datastore\./) {
> + $res->{storage}->{$priv} = 1;
> + } elsif ($priv eq 'Permissions.Modify') {
> + $res->{storage}->{$priv} = 1;
> + $res->{vms}->{$priv} = 1;
> + }
> + }
> + } else {
> + my $priv_regex = $priv_re_map->{$toplevel} // next;
> + foreach my $priv (keys %$path_perm) {
> + next if $priv !~ m/^($priv_regex)/;
> + $res->{$toplevel}->{$priv} = 1;
> + }
> + }
> + }
> +
> + return $res;
> +}
> +
> sub get_effective_permissions {
> my ($self, $user) = @_;
>
> --
> 2.30.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list