[pmg-devel] [PATCH pmg-api 0/4] cleanups for OIDC patch-series

[pmg-devel] [PATCH pmg-api 0/4] cleanups for OIDC patch-series

[Stoiko Ivanov]

the following patches address a few issues that were reported on-list and
off-list for the OIDC patch series and the rework there.

I did run some isolated minimal tests, but another set of eyes and testers
would not harm

Stoiko Ivanov (4):
  api: authrealm: create: add quarantine as reserved realm
  utils: drop unused variable assignment
  utils: user schema: explicitly forbid @ in user-names
  api: users: rework userid validation

 src/PMG/API2/AuthRealm.pm |  2 +-
 src/PMG/API2/Users.pm     | 16 ++++++++++++----
 src/PMG/Utils.pm          |  4 +---
 3 files changed, 14 insertions(+), 8 deletions(-)

-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 1/4] api: authrealm: create: add quarantine as reserved realm

[Stoiko Ivanov]

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/AuthRealm.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/API2/AuthRealm.pm b/src/PMG/API2/AuthRealm.pm
index e9fee38..4851582 100644
--- a/src/PMG/API2/AuthRealm.pm
+++ b/src/PMG/API2/AuthRealm.pm
@@ -94,7 +94,7 @@ __PACKAGE__->register_method ({
 		    if $ids->{$realm};
 
 		die "unable to use reserved name '$realm'\n"
-		    if ($realm eq 'pam' || $realm eq 'pmg');
+		    if ($realm eq 'pam' || $realm eq 'pmg' || $realm eq 'quarantine');
 
 		die "unable to create builtin type '$type'\n"
 		    if ($type eq 'pam' || $type eq 'pmg');
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 2/4] utils: drop unused variable assignment

[Stoiko Ivanov]

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Utils.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 33f80ae..9a50de2 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -54,7 +54,6 @@ my $user_regex = qr![^\s:/]+!;
 
 sub valid_pmg_realm_regex {
     my $cfg = PVE::INotify::read_file(PMG::Auth::Plugin::realm_conf_id());
-    my $ids = $cfg->{ids};
     my $realms = ['pam', 'quarantine', sort keys $cfg->{ids}->%* ];
     return join('|', @$realms);
 }
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 3/4] utils: user schema: explicitly forbid @ in user-names

[Stoiko Ivanov]

PMGs terms are:
* 'userid' consists of 'username'@'realm'

without this patch it was possible to create a user through the api,
with @ in the username ('foo@bar@pmg'), and it got written to the
user-conf.
Reading that entry was not possible, as the verification on read was
stricter.

This patch forbids '@' in usernames, and additionally drops the
maxLength of 64, as 60 are already enforced by the regex pattern match
(leaving 4 as minimal length for '@pmg'/'@pam').

Potential for regression should be minimal (the users could not be
read-back from the config).

Reported-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Utils.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 9a50de2..7e4b70b 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -50,7 +50,7 @@ postgres_admin_cmd
 try_decode_utf8
 );
 
-my $user_regex = qr![^\s:/]+!;
+my $user_regex = qr![^\s:@/]+!;
 
 sub valid_pmg_realm_regex {
     my $cfg = PVE::INotify::read_file(PMG::Auth::Plugin::realm_conf_id());
@@ -110,7 +110,6 @@ PVE::JSONSchema::register_standard_option('username', {
     description => "Username (without realm)",
     type => 'string',
     pattern => '[^\s:\/\@]{1,60}',
-    maxLength => 64,
 });
 
 PVE::JSONSchema::register_standard_option('pmg-email-address', {
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 4/4] api: users: rework userid validation

[Stoiko Ivanov]

reuse the existing helpers in PMG::Utils to forbid creating:
* users with an unallowed realm
* passing a different realm than what we receive in the userid
* users in the pam realm

Reported-by: Mira Limbeck <m.limbeck@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/Users.pm | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/PMG/API2/Users.pm b/src/PMG/API2/Users.pm
index f137b87..1edf29a 100644
--- a/src/PMG/API2/Users.pm
+++ b/src/PMG/API2/Users.pm
@@ -111,9 +111,6 @@ __PACKAGE__->register_method ({
 	    die "User '$param->{userid}' already exists\n"
 		if $cfg->{$param->{userid}};
 
-	    die "Currently you cannot create user in the PAM realm\n"
-		if $param->{realm} && $param->{realm} eq 'pam';
-
 	    my $entry = {};
 	    foreach my $k (keys %$param) {
 		my $v = $param->{$k};
@@ -124,10 +121,21 @@ __PACKAGE__->register_method ({
 		}
 	    }
 
+	    my ($userid, $username, $realm) = PMG::Utils::verify_username($entry->{userid});
+	    my $realm_regex = PMG::Utils::valid_pmg_realm_regex();
+	    die "invalid realm in userid\n" if $realm !~ /$realm_regex/;
+
+	    if ($entry->{realm}) {
+		die "realm parameter does not fit userid\n" if ($entry->{realm} ne $realm);
+	    } else {
+		$entry->{realm} = $realm;
+	    }
+
+	    die "Currently you cannot create user in the PAM realm\n" if $entry->{realm} eq 'pam';
+
 	    $entry->{enable} //= 0;
 	    $entry->{expire} //= 0;
 	    $entry->{role} //= 'audit';
-	    $entry->{realm} //= 'pmg';
 
 	    $cfg->{$param->{userid}} = $entry;
 
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api 0/4] cleanups for OIDC patch-series

[Stoiko Ivanov]

On Wed, 26 Feb 2025 20:16:58 +0100
Stoiko Ivanov <s.ivanov@proxmox.com> wrote:

> the following patches address a few issues that were reported on-list and
> off-list for the OIDC patch series and the rework there.
forgot to mention that this is based on the rename realms.cfg to
realms.conf series:
https://lore.proxmox.com/pmg-devel/20250226173737.577869-1-s.ivanov@proxmox.com/T/#t

> 
> I did run some isolated minimal tests, but another set of eyes and testers
> would not harm
> 
> Stoiko Ivanov (4):
>   api: authrealm: create: add quarantine as reserved realm
>   utils: drop unused variable assignment
>   utils: user schema: explicitly forbid @ in user-names
>   api: users: rework userid validation
> 
>  src/PMG/API2/AuthRealm.pm |  2 +-
>  src/PMG/API2/Users.pm     | 16 ++++++++++++----
>  src/PMG/Utils.pm          |  4 +---
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH pmg-api 1/4] api: authrealm: create: add quarantine as reserved realm

[Thomas Lamprecht]

Am 26.02.25 um 20:16 schrieb Stoiko Ivanov:
> Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/API2/AuthRealm.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api 2/4] utils: drop unused variable assignment

[Thomas Lamprecht]

Am 26.02.25 um 20:17 schrieb Stoiko Ivanov:
> Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Utils.pm | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 33f80ae..9a50de2 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -54,7 +54,6 @@ my $user_regex = qr![^\s:/]+!;
>  
>  sub valid_pmg_realm_regex {
>      my $cfg = PVE::INotify::read_file(PMG::Auth::Plugin::realm_conf_id());
> -    my $ids = $cfg->{ids};
>      my $realms = ['pam', 'quarantine', sort keys $cfg->{ids}->%* ];
>      return join('|', @$realms);
>  }

this is supererseded as I moved the method and cleaned it, and its usage, up.


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api 3/4] utils: user schema: explicitly forbid @ in user-names

[Thomas Lamprecht]

Am 26.02.25 um 20:17 schrieb Stoiko Ivanov:
> PMGs terms are:
> * 'userid' consists of 'username'@'realm'
> 
> without this patch it was possible to create a user through the api,
> with @ in the username ('foo@bar@pmg'), and it got written to the
> user-conf.
> Reading that entry was not possible, as the verification on read was
> stricter.
> 
> This patch forbids '@' in usernames, and additionally drops the
> maxLength of 64, as 60 are already enforced by the regex pattern match
> (leaving 4 as minimal length for '@pmg'/'@pam').
> 
> Potential for regression should be minimal (the users could not be
> read-back from the config).
> 
> Reported-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/Utils.pm | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 9a50de2..7e4b70b 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -50,7 +50,7 @@ postgres_admin_cmd
>  try_decode_utf8
>  );
>  
> -my $user_regex = qr![^\s:/]+!;
> +my $user_regex = qr![^\s:@/]+!;
>  
>  sub valid_pmg_realm_regex {
>      my $cfg = PVE::INotify::read_file(PMG::Auth::Plugin::realm_conf_id());

context is now outdated so this needs rebasing

> @@ -110,7 +110,6 @@ PVE::JSONSchema::register_standard_option('username', {
>      description => "Username (without realm)",
>      type => 'string',
>      pattern => '[^\s:\/\@]{1,60}',
> -    maxLength => 64,
>  });
>  
>  PVE::JSONSchema::register_standard_option('pmg-email-address', {



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH pmg-api 4/4] api: users: rework userid validation

[Thomas Lamprecht]

Am 26.02.25 um 20:17 schrieb Stoiko Ivanov:
> reuse the existing helpers in PMG::Utils to forbid creating:
> * users with an unallowed realm
> * passing a different realm than what we receive in the userid
> * users in the pam realm
> 
> Reported-by: Mira Limbeck <m.limbeck@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  src/PMG/API2/Users.pm | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
>

applied, thanks!


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel