Обсуждение: pgsql: Revert "Secure Unix-domain sockets of "make check" temporary clu
Revert "Secure Unix-domain sockets of "make check" temporary clusters." About half of the buildfarm members use too-long directory names, strongly suggesting that this approach is a dead end. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/8f5578d0f9681ef81bc71a3762a191d66a29c8b1 Modified Files -------------- contrib/pg_upgrade/test.sh | 9 ++++----- doc/src/sgml/regress.sgml | 34 +++++++++++++++------------------- src/test/regress/pg_regress.c | 31 +++++++------------------------ 3 files changed, 26 insertions(+), 48 deletions(-)
Noah Misch <noah@leadboat.com> writes:
> Revert "Secure Unix-domain sockets of "make check" temporary clusters."
> About half of the buildfarm members use too-long directory names,
> strongly suggesting that this approach is a dead end.
I'm confused. I do not see a single buildfarm failure that appears
attributable to that patch.
regards, tom lane
On 03/29/2014 09:32 AM, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
>> Revert "Secure Unix-domain sockets of "make check" temporary clusters."
>> About half of the buildfarm members use too-long directory names,
>> strongly suggesting that this approach is a dead end.
> I'm confused. I do not see a single buildfarm failure that appears
> attributable to that patch.
I too am confused.
The TestUpgrade buildfarm module contains this code (as of release 4.10,
14 months ago)
die
"overly long build root $buildroot will cause upgrade problems - try
something shorter than 46 chars"
if (length($buildroot) > 46);
Not all animals run this module, although it's recommended, but those
who do should be safe.
cheers
andrew
On Sat, Mar 29, 2014 at 09:32:06AM -0400, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: > > Revert "Secure Unix-domain sockets of "make check" temporary clusters." > > About half of the buildfarm members use too-long directory names, > > strongly suggesting that this approach is a dead end. > > I'm confused. I do not see a single buildfarm failure that appears > attributable to that patch. Here are some: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=reindeer&dt=2014-03-29%2006%3A00%3A03 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=guaibasaurus&dt=2014-03-29%2005%3A55%3A01 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=panther&dt=2014-03-29%2005%3A59%3A01 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bushpig&dt=2014-03-29%2006%3A15%3A02 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=olinguito&dt=2014-03-29%2005%3A59%3A03 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary&dt=2014-03-29%2007%3A02%3A48 -- Noah Misch EnterpriseDB http://www.enterprisedb.com
On 03/29/2014 01:22 PM, Noah Misch wrote: > On Sat, Mar 29, 2014 at 09:32:06AM -0400, Tom Lane wrote: >> Noah Misch <noah@leadboat.com> writes: >>> Revert "Secure Unix-domain sockets of "make check" temporary clusters." >>> About half of the buildfarm members use too-long directory names, >>> strongly suggesting that this approach is a dead end. >> I'm confused. I do not see a single buildfarm failure that appears >> attributable to that patch. > Here are some: > > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=reindeer&dt=2014-03-29%2006%3A00%3A03 > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=guaibasaurus&dt=2014-03-29%2005%3A55%3A01 > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=panther&dt=2014-03-29%2005%3A59%3A01 > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bushpig&dt=2014-03-29%2006%3A15%3A02 > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=olinguito&dt=2014-03-29%2005%3A59%3A03 > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary&dt=2014-03-29%2007%3A02%3A48 > Hmm. Can we use a location with a bit more head room than the tmp_check/data directory? Maybe something like src/test/sockets? Note that the buildfarm's buildroot (the part of the name before the branch name) is not terribly long in some of these cases. e.g. in the first case it's only 32 chars long. cheers andrew
Noah Misch <noah@leadboat.com> writes: > On Sat, Mar 29, 2014 at 09:32:06AM -0400, Tom Lane wrote: >> I'm confused. I do not see a single buildfarm failure that appears >> attributable to that patch. > Here are some: > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=reindeer&dt=2014-03-29%2006%3A00%3A03 Hm, I was expecting that any failures would be in the main or contrib checks. I guess ecpg is a couple directory levels further down though. In this particular example the path is only about 7 characters too long, but I suppose trying to shave off a few characters isn't really going to be a robust solution. regards, tom lane
On 03/29/2014 04:47 PM, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: >> On Sat, Mar 29, 2014 at 09:32:06AM -0400, Tom Lane wrote: >>> I'm confused. I do not see a single buildfarm failure that appears >>> attributable to that patch. >> Here are some: >> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=reindeer&dt=2014-03-29%2006%3A00%3A03 > Hm, I was expecting that any failures would be in the main or contrib > checks. I guess ecpg is a couple directory levels further down though. > > In this particular example the path is only about 7 characters too long, > but I suppose trying to shave off a few characters isn't really going > to be a robust solution. > > Since you have one of the offending buildfarm machines (dromedary) why not try this: in the build_env section of the config file, add PG_REGRESS_SOCK_DIR => "/Users/buildfarm/bf-data/$branch/", If it works I'll have the buildfarm script default that to a slightly better location in the release I'm about to make. cheers andrew > >
Andrew Dunstan <andrew@dunslane.net> writes:
> On 03/29/2014 04:47 PM, Tom Lane wrote:
>> In this particular example the path is only about 7 characters too long,
>> but I suppose trying to shave off a few characters isn't really going
>> to be a robust solution.
> Since you have one of the offending buildfarm machines (dromedary) why
> not try this: in the build_env section of the config file, add
> PG_REGRESS_SOCK_DIR => "/Users/buildfarm/bf-data/$branch/",
I'm happy to tweak the config, but given that Noah reverted the patch,
what is it that we're testing exactly?
regards, tom lane
On 03/29/2014 05:39 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 03/29/2014 04:47 PM, Tom Lane wrote: >>> In this particular example the path is only about 7 characters too long, >>> but I suppose trying to shave off a few characters isn't really going >>> to be a robust solution. >> Since you have one of the offending buildfarm machines (dromedary) why >> not try this: in the build_env section of the config file, add >> PG_REGRESS_SOCK_DIR => "/Users/buildfarm/bf-data/$branch/", > I'm happy to tweak the config, but given that Noah reverted the patch, > what is it that we're testing exactly? > > Oh, the commit emails arrived in my mailbox after the reversion emails, so I thought he'd reapplied the patch. Never mind. I can rig up a test with a long path and test with and without the patch. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 03/29/2014 05:39 PM, Tom Lane wrote:
>> I'm happy to tweak the config, but given that Noah reverted the patch,
>> what is it that we're testing exactly?
> Oh, the commit emails arrived in my mailbox after the reversion emails,
> so I thought he'd reapplied the patch. Never mind.
I suppose they got held for moderation because he'd attached "Security:"
markers to the commit messages (which, for the record, wasn't the right
thing to do in this case, since these weren't fixing live security bugs).
regards, tom lane
On Sat, Mar 29, 2014 at 01:48:33PM -0400, Andrew Dunstan wrote: > On 03/29/2014 01:22 PM, Noah Misch wrote: > >http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary&dt=2014-03-29%2007%3A02%3A48 > > Hmm. Can we use a location with a bit more head room than the > tmp_check/data directory? Maybe something like src/test/sockets? > Note that the buildfarm's buildroot (the part of the name before the > branch name) is not terribly long in some of these cases. e.g. in > the first case it's only 32 chars long. That's tempting, but I don't think freeing up ~25 bytes changes the verdict. Christoph brought up that Debian builds in directory trees deeper than those the buildfarm uses, and I suspect Debian is not alone. I think we're back looking at using a subdirectory of /tmp, with the open question being what properties (sticky bit, ownership, _PC_CHOWN_RESTRICTED), if any, to verify on /tmp and its parent(s) before proceeding. I looked around to see what other projects are doing. File::Temp is the one project I found that has an option[1], disabled by default, to security-check /tmp. Even OpenSSH simply assumes /tmp is suitable. Perhaps the threat of insecure /tmp has received less attention than it deserves, or perhaps secure /tmp is considered a mandatory component of a multi-user Unix system. In any event, I do not feel the need to put PostgreSQL "make check" in the vanguard concerning this issue. Assuming a secure /tmp, like OpenSSH does, is reasonable. -- Noah Misch EnterpriseDB http://www.enterprisedb.com [1] http://search.cpan.org/~dagolden/File-Temp-0.2304/lib/File/Temp.pm#safe_level