From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A706F716C1 for ; Tue, 29 Jun 2021 10:22:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9ADEEB449 for ; Tue, 29 Jun 2021 10:22:35 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 5D493B43E for ; Tue, 29 Jun 2021 10:22:34 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2ACA04094D for ; Tue, 29 Jun 2021 10:22:34 +0200 (CEST) Date: Tue, 29 Jun 2021 10:22:26 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210624081802.2090614-1-dietmar@proxmox.com> <20210624081802.2090614-4-dietmar@proxmox.com> In-Reply-To: <20210624081802.2090614-4-dietmar@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1624954399.k1a5fmdqsn.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.560 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [openid.pm, group.pm, rpcenvironment.pm, role.pm, proxmox.com, user.pm, accesscontrol.pm, acl.pm] Subject: Re: [pve-devel] [PATCH pve-access-control 3/4] api: implement openid API X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jun 2021 08:22:35 -0000 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 >=20 > diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.p= m > 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; > =20 > my $u2f_available =3D 0; > eval { > @@ -56,6 +56,11 @@ __PACKAGE__->register_method ({ > path =3D> 'domains', > }); > =20 > +__PACKAGE__->register_method ({ > + subclass =3D> "PVE::API2::OpenId", > + path =3D> 'openid', > +}); > + > __PACKAGE__->register_method ({ > name =3D> 'index', > path =3D> '', > @@ -165,55 +170,6 @@ my $create_ticket =3D sub { > }; > }; > =20 > -my $compute_api_permission =3D sub { > - my ($rpcenv, $authuser) =3D @_; > - > - my $usercfg =3D $rpcenv->{user_cfg}; > - > - my $res =3D {}; > - my $priv_re_map =3D { > - vms =3D> qr/VM\.|Permissions\.Modify/, > - access =3D> qr/(User|Group)\.|Permissions\.Modify/, > - storage =3D> qr/Datastore\.|Permissions\.Modify/, > - nodes =3D> qr/Sys\.|Permissions\.Modify/, > - sdn =3D> qr/SDN\.|Permissions\.Modify/, > - dc =3D> qr/Sys\.Audit|SDN\./, > - }; > - map { $res->{$_} =3D {} } keys %$priv_re_map; > - > - my $required_paths =3D ['/', '/nodes', '/access/groups', '/vms', '/s= torage', '/sdn']; > - > - my $checked_paths =3D {}; > - foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) { > - next if $checked_paths->{$path}; > - $checked_paths->{$path} =3D 1; > - > - my $path_perm =3D $rpcenv->permissions($authuser, $path); > - > - my $toplevel =3D ($path =3D~ /^\/(\w+)/) ? $1 : 'dc'; > - if ($toplevel eq 'pool') { > - foreach my $priv (keys %$path_perm) { > - if ($priv =3D~ m/^VM\./) { > - $res->{vms}->{$priv} =3D 1; > - } elsif ($priv =3D~ m/^Datastore\./) { > - $res->{storage}->{$priv} =3D 1; > - } elsif ($priv eq 'Permissions.Modify') { > - $res->{storage}->{$priv} =3D 1; > - $res->{vms}->{$priv} =3D 1; > - } > - } > - } else { > - my $priv_regex =3D $priv_re_map->{$toplevel} // next; > - foreach my $priv (keys %$path_perm) { > - next if $priv !~ m/^($priv_regex)/; > - $res->{$toplevel}->{$priv} =3D 1; > - } > - } > - } > - > - return $res; > -}; > - > __PACKAGE__->register_method ({ > name =3D> 'get_ticket', > path =3D> 'ticket', > @@ -314,7 +270,7 @@ __PACKAGE__->register_method ({ > die PVE::Exception->new("authentication failure\n", code =3D> 401); > } > =20 > - $res->{cap} =3D &$compute_api_permission($rpcenv, $username) > + $res->{cap} =3D $rpcenv->compute_api_permission($username) > if !defined($res->{NeedTFA}); > =20 > my $clinfo =3D PVE::Cluster::get_clinfo(); > @@ -659,7 +615,7 @@ __PACKAGE__->register_method({ > =20 > return { > ticket =3D> PVE::AccessControl::assemble_ticket($authuser), > - cap =3D> &$compute_api_permission($rpcenv, $authuser), > + cap =3D> $rpcenv->compute_api_permission($authuser), > } > }}); > =20 > 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=3D \ > ACL.pm \ > Role.pm \ > Group.pm \ > - User.pm > + User.pm \ > + OpenId.pm > =20 > .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 =3D "/var/lib/pve-manager"; > + > +my $lookup_openid_auth =3D sub { > + my ($realm, $redirect_url) =3D @_; > + > + my $cfg =3D cfs_read_file('domains.cfg'); > + my $ids =3D $cfg->{ids}; > + > + die "authentication domain '$realm' does not exist\n" if !$ids->{$re= alm}; > + > + my $config =3D $ids->{$realm}; > + die "wrong realm type ($config->{type} !=3D openid)" if $config->{ty= pe} ne "openid"; missing \n > + > + my $openid_config =3D { > + issuer_url =3D> $config->{'issuer-url'}, > + client_id =3D> $config->{'client-id'}, > + client_key =3D> $config->{'client-key'}, > + }; > + > + my $openid =3D PVE::RS::OpenId->discover($openid_config, $redirect_u= rl); 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=20 the endpoints below) also, most likely an error here happens when either of the three=20 parameters is wrong / not matching what the provider expects. when=20 testing I got 404 here because I entered the full issuer-url instead of=20 just the https://FQDN part, maybe we could improve the error handling=20 story in proxmox-openid-connect-rs by being more explicit which steps=20 fail, or even detecting common pitfalls like that.. > + return ($config, $openid); > +}; > + > +__PACKAGE__->register_method ({ > + name =3D> 'index', > + path =3D> '', > + method =3D> 'GET', > + description =3D> "Directory index.", > + permissions =3D> { > + user =3D> 'all', > + }, > + parameters =3D> { > + additionalProperties =3D> 0, > + properties =3D> {}, > + }, > + returns =3D> { > + type =3D> 'array', > + items =3D> { > + type =3D> "object", > + properties =3D> { > + subdir =3D> { type =3D> 'string' }, > + }, > + }, > + links =3D> [ { rel =3D> 'child', href =3D> "{subdir}" } ], > + }, > + code =3D> sub { > + my ($param) =3D @_; > + > + return [ > + { subdir =3D> 'auth-url' }, > + { subdir =3D> 'login' }, > + ]; > + }}); > + > +__PACKAGE__->register_method ({ > + name =3D> 'auth_url', > + path =3D> 'auth-url', > + method =3D> 'POST', > + protected =3D> 1, > + description =3D> "Get the OpenId Authorization Url for the specified= realm.", > + parameters =3D> { > + additionalProperties =3D> 0, > + properties =3D> { > + realm =3D> get_standard_option('realm'), > + 'redirect-url' =3D> { > + description =3D> "Redirection Url. The client should set this to the u= sed server url (location.origin).", > + type =3D> 'string', > + maxLength =3D> 255, > + }, > + }, > + }, > + returns =3D> { > + type =3D> "string", > + description =3D> "Redirection URL.", > + }, > + permissions =3D> { user =3D> 'world' }, > + code =3D> sub { > + my ($param) =3D @_; > + > + my $realm =3D extract_param($param, 'realm'); > + my $redirect_url =3D extract_param($param, 'redirect-url'); > + > + my ($config, $openid) =3D $lookup_openid_auth->($realm, $redirect_url); > + my $url =3D $openid->authorize_url($openid_state_path , $realm); > + > + return $url; > + }}); > + > +__PACKAGE__->register_method ({ > + name =3D> 'login', > + path =3D> 'login', > + method =3D> 'POST', > + protected =3D> 1, > + description =3D> " Verify OpenID authorization code and create a tic= ket.", > + parameters =3D> { > + additionalProperties =3D> 0, > + properties =3D> { > + 'state' =3D> { > + description =3D> "OpenId state.", > + type =3D> 'string', > + maxLength =3D> 1024, > + }, > + code =3D> { > + description =3D> "OpenId authorization code.", > + type =3D> 'string', > + maxLength =3D> 1024, > + }, > + 'redirect-url' =3D> { > + description =3D> "Redirection Url. The client should set this to the u= sed server url (location.origin).", > + type =3D> 'string', > + maxLength =3D> 255, > + }, > + }, > + }, > + returns =3D> { > + properties =3D> { > + username =3D> { type =3D> 'string' }, > + ticket =3D> { type =3D> 'string' }, > + CSRFPreventionToken =3D> { type =3D> 'string' }, > + cap =3D> { type =3D> 'object' }, # computed api permissions > + clustername =3D> { type =3D> 'string', optional =3D> 1 }, > + }, > + }, > + permissions =3D> { user =3D> 'world' }, > + code =3D> sub { > + my ($param) =3D @_; > + > + my $rpcenv =3D 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) =3D PVE::RS::OpenId::verify_public= _auth_state( > + $openid_state_path, $param->{'state'}); > + > + my $redirect_url =3D extract_param($param, 'redirect-url'); > + > + my ($config, $openid) =3D $lookup_openid_auth->($realm, $redirect_u= rl); > + > + my $info =3D $openid->verify_authorization_code($param->{code}, $pr= ivate_auth_state); > + my $subject =3D $info->{'sub'}; > + > + die "missing openid claim 'sub'" if !defined($subject); missing \n > + > + my $unique_name =3D $subject; # default > + if (defined(my $user_attr =3D $config->{'user-attr'})) { > + if ($user_attr eq 'subject') { > + $unique_name =3D $subject; > + } elsif ($user_attr eq 'username') { > + my $username =3D $info->{'preferred_username'}; > + die "missing claim 'preferred_username'" if !defined($username); same > + $unique_name =3D $username; > + } elsif ($user_attr eq 'email') { > + my $email =3D $info->{'email'}; > + die "missing claim 'email'" if !defined($email); same > + $unique_name =3D $email; > + } else { > + die "got unexpected value for user_attr '${user_attr}'"; same > + } > + } > + > + my $username =3D "${unique_name}\@${realm}"; > + > + # test if user exists and is enabled > + $rpcenv->check_user_enabled($username); missing check for user being expired > + > + my $ticket =3D PVE::AccessControl::assemble_ticket($username); > + my $csrftoken =3D PVE::AccessControl::assemble_csrf_prevention_toke= n($username); > + my $cap =3D $rpcenv->compute_api_permission($username); > + > + $res =3D { > + ticket =3D> $ticket, > + username =3D> $username, > + CSRFPreventionToken =3D> $csrftoken, > + cap =3D> $cap, > + }; > + > + my $clinfo =3D PVE::Cluster::get_clinfo(); > + if ($clinfo->{cluster}->{name} && $rpcenv->check($username, '/', ['= Sys.Audit'], 1)) { > + $res->{clustername} =3D $clinfo->{cluster}->{name}; > + } > + }; > + if (my $err =3D $@) { > + my $clientip =3D $rpcenv->get_client_ip() || ''; > + syslog('err', "openid authentication failure; rhost=3D$clientip msg= =3D$err"); > + # do not return any info to prevent user enumeration attacks > + die PVE::Exception->new("authentication failure\n", code =3D> 401); > + } > + > + PVE::Cluster::log_msg('info', 'root@pam', "successful openid auth for u= ser '$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); > } > =20 > +sub compute_api_permission { > + my ($self, $authuser) =3D @_; > + > + my $usercfg =3D $self->{user_cfg}; > + > + my $res =3D {}; > + my $priv_re_map =3D { > + vms =3D> qr/VM\.|Permissions\.Modify/, > + access =3D> qr/(User|Group)\.|Permissions\.Modify/, > + storage =3D> qr/Datastore\.|Permissions\.Modify/, > + nodes =3D> qr/Sys\.|Permissions\.Modify/, > + sdn =3D> qr/SDN\.|Permissions\.Modify/, > + dc =3D> qr/Sys\.Audit|SDN\./, > + }; > + map { $res->{$_} =3D {} } keys %$priv_re_map; > + > + my $required_paths =3D ['/', '/nodes', '/access/groups', '/vms', '/s= torage', '/sdn']; > + > + my $checked_paths =3D {}; > + foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) { > + next if $checked_paths->{$path}; > + $checked_paths->{$path} =3D 1; > + > + my $path_perm =3D $self->permissions($authuser, $path); > + > + my $toplevel =3D ($path =3D~ /^\/(\w+)/) ? $1 : 'dc'; > + if ($toplevel eq 'pool') { > + foreach my $priv (keys %$path_perm) { > + if ($priv =3D~ m/^VM\./) { > + $res->{vms}->{$priv} =3D 1; > + } elsif ($priv =3D~ m/^Datastore\./) { > + $res->{storage}->{$priv} =3D 1; > + } elsif ($priv eq 'Permissions.Modify') { > + $res->{storage}->{$priv} =3D 1; > + $res->{vms}->{$priv} =3D 1; > + } > + } > + } else { > + my $priv_regex =3D $priv_re_map->{$toplevel} // next; > + foreach my $priv (keys %$path_perm) { > + next if $priv !~ m/^($priv_regex)/; > + $res->{$toplevel}->{$priv} =3D 1; > + } > + } > + } > + > + return $res; > +} > + > sub get_effective_permissions { > my ($self, $user) =3D @_; > =20 > --=20 > 2.30.2 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20