Re: [PATCHES] pg_resetxlog as root

2004-12-13 Thread Dave Page
 

 -Original Message-
 From: Neil Conway [mailto:[EMAIL PROTECTED] 
 Sent: 13 December 2004 03:59
 To: Dave Page
 Cc: pgsql-patches
 Subject: RE: [PATCHES] pg_resetxlog as root
 
 On Sun, 2004-12-12 at 23:59 +, Dave Page wrote:
  Sounds reasonable to me.
 
 Attached is a patch that implements this.
 src/port/backend/win32/security.c is moved to 
 src/port/win32_security.c, and conditionally added to 
 LIBOBJS. Note that I don't have much experience with the 
 build system, and less still with the Win32 port, so please 
 let me know if there's a better way to do this. Also, I don't 
 have a Windows build environment -- could someone verify 
 whether this builds on Win32?

Not quite there I'm afraid. Unfortunately I don't have time right now to
look further, but I don't suppose it's a major problem:

make[3]: Entering directory `/cvs/pgsql/src/bin/pg_resetxlog'
msgfmt -o po/cs.mo po/cs.po
msgfmt -o po/de.mo po/de.po
msgfmt -o po/es.mo po/es.po
msgfmt -o po/fr.mo po/fr.po
msgfmt -o po/hu.mo po/hu.po
msgfmt -o po/it.mo po/it.po
msgfmt -o po/nb.mo po/nb.po
msgfmt -o po/pt_BR.mo po/pt_BR.po
msgfmt -o po/ro.mo po/ro.po
msgfmt -o po/ru.mo po/ru.po
msgfmt -o po/sk.mo po/sk.po
msgfmt -o po/sl.mo po/sl.po
msgfmt -o po/sv.mo po/sv.po
msgfmt -o po/tr.mo po/tr.po
msgfmt -o po/zh_CN.mo po/zh_CN.po
msgfmt -o po/zh_TW.mo po/zh_TW.po
make -C ../../../src/port all
make[4]: Entering directory `/cvs/pgsql/src/port'
make[4]: Nothing to be done for `all'.
make[4]: Leaving directory `/cvs/pgsql/src/port'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels
-fno-strict-aliasing -I../../../src/include -I./src/include/port/win32
-DEXEC_BACKEND  -I../../../src/include/port/win32 -DFRONTEND  -c -o
pg_resetxlog.o pg_resetxlog.c
pg_resetxlog.c: In function `main':
pg_resetxlog.c:190: warning: implicit declaration of function
`pgwin32_is_admin'
pg_resetxlog.c: In function `PrintControlValues':
pg_resetxlog.c:468: warning: unsigned int format, different type arg
(arg 4)
rm -f pg_crc.c  ln -s ../../../src/backend/utils/hash/pg_crc.c .
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels
-fno-strict-aliasing -I../../../src/include -I./src/include/port/win32
-DEXEC_BACKEND  -I../../../src/include/port/win32 -DFRONTEND  -c -o
pg_crc.o pg_crc.c
sed -e 's;FILEDESC;pg_resetxlog - reset PostgreSQL WAL log;' -e
's;VFT_APP;VFT_APP;' -e 's;_ICO_;;' ../../../src/port/win32ver.rc 
win32ver.rc
windres -i win32ver.rc -o win32ver.o --include-dir=../../../src/include
rm -f win32ver.rc
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels
-fno-strict-aliasing pg_resetxlog.o pg_crc.o win32ver.o
-L../../../src/port -Wl,--allow-multiple-definition   -lpgport -lintl
-lssleay32 -leay32 -lz -lwsock32 -lm  -lws2_32 -o pg_resetxlog.exe
../../../src/port/libpgport.a(win32_security.o)(.text+0x2cf):win32_secur
ity.c: undefined reference to `write_stderr'
../../../src/port/libpgport.a(win32_security.o)(.text+0x2f2):win32_secur
ity.c: undefined reference to `write_stderr'
make[3]: *** [pg_resetxlog] Error 1
make[3]: Leaving directory `/cvs/pgsql/src/bin/pg_resetxlog'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/cvs/pgsql/src/bin'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/cvs/pgsql/src'
make: *** [all] Error 2

Regards, Dave.

---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly


Re: [PATCHES] pg_resetxlog as root

2004-12-13 Thread Neil Conway
On Mon, 2004-12-13 at 11:30 +, Dave Page wrote:
  
  -Original Message-
  From: Neil Conway [mailto:[EMAIL PROTECTED] 
  Sent: 13 December 2004 03:59
  To: Dave Page
  Cc: pgsql-patches
  Subject: RE: [PATCHES] pg_resetxlog as root
  
  On Sun, 2004-12-12 at 23:59 +, Dave Page wrote:
   Sounds reasonable to me.
  
  Attached is a patch that implements this.
  src/port/backend/win32/security.c is moved to 
  src/port/win32_security.c, and conditionally added to 
  LIBOBJS. Note that I don't have much experience with the 
  build system, and less still with the Win32 port, so please 
  let me know if there's a better way to do this. Also, I don't 
  have a Windows build environment -- could someone verify 
  whether this builds on Win32?
 
 Not quite there I'm afraid. Unfortunately I don't have time right now to
 look further, but I don't suppose it's a major problem [...]

On closer inspection, there are other places where Win32 could make this
check but does not (e.g. pg_ctl, initdb). Therefore I've decided not to
fix the Win32 side of things, and just committed the patch as is. IMHO
it would be wise for the Win32 folks to fix this, but I'll leave that
decision to them (... without a Win32 build environment I'm not really
comfortable committing fixes during RC without being able to compile,
much less test them).

-Neil



---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match


Re: [PATCHES] pg_resetxlog as root

2004-12-12 Thread Dave Page
 

 -Original Message-
 From: [EMAIL PROTECTED] 
 [mailto:[EMAIL PROTECTED] On Behalf Of Neil Conway
 Sent: 12 December 2004 22:57
 To: pgsql-patches
 Subject: [PATCHES] pg_resetxlog as root
 
 I suppose a similar fix is needed for Win32? If so, 
 pgwin32_is_admin() would be the natural routine to call, but 
 that is currently in src/backend/port -- we would need to 
 move it to src/port, probably. Comments?

Sounds reasonable to me.

Regards, Dave.

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match


[PATCHES] pg_resetxlog as root

2004-12-12 Thread Neil Conway
We should prevent pg_resetxlog from being run as root: it writes new
files to $PGDATA, so running the tool as root will result in those files
being owned by root, which makes the data directory unusable.

Attached is a trivial patch that makes this change for Unix. I suppose a
similar fix is needed for Win32? If so, pgwin32_is_admin() would be the
natural routine to call, but that is currently in src/backend/port -- we
would need to move it to src/port, probably. Comments?

-Neil

Index: src/bin/pg_resetxlog/pg_resetxlog.c
===
RCS file: /var/lib/cvs/pgsql/src/bin/pg_resetxlog/pg_resetxlog.c,v
retrieving revision 1.25
diff -c -r1.25 pg_resetxlog.c
*** src/bin/pg_resetxlog/pg_resetxlog.c	17 Nov 2004 21:37:47 -	1.25
--- src/bin/pg_resetxlog/pg_resetxlog.c	9 Dec 2004 07:03:20 -
***
*** 181,186 
--- 181,200 
  	snprintf(ControlFilePath, MAXPGPATH, %s/global/pg_control, DataDir);
  
  	/*
+ 	 * Don't allow pg_resetxlog to be run as root, to avoid
+ 	 * overwriting the ownership of files in the data directory. We
+ 	 * need only check for root -- any other user won't have
+ 	 * sufficient permissions to modify files in the data directory.
+ 	 */
+ 	if (geteuid() == 0)
+ 	{
+ 		fprintf(stderr, _(%s: cannot be run as \root\\n), progname);
+ 		fprintf(stderr, _(You must run pg_resetxlog 
+ 		  as the PostgreSQL superuser.\n));
+ 		exit(1);
+ 	}
+ 
+ 	/*
  	 * Check for a postmaster lock file --- if there is one, refuse to
  	 * proceed, on grounds we might be interfering with a live
  	 * installation.

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])


Re: [PATCHES] pg_resetxlog as root

2004-12-12 Thread Neil Conway
On Sun, 2004-12-12 at 23:59 +, Dave Page wrote:
 Sounds reasonable to me.

Attached is a patch that implements this.
src/port/backend/win32/security.c is moved to src/port/win32_security.c,
and conditionally added to LIBOBJS. Note that I don't have much
experience with the build system, and less still with the Win32 port, so
please let me know if there's a better way to do this. Also, I don't
have a Windows build environment -- could someone verify whether this
builds on Win32?

-Neil

# 
# delete_file src/backend/port/win32/security.c
# 
# add_file src/port/win32_security.c
# 
# patch src/Makefile.global.in
#  from [4183afa6d7d7945d4e29a3aa30a18c312251ed8e]
#to [6a0e08d33676e1c8c17a296653edbbd8afd5a2c2]
# 
# patch src/backend/port/win32/Makefile
#  from [009b45987d1428cbbd1d0bb78ee55868f9729ca3]
#to [97a5831267b24977fa4f3770d06e0fef7884bf2f]
# 
# patch src/bin/pg_resetxlog/pg_resetxlog.c
#  from [df43f11496af96317f9f927f84c488ce33492c93]
#to [58ddbddf7765426d60a8fddaaea2034e1e259cfb]
# 
# patch src/include/port/win32.h
#  from [39b47f7b5c1eaafadaff48cb2c5ccf60ca59]
#to [e346ae59f9fde1a95a46ccf81ba3ca8c50bf8cef]
# 
# patch src/port/win32_security.c
#  from []
#to [4766ac4547aa1648eaa8596ce31ce12da763a3c8]
# 
--- src/Makefile.global.in
+++ src/Makefile.global.in
@@ -362,6 +362,10 @@
 
 LIBOBJS = @LIBOBJS@ dirmod.o exec.o noblock.o path.o pipe.o pgsleep.o pgstrcasecmp.o sprompt.o thread.o
 
+ifeq ($(PORTNAME),win32)
+LIBOBJS += win32_security.o
+endif
+
 ifneq (,$(LIBOBJS))
 LIBS := -lpgport $(LIBS)
 ifdef PGXS
--- src/backend/port/win32/Makefile
+++ src/backend/port/win32/Makefile
@@ -12,7 +12,7 @@
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = sema.o shmem.o timer.o socket.o signal.o security.o error.o
+OBJS = sema.o shmem.o timer.o socket.o signal.o error.o
 
 all: SUBSYS.o
 
--- src/bin/pg_resetxlog/pg_resetxlog.c
+++ src/bin/pg_resetxlog/pg_resetxlog.c
@@ -181,6 +181,32 @@
 	snprintf(ControlFilePath, MAXPGPATH, %s/global/pg_control, DataDir);
 
 	/*
+	 * Don't allow pg_resetxlog to be run as root, to avoid
+	 * overwriting the ownership of files in the data directory. We
+	 * need only check for root -- any other user won't have
+	 * sufficient permissions to modify files in the data directory.
+	 */
+#ifdef WIN32
+	if (pgwin32_is_admin())
+	{
+		fprintf(stderr, _(%s: cannot be executed by a user with 
+		  administrative permissions\n), progname);
+		fprintf(stderr, _(You must run %s as the PostgreSQL superuser.\n),
+progname);
+		exit(1);
+	}
+#else
+	if (geteuid() == 0)
+	{
+		fprintf(stderr, _(%s: cannot be executed by \root\\n),
+progname);
+		fprintf(stderr, _(You must run %s as the PostgreSQL superuser.\n),
+progname);
+		exit(1);
+	}
+#endif
+
+	/*
 	 * Check for a postmaster lock file --- if there is one, refuse to
 	 * proceed, on grounds we might be interfering with a live
 	 * installation.
--- src/include/port/win32.h
+++ src/include/port/win32.h
@@ -145,15 +145,14 @@
 
 const char *pgwin32_socket_strerror(int err);
 int pgwin32_waitforsinglesocket(SOCKET s, int what);
-
-/* in backend/port/win32/security.c */
-extern int	pgwin32_is_admin(void);
-extern int	pgwin32_is_service(void);
 #endif
 
 /* in backend/port/win32/error.c */
 void		_dosmaperr(unsigned long);
 
+/* in port/win32_security.c */
+extern int	pgwin32_is_admin(void);
+extern int	pgwin32_is_service(void);
 
 #define WEXITSTATUS(w)	(((w)  8)  0xff)
 #define WIFEXITED(w)	(((w)  0xff) == 0)
--- src/port/win32_security.c
+++ src/port/win32_security.c
@@ -0,0 +1,248 @@
+/*-
+ *
+ * win32_security.c
+ *	  Microsoft Windows Win32 Security Support Functions
+ *
+ * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  $PostgreSQL: pgsql/src/backend/port/win32/security.c,v 1.7 2004/11/16 19:52:22 tgl Exp $
+ *
+ *-
+ */
+
+#include postgres.h
+
+
+static BOOL pgwin32_get_dynamic_tokeninfo(HANDLE token,
+		  TOKEN_INFORMATION_CLASS class, char **InfoBuffer,
+		  char *errbuf, int errsize);
+
+/*
+ * Returns nonzero if the current user has administrative privileges,
+ * or zero if not.
+ *
+ * Note: this cannot use ereport() because it's called too early during
+ * startup.
+ */
+int
+pgwin32_is_admin(void)
+{
+	HANDLE		AccessToken;
+	char	   *InfoBuffer = NULL;
+	charerrbuf[256];
+	PTOKEN_GROUPS Groups;
+	PSID		AdministratorsSid;
+	PSID		PowerUsersSid;
+	SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
+	UINT		x;
+	BOOL		success;
+
+	if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, AccessToken))
+	{
+		write_stderr(could not open process token: error code %d\n,
+	 (int) GetLastError());
+		exit(1);
+	}
+
+	if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups,
+	   InfoBuffer, errbuf, sizeof(errbuf)))
+	{
+