bin/125932: pkg_add(1) doesn't prompt for root credentials and then fails badly

[Garrett Cooper]

The following reply was made to PR bin/125932; it has been noted by GNATS.

From: "Garrett Cooper" <yanefbsd at gmail.com>
To: "Bruce Cran" <bruce at cran.org.uk>
Cc: bug-followup at freebsd.org, freebsd-bugs at freebsd.org
Subject: Re: bin/125932: pkg_add(1) doesn't prompt for root credentials and then fails badly
Date: Sun, 14 Sep 2008 21:10:09 -0700

 On Thu, Sep 11, 2008 at 11:24 PM, Garrett Cooper <yanefbsd at gmail.com> wrote:
 > On Tue, Sep 9, 2008 at 1:23 PM, Bruce Cran <bruce at cran.org.uk> wrote:
 >> On Thu, 24 Jul 2008 11:18:31 -0700
 >> "Garrett Cooper" <yanefbsd at gmail.com> wrote:
 >>
 >>> On Thu, Jul 24, 2008 at 6:48 AM, Bruce Cran <bruce at cran.org.uk> wrote:
 >>> >>How-To-Repeat:
 >>> > Run pkg_add -r as a non-root user.
 >>> >>Fix:
 >>>
 >>> The issue isn't the fact that you're running as non-root; it's that
 >>> someone's not checking to see whether or not a fetch init succeeded
 >>> (filehandle's open, writing's being done) before continuing.
 >>>
 >>> I'll fix this later on tonight when I get back from San Jose.
 >>>
 >>
 >> Did you make any progress with this?
 >
 > I thought I made a comment about this earlier, but apparently I didn't
 > send it out or it wasn't recorded:
 >
 > Symptom:
 >
 > The issue is caused by tar in the PUSHOUT macro in add/extract.c as
 > identified below, during the extract. If and when the tar stuff is
 > replaced with libarchive, this issue will fail sooner (and this should
 > be done because this would save a lot of time and resources when
 > extracting large packages like openoffice):
 >
 > #define PUSHOUT(todir) /* push out string */ \
 >   if (where_count > (int)sizeof(STARTSTRING)-1) { \
 >       strcat(where_args, "|/usr/bin/tar --unlink -xpPf - -C "); \
 >       strcat(where_args, todir); \
 >       if (system(where_args)) { \ /*** XXX: FAILS HERE ***/
 >           cleanup(0); \
 >           errx(2, "%s: can not invoke %ld byte tar pipeline: %s", \
 >                __func__, (long)strlen(where_args), where_args); \
 >       } \
 >
 > Real problem:
 >
 > The actual problem is that the master and slave pkg_add processes
 > aren't communicating properly with one another, s.t. the slave
 > instances aren't breaking the master execution at the first sign of
 > failure.
 >
 > HTH,
 > -Garrett
 
 Here's a proposed patch for the first set of cleanup to pkg_install:
 <http://pastebin.com/f1cee71c2>, and some fixing to alleviate the
 issue in bin/125932.
 
 Rather than biting off more than I can chew with the perforce project,
 I'm going to work off the changes Anders has made and incrementally
 polish pkg_install (like I should have done last year -_-...)
 
 This patch causes pkg_install to error out at the first sign of
 install failure (which could take a while as it's still using tar(1)
 to extract archives in add/extract.c), BUT in getFileByURL I've
 completely replaced the tar requirement in lib/url.c with
 archive(3)'s, quite handy hooks for writing to files. So don't be
 alarmed when you see that the file has grown 4 times ;)...
 
 This patch hasn't gotten much mileage, other than a few failure and
 success cases, so if others could please take a look at this and
 provide comments I'd much appreciate it.
 
 Cheers,
 -Garrett
 
 PS Packages might not be dumped in the correct spot -- I just chose
 /var/tmp, but if someone could point me to the "industry standard"
 location that portupgrade uses for instance, I'd be more than happy to
 point there.