[BackupPC-devel] Inherited patch question

[Richard Shaw]

I'm working on the 4.1.0 release for Fedora and I've been working through
all the patches I inherited, most of which aren't necessary anymore.

Here's one that I'm not sure what effect it actually has:

$ cat rpmbuild/BackupPC/SOURCES/BackupPC-4.0.0-fix-shadow-access.patch
--- a/lib/BackupPC/CGI/Lib.pm
+++ b/lib/BackupPC/CGI/Lib.pm
@@ -143,7 +143,7 @@ sub NewRequest
# Verify we are running as the correct user
#
if ( $Conf{BackupPCUserVerify}
- && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))[2]) ) {
+ && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))) ) {
ErrorExit(eval("qq{$Lang->{Wrong_user__my_userid_is___}}"), <<EOF);
This script needs to run as the user specified in \$Conf{BackupPCUser},
which is set to $Conf{BackupPCUser}.
--- a/lib/BackupPC/Lib.pm
+++ b/lib/BackupPC/Lib.pm
@@ -127,7 +127,7 @@ sub new
#
if ( !$noUserCheck
&& $bpc->{Conf}{BackupPCUserVerify}
- && $> != (my $uid = (getpwnam($bpc->{Conf}{BackupPCUser}))[2])
) {
+ && $> != (my $uid = (getpwnam($bpc->{Conf}{BackupPCUser}))) ) {
print(STDERR "$0: Wrong user: my userid is $>, instead of $uid"
. " ($bpc->{Conf}{BackupPCUser})\n");
print(STDERR "Please su $bpc->{Conf}{BackupPCUser} first\n");

What's the effect of removing the [2] from these?

Thanks,
Richard

[Holger Parplies]

Hi,

Post by Richard Shaw
[...]
$ cat rpmbuild/BackupPC/SOURCES/BackupPC-4.0.0-fix-shadow-access.patch
[...]
- && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))[2]) ) {
+ && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))) ) {
[...]
What's the effect of removing the [2] from these?

well, in theory (and practise, at least on my local system here) getpwnam
returns something like 'split /:/, $passwd_line' in list context and the
uid in scalar context. The third element (index [2]) of the split would
also be the uid, which explains why the two lines can be equivalent, even
though they seem very different.

Post by Richard Shaw
From the *name* of the patch, I would guess that there might be a potential

problem on systems with shadow passwords in some cases, though I can't see
one here on my system. I could *imagine* though, that there might be systems
that differ.

A closer look reveals the following:

% perl -e 'my @p = getpwnam "foo"; print ">", (join ",", @p), "<\n";'
foo,x,1234,1234,,,Holger Parplies,/home/foo,/bin/tcsh
# perl -e 'my @p = getpwnam "foo"; print ">", (join ",", @p), "<\n";'
foo,<my-hashed-password>,1234,1234,,,Holger Parplies,/home/foo,/bin/tcsh

(no, my user name is not "foo" and my uid is not 1234 ;-), so my Perl (or
rather getpwnam(3)) merges in the shadow password, privilege permitting.
Although I can't find any hint in the documentation, I could imagine that
the attempt to do so could trigger unwanted behaviour (e.g. an audit log or
even termination of the process) under some security systems, depending on
how the determination of "privilege permitting" might be implemented.

In any case, I would *hope* that the scalar context case would be slightly
more efficient, because the unneeded information in the additional array
elements not corresponding to /etc/passwd fields ($quota, $comment, $expire)
does not need to be retrieved.

For an explanation of the getpwnam function look at 'perldoc -f getpwuid'
(strangely, 'perldoc -f getpwnam' is not very helpful, at least on some
systems ;-).

Regards,
Holger

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
BackupPC-devel mailing list
BackupPC-***@lists.sourceforge.net
List: https://lists.sourceforge.net/lists/listinfo/backuppc-devel
Wiki: http://backuppc.wiki.sourceforge.net
Project: http://backuppc.sourceforge.net/

[Craig Barratt]

Holger,

Thanks for the explanation. I'm happy to remove the [2] and use scalar
context.

Craig

Hi,
Richard Shaw wrote on 2017-03-29 13:38:16 -0500 [[BackupPC-devel]

Post by Richard Shaw
[...]
$ cat rpmbuild/BackupPC/SOURCES/BackupPC-4.0.0-fix-shadow-access.patch
[...]
- && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))[2]) ) {
+ && $> != (my $uid = (getpwnam($Conf{BackupPCUser}))) ) {
[...]
What's the effect of removing the [2] from these?

well, in theory (and practise, at least on my local system here) getpwnam
returns something like 'split /:/, $passwd_line' in list context and the
uid in scalar context. The third element (index [2]) of the split would
also be the uid, which explains why the two lines can be equivalent, even
though they seem very different.

Post by Richard Shaw
From the *name* of the patch, I would guess that there might be a

potential
problem on systems with shadow passwords in some cases, though I can't see
one here on my system. I could *imagine* though, that there might be systems
that differ.
foo,x,1234,1234,,,Holger Parplies,/home/foo,/bin/tcsh
foo,<my-hashed-password>,1234,1234,,,Holger
Parplies,/home/foo,/bin/tcsh
(no, my user name is not "foo" and my uid is not 1234 ;-), so my Perl (or
rather getpwnam(3)) merges in the shadow password, privilege permitting.
Although I can't find any hint in the documentation, I could imagine that
the attempt to do so could trigger unwanted behaviour (e.g. an audit log or
even termination of the process) under some security systems, depending on
how the determination of "privilege permitting" might be implemented.
In any case, I would *hope* that the scalar context case would be slightly
more efficient, because the unneeded information in the additional array
elements not corresponding to /etc/passwd fields ($quota, $comment, $expire)
does not need to be retrieved.
For an explanation of the getpwnam function look at 'perldoc -f getpwuid'
(strangely, 'perldoc -f getpwnam' is not very helpful, at least on some
systems ;-).
Regards,
Holger
------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
BackupPC-devel mailing list
List: https://lists.sourceforge.net/lists/listinfo/backuppc-devel
Wiki: http://backuppc.wiki.sourceforge.net
Project: http://backuppc.sourceforge.net/