lowering pg_regress privileges on Windows
From the "scratch a long running itch" department.
The attached ridiculously tiny patch solves the problem whereby while we
can run Postgres on Windows safely from an Administrator account, we
can't run run the regression tests from the same account, since it fails
on the tablespace test, the tablespace directory having been set up
without first having lowered privileges. The solution is to lower
pg_regress' privileges in the same way that we do with other binaries.
This is useful in setups like Appveyor where running under any other
account is ... difficult. For the cfbot Thomas has had to make the
script hack the schedule file to omit the tablespace test. This would
make that redundant.
I propose to backpatch this. It's close enough to a bug and the risk is
almost infinitely small.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
regress_restricted.patchtext/x-patch; name=regress_restricted.patchDownload
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 6890678..3248603 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2081,6 +2081,8 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_regress"));
+ get_restricted_token(progname);
+
atexit(stop_postmaster);
#ifndef HAVE_UNIX_SOCKETS
On Thu, Oct 18, 2018 at 08:31:11AM -0400, Andrew Dunstan wrote:
The attached ridiculously tiny patch solves the problem whereby while we can
run Postgres on Windows safely from an Administrator account, we can't run
run the regression tests from the same account, since it fails on the
tablespace test, the tablespace directory having been set up without first
having lowered privileges. The solution is to lower pg_regress' privileges
in the same way that we do with other binaries. This is useful in setups
like Appveyor where running under any other account is ... difficult. For
the cfbot Thomas has had to make the script hack the schedule file to omit
the tablespace test. This would make that redundant.I propose to backpatch this. It's close enough to a bug and the risk is
almost infinitely small.
+1. get_restricted_token() refactoring has been done down to
REL9_5_STABLE. With 9.4 and older you would need to copy again this
full routine into pg_regress.c, which is in my opinion not worth
worrying about.
--
Michael
On Fri, Oct 19, 2018 at 1:13 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Oct 18, 2018 at 08:31:11AM -0400, Andrew Dunstan wrote:
The attached ridiculously tiny patch solves the problem whereby while we can
run Postgres on Windows safely from an Administrator account, we can't run
run the regression tests from the same account, since it fails on the
tablespace test, the tablespace directory having been set up without first
having lowered privileges. The solution is to lower pg_regress' privileges
in the same way that we do with other binaries. This is useful in setups
like Appveyor where running under any other account is ... difficult. For
the cfbot Thomas has had to make the script hack the schedule file to omit
the tablespace test. This would make that redundant.I propose to backpatch this. It's close enough to a bug and the risk is
almost infinitely small.+1. get_restricted_token() refactoring has been done down to
REL9_5_STABLE. With 9.4 and older you would need to copy again this
full routine into pg_regress.c, which is in my opinion not worth
worrying about.
FWIW here is a successful Appveyor build including the full test
schedule (CI patch attached in case anyone is interested). Woohoo!
Thanks for figuring that out Andrew. I will be very happy to remove
that wart from my workflows.
https://ci.appveyor.com/project/macdice/postgres/builds/19626669
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
0001-CI-configuration-for-Appveyor.patchapplication/octet-stream; name=0001-CI-configuration-for-Appveyor.patchDownload
From f9fddc8245b5692596e76b822c067febb9aa55ef Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@enterprisedb.com>
Date: Fri, 28 Sep 2018 21:00:47 +1200
Subject: [PATCH 1/2] CI configuration for Appveyor.
---
appveyor.yml | 22 ++++++++++++++++++++++
buildsetup.pl | 38 ++++++++++++++++++++++++++++++++++++++
dumpregr.pl | 21 +++++++++++++++++++++
3 files changed, 81 insertions(+)
create mode 100644 appveyor.yml
create mode 100644 buildsetup.pl
create mode 100644 dumpregr.pl
diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 00000000000..4a2f068d284
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,22 @@
+# appveyor.yml
+install:
+ - appveyor-retry cinst winflexbison
+ - '"C:\Program Files\Microsoft SDKs\Windows\v7.1\Bin\SetEnv.cmd" /x64'
+ - '"C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\vcvarsall.bat" x86_amd64'
+
+before_build:
+ - rename c:\ProgramData\chocolatey\bin\win_flex.exe flex.exe
+ - rename c:\ProgramData\chocolatey\bin\win_bison.exe bison.exe
+ - perl buildsetup.pl
+
+build:
+ project: pgsql.sln
+
+test_script:
+ - cd src\tools\msvc && vcregress check
+
+on_failure:
+ - perl dumpregr.pl
+
+configuration:
+ - Release
diff --git a/buildsetup.pl b/buildsetup.pl
new file mode 100644
index 00000000000..23df2fb1aa4
--- /dev/null
+++ b/buildsetup.pl
@@ -0,0 +1,38 @@
+# first part of postgres build.pl, just doesn't run msbuild
+
+use strict;
+
+BEGIN
+{
+
+ chdir("../../..") if (-d "../msvc" && -d "../../../src");
+
+}
+
+use lib "src/tools/msvc";
+
+use Cwd;
+
+use Mkvcbuild;
+
+# buildenv.pl is for specifying the build environment settings
+# it should contain lines like:
+# $ENV{PATH} = "c:/path/to/bison/bin;$ENV{PATH}";
+
+if (-e "src/tools/msvc/buildenv.pl")
+{
+ do "src/tools/msvc/buildenv.pl";
+}
+elsif (-e "./buildenv.pl")
+{
+ do "./buildenv.pl";
+}
+
+# set up the project
+our $config;
+do "config_default.pl";
+do "config.pl" if (-f "src/tools/msvc/config.pl");
+
+# print "PATH: $_\n" foreach (split(';',$ENV{PATH}));
+
+Mkvcbuild::mkvcbuild($config);
diff --git a/dumpregr.pl b/dumpregr.pl
new file mode 100644
index 00000000000..f0bd624bba0
--- /dev/null
+++ b/dumpregr.pl
@@ -0,0 +1,21 @@
+use strict;
+use warnings FATAL => qw(all);
+
+use File::Find;
+
+my $Target = "regression.diffs";
+
+find(\&dump, "src");
+
+sub dump {
+ if ($_ eq $Target) {
+ my $path = $File::Find::name;
+ print "=== \$path ===\\n";
+ open(my $fh, "<", $_) || die "wtf";
+ for (1..1000) {
+ my $line = <$fh>;
+ last unless defined $line;
+ print $line;
+ }
+ }
+}
--
2.17.1 (Apple Git-112)
On 10/18/2018 08:25 PM, Thomas Munro wrote:
On Fri, Oct 19, 2018 at 1:13 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Oct 18, 2018 at 08:31:11AM -0400, Andrew Dunstan wrote:
The attached ridiculously tiny patch solves the problem whereby while we can
run Postgres on Windows safely from an Administrator account, we can't run
run the regression tests from the same account, since it fails on the
tablespace test, the tablespace directory having been set up without first
having lowered privileges. The solution is to lower pg_regress' privileges
in the same way that we do with other binaries. This is useful in setups
like Appveyor where running under any other account is ... difficult. For
the cfbot Thomas has had to make the script hack the schedule file to omit
the tablespace test. This would make that redundant.I propose to backpatch this. It's close enough to a bug and the risk is
almost infinitely small.+1. get_restricted_token() refactoring has been done down to
REL9_5_STABLE. With 9.4 and older you would need to copy again this
full routine into pg_regress.c, which is in my opinion not worth
worrying about.FWIW here is a successful Appveyor build including the full test
schedule (CI patch attached in case anyone is interested). Woohoo!
Thanks for figuring that out Andrew. I will be very happy to remove
that wart from my workflows.https://ci.appveyor.com/project/macdice/postgres/builds/19626669
Excellent. I'll apply back to 9.5 as Michael suggests.
Having got past that hurdle I encountered another one in the same area.
pg_upgrade gives up its privileges and is then unable to write things
like log files and analyze scripts.
The attached patch cures the problem, but it doesn't seem like the best
cure. Maybe there is a more secure way to do it. Essentially it saves
out the ACLS for the current directory and its subdirectories and then
allows everyone to write to them, right before running pg_upgrade. When
pg_upgrade is done it restores the saved ACLs.
Maybe someone who understands more about how this all works can suggest
a less blunt force approach.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
upgrade-check-permissions.patchtext/x-patch; name=upgrade-check-permissions.patchDownload
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index ce5c976c16..0f25b44d0a 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -569,11 +569,14 @@ sub upgradecheck
$ENV{PGDATA} = "$data";
print "\nSetting up new cluster\n\n";
standard_initdb() or exit 1;
+ system('icacls . /save savedacls /t');
+ system('icacls . /grant "*S-1-1-0":(OI)(CI)M');
print "\nRunning pg_upgrade\n\n";
@args = (
'pg_upgrade', '-d', "$data.old", '-D', $data, '-b',
$bindir, '-B', $bindir);
system(@args) == 0 or exit 1;
+ system('icacls . /restore savedacls');
print "\nStarting new cluster\n\n";
@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');
system(@args) == 0 or exit 1;