[pmg-devel] [PATCH pmg-api] Fix setresuid to 'postgres' error handling

[Thomas Lamprecht]

On 4/15/19 7:58 PM, Stoiko Ivanov wrote:
> PMG::DBTools::postgres_admin_cmd switches the euid to postgres. The error
> handling expected that the setresuid (2) call failed if $! was != 0, without
> explicitly setting it to 0 beforehand. This lead to a false positive if errno
> was set from a previous library call.
> 
> Steps to reproduce:
> * install nscd on a system
> * try installing pmg-api (the postinst script invokes `pmgdb init`)
> 
> The issue was further discussed in [0].
> 
> [0] https://pve.proxmox.com/pipermail/pmg-devel/2019-April/000362.html
> 
> Reported-By: Patrick Fogarty <patrick.fogarty at patanne.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
> Since this potentially affects some users (AFAIR nscd is recommended by quite
> a few packages) and breaks upgrades I went ahead and prepared the patch as a
> result of our discussion.
> 
> Patrick, I hope this is ok with you!
> 
> 
>  PMG/DBTools.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/PMG/DBTools.pm b/PMG/DBTools.pm
> index 24a692a..8933b31 100644
> --- a/PMG/DBTools.pm
> +++ b/PMG/DBTools.pm
> @@ -82,8 +82,11 @@ sub postgres_admin_cmd {
>      $cmd = ref($cmd) ? $cmd : [ $cmd ];
>      my $uid = getpwnam('postgres') || die "getpwnam postgres failed\n";
>  
> +    # set $! to 0, it can be != 0 from a previous library call (e.g. getpwnam
> +    # when nscd is running).
> +    $! = 0;

setting it explicitly but then removing the check for it seems a bit weird..

But, we could throw this perl magic variable stuff all away and directly use
setuid from the perl POSIX module

> "setuid"
>      Sets the real user identifier and the effective user identifier
>      for this process. Similar to assigning a value to the Perl's
>      builtin $< variable, see "$UID" in perlvar, except that the latter
>      will change only the real user identifier.
-- perldoc POSIX

$< makes a set*e*uid call, but AFAICT changing both EUID and RUID does not
matters here in this context, EUID is what enforces (or limits) stuff..

The advantage should be that it has (hopefully, didn't tried it) some sane
error handling (``On success, zero is returned.  On error, -1 is returned, and
errno is set appropriately.'' - man setuid; man seteuid) and it  does hides behind
some magic single character Perl variable..

>      local $> = $uid;
> -    $! &&  die "setuid postgres ($uid) failed - $!\n";
> +    die "setuid postgres ($uid) failed - $!\n" if $> != $uid;
>  
>      PVE::Tools::run_command([@$cmd, '-U', 'postgres', @params], %$options);
>  }
>