[pmg-devel] code adjustment for PMG in /usr/share/perl5/PMG/DBTools.pm

[Stoiko Ivanov]

Hi,

Patrick, thanks for picking this up and bringing it to our attention!
On Fri, Apr 12, 2019 at 12:35:01PM -0400, Patrick Fogarty wrote:
> Thomas
> 
> I agree that my change works, but introduces the potential for problems, and
> therefore may not be the "correct" answer. In fact I have no confirmation
> that everything is okay. Everything appears to be okay after upgrade, except
> that tracking center loops forever with a "connection error". However I did
> not press matters to determine if there truly is an issue.

Hmm - this should not happen - but is probably not directly related to the
postgres_admin_cmd issue. However I could imagine that some services did
not get setup correctly, because of the failing postinst script - You could
try to run `dpkg --configure -a` (with your patch in place) and restart
all services (or the whole PMG-instance).

If this does not fix the issue - I would suggest to open a bug-report in
https://bugzilla.proxmox.com (or a forum thread) for further analysis.

> 
> The environment...
> PVE: 5.3-12
> PMG: 5.1-3
> 
> The is a test environment, so we are using your community repo, not
> subscription. However it is happening in subscription as well.
> 
> PMG is running inside a container based on Debian Stretch. It is about as
> unmodified as you can get.
> 
> We did make modifications to the PMG install. They are limited to
> /var/lib/pmg/templates/main.cf.in and /var/lib/pmg/templates/init.pre.in
> (geoip) and some custom spamassassin rules. Beyond that, nothing has been
> touched.

If you modify the configuration templates - please put them in
'/etc/pmg/templates' - that way they are preserved across upgrades (see
section 4.3 of the admin guide
https://www.proxmox.com/images/download/pmg/docs/pmg-admin-guide.pdf).

> 
> The upgrade failure happens at the very end during the configuration phase.
> As the post mentioned, I traced the issue to PMG:: DBTools::
> postgres_admin_cmd. But you have already reviewed the post and the code, so
> you already know that.
> 
Your analysis is correct - and as the strace output of the OP shows the
setresuid call is successful, but $! (perl's variable for errno still is
set from a previous syscall (the TCGETS ioctl).
I managed to reproduce the issue by installing 'nscd' (without it the
getpwnam call opens and reads '/etc/passwd' directly and this resets errno
to 0 somewhere, with it installed it asks the nscd, and errno still is
set).

We would gladly welcome a patch by you! (I would probably use the approach
from [0] for readability reasons). Although setting $! to 0 before `$> =
$uid;` should work as well.
If you don't find the time, I can also send the patch.

BTW. this line is perl's way of 'su postgres' (what Thomas refers to as
Perl_magic_set :)

I quickly skimmed through the pmg-api repos (with `git grep '\$\!') - and
this seems the only problematic use of relying on $! being 0 instead of
explictly checking for an error-condition.

cheers,
stoiko

[0] https://www.perlmonks.org/bare/?node_id=117878
> 
> 
> Let me know how I can help further.
> 
> 
> 
> Thomas Lamprecht wrote on 4/12/19 11:23 AM:
> > Hi,
> > 
> > On 4/12/19 4:17 PM, Patrick Fogarty wrote:
> > > If you follow this thread...
> > > https://forum.proxmox.com/threads/setuid-postgres-114-failed-inappropriate-ioctl-for-device.41414/
> > > 
> > > ...you will find that I had an issue, as did others, performing upgrades/updates. I adjusted my code manually and things went back to normal. Last month you released PMG 5.2. The upgrade downloads a package that includes the original code, so the upgrade overwrites my changes, then fails the upgrade process "pmg-api --configure", after which I believe the process is in an incomplete state.
> > > 
> > > How can I assist, beyond my post in the above thread, showing the code change I made?
> > Either, you could sent a patch here, for this see:
> > https://pve.proxmox.com/wiki/Developer_Documentation
> > (while written for PVE, almost all is valid for PMG too)
> > 
> > Else you normally also can make a Bug report, which does not gets as easily forgotten as
> > some Forum/Mailing list thread: https://bugzilla.proxmox.com/enter_bug.cgi
> > 
> > But posting here may be enough this time, Stoiko, could you please take a look
> > at the proposed change:
> > https://forum.proxmox.com/threads/setuid-postgres-114-failed-inappropriate-ioctl-for-device.41414/#post-233016
> > 
> > Looking at perl's source code at the Perl_magic_set case for EUID (case '>')
> > we just see that there's no special error handling, thus the recommendation
> > to check $! (errno or sys_errlist[errno] depending on context) after the call.
> > 
> > We do that, and it seems correct, so either we need to reset it to 0 befor the
> > seteuid call to ensure we do not check an old value or this is correct an the
> > seteuid really fails in the posted case..
> > 
> > Patrick, can you post a bit more details about your setup, you said you run it in
> > PVE, inside a Virtual Machine (VM) or Container (CT)?
> > 
> > Because your change could just omit error checking completely, and thus not be
> > the "correct" solution here..
> > 
> > cheers,
> > Thomas
> > 
> > 
> 

> _______________________________________________
> pmg-devel mailing list
> pmg-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel