Thread: [PATCH] Prompt for password on Windows platforms
The current version of psql, pg_dump, ... do not properly prompt the
user for a password, when the backend database is setup for password
authentication on a Windows platform. In investigating the code, I
noticed that the file '/dev/tty' is opened to interact with the console.
It appears as if on a Windows platform, the actual file /dev/tty is
opened, instead of the console. I am supplying a patch to address this
issue on the Windows platform.
*** ./src/port/sprompt.c.orig Wed Feb 22 13:07:43 2006
--- ./src/port/sprompt.c Wed Feb 22 13:08:14 2006
***************
*** 40,47 ****
{
int length;
char *destination;
! FILE *termin,
! *termout;
#ifdef HAVE_TERMIOS_H
struct termios t_orig,
--- 40,47 ----
{
int length;
char *destination;
! FILE *termin = NULL,
! *termout = NULL;
#ifdef HAVE_TERMIOS_H
struct termios t_orig,
***************
*** 63,70 ****
--- 63,72 ----
* Do not try to collapse these into one "w+" mode file. Doesn't work
on
* some platforms (eg, HPUX 10.20).
*/
+ #ifndef WIN32
termin = fopen("/dev/tty", "r");
termout = fopen("/dev/tty", "w");
+ #endif
if (!termin || !termout)
{
if (termin)
Let me know what you think.
Robert Kinberg
"Robert Kinberg" <kinberg@tecore.com> writes:
> The current version of psql, pg_dump, ... do not properly prompt the
> user for a password, when the backend database is setup for password
> authentication on a Windows platform.
Curious that you are the *only* person to have noticed such a
fundamental problem... please provide some evidence that this
patch is needed.
regards, tom lane
Tom, There are several posts to the internet about this issue. I found no solution to these posts either. Here is a except from a post on the pgsql-bugs mailing list: Hello, I have a problem with psql on Windows XP Pro. After an attempt to connect to a database I get this error: psql: FATAL: password authentication failed for user "<username>" This happens both for local (Windows) and remote (Linux) database. Both databases are easily accessible from other clients (psql for Linux, pgAdmin III, jdbc). The problem is that there is no password prompt even if I try to force it by -W option. According to pg_hba.conf, there should be no need to ask for password at all (local/all databases/trust). There is the same error message in the postgresql log as on the screen (see above). Version: psql (PostgreSQL) 8.0.0 Similar bug report: #1354 Best regards Filip Hrbek http://archives.postgresql.org/pgsql-bugs/2005-02/msg00023.php Another post on the internet indicates that you must change the authentication type to trust or else you will not be able to authenticate in a Windows environment. Install Postgres 8.x (http://www.postgresql.org/download/). Requires Windows 2000 or later and NTFS. See http://pginstaller.projects.postgresql.org/FAQ_windows.html for more info. * Create a regular windows user "postgres" with password "postgres". THIS MUST NOT be an administrator account since the postgres server WILL NOT RUN as an admin. * Install as a service. Use the above postgres account as owner of the service. * Check /your_dir/data/pg_hba.conf file. Change md5 or password methods to trust: o host all all md5 change this to: o host all all trust * If you don't make this change, then running psql commands will fail with: o psql: FATAL: password authentication failed for user "postgres" http://www.metatrontech.com/sql-ledger-wiki/index.php?WindowsInstallatio n I was very surprised by this issue as well. I am just assuming that not many people have a c:\dev directory on their machines where they run postgreSQL from. That is the only way that you would be able to open the file '/dev/tty', if the cwd is c:. If I switch my cwd to a drive that does not have a dev directory at the root, the problem goes away. I realize that this seems unbelievable, but I think it is a *real* problem. Let me know what you think. Robert -----Original Message----- From: Tom Lane [mailto:tgl@sss.pgh.pa.us] Sent: Wednesday, February 22, 2006 2:02 PM To: Robert Kinberg Cc: pgsql-patches@postgresql.org Subject: Re: [PATCHES] [PATCH] Prompt for password on Windows platforms "Robert Kinberg" <kinberg@tecore.com> writes: > The current version of psql, pg_dump, ... do not properly prompt the > user for a password, when the backend database is setup for password > authentication on a Windows platform. Curious that you are the *only* person to have noticed such a fundamental problem... please provide some evidence that this patch is needed. regards, tom lane
> I was very surprised by this issue as well. I am just > assuming that not many people have a c:\dev directory on > their machines where they run postgreSQL from. That is the > only way that you would be able to open the file '/dev/tty', > if the cwd is c:. > > If I switch my cwd to a drive that does not have a dev > directory at the root, the problem goes away. This sounds to me like a reasonable explanation to the fact that this works in most cases but not all. And that some have it working when they're on C: but not on a network drive or the other way around. And I can confirm this problem definitly exists. Haven't had time to test the patch, but: F:\Program Files\PostgreSQL\8.1\bin>psql Password: psql: FATAL: password authentication failed for user "maghag" F:\Program Files\PostgreSQL\8.1\bin>echo foo > \dev\tty F:\Program Files\PostgreSQL\8.1\bin>psql psql: FATAL: password authentication failed for user "maghag" F:\Program Files\PostgreSQL\8.1\bin>del \dev\tty F:\Program Files\PostgreSQL\8.1\bin>psql Password: psql: FATAL: password authentication failed for user "maghag" (notice that I got a chance to enter my password when \dev\tty did not exist, but when it does exist, it breaks. Yes, I forgot my password :P) So yes, it looks like this patch will be needed. A very good catch, Robert! This one has been annoying me for a long time! Tom - if you're unsure the patch fixes the problem, I'll try to test it soonest. But the problem definitly exists! //Magnus
> > I was very surprised by this issue as well. I am just assuming that > > not many people have a c:\dev directory on their machines > where they > > run postgreSQL from. That is the only way that you would be able to > > open the file '/dev/tty', if the cwd is c:. > > > > If I switch my cwd to a drive that does not have a dev directory at > > the root, the problem goes away. > > This sounds to me like a reasonable explanation to the fact > that this works in most cases but not all. And that some have > it working when they're on C: but not on a network drive or > the other way around. > > And I can confirm this problem definitly exists. Haven't had > time to test the patch, but: > So yes, it looks like this patch will be needed. A very good > catch, Robert! This one has been annoying me for a long time! > > Tom - if you're unsure the patch fixes the problem, I'll try > to test it soonest. But the problem definitly exists! I have now tested the patch, and it does work. Didn't apply cleanly, most likely because the mailer (either yours or mine) messed up - probably with the tabs. Attached is a version as an attachment which should survive this (win32_tty.patch). This patch fixes a longstanding issue. Anybody who has a \dev directory on the drive that happens to be current when executing psql will get broken password authentication without any error msg. I beleive this bug is responsible for most, if not all, the reports of this kind of issue we've seen on win32. The only reason we don't se eit all the time is that c:\dev isn't a very common directory on win32. But I'm sure several packages doing "I wanna look like unix" stuff creates one (cygwin, which breaks a lot of other things, doesn't though - they stick thereis in the cygwin directory. but there are others) Now, this bug can in theory affect all platforms not just win32 - any platform where /dev/tty is not a file, or when it doesn't exist but can be created (in which case it will be created the first time you run psql, and then it'll be used later). I've attached a second version of the patch (alternate_tty.patch) which I think could help in this case. But I haven't tested it on != win32. (Specifically, it's bad that we open /dev/tty for writing even if we failed it for reading (that will create a new file), and that we don't check if it's a tty at all). Since this bug is fairly bad for win32, please apply whichever version of this patch you prefer to HEAD and also to both 8.0 and 8.1 branches. //Magnus
Attachment
Magnus Hagander wrote: >>>I was very surprised by this issue as well. I am just assuming that >>>not many people have a c:\dev directory on their machines >>> >>> >>where they >> >> >>>run postgreSQL from. That is the only way that you would be able to >>>open the file '/dev/tty', if the cwd is c:. >>> >>>If I switch my cwd to a drive that does not have a dev directory at >>>the root, the problem goes away. >>> >>> >>This sounds to me like a reasonable explanation to the fact >>that this works in most cases but not all. And that some have >>it working when they're on C: but not on a network drive or >>the other way around. >> >>And I can confirm this problem definitly exists. Haven't had >>time to test the patch, but: >> >> > > > >>So yes, it looks like this patch will be needed. A very good >>catch, Robert! This one has been annoying me for a long time! >> >>Tom - if you're unsure the patch fixes the problem, I'll try >>to test it soonest. But the problem definitly exists! >> >> > >I have now tested the patch, and it does work. Didn't apply cleanly, >most likely because the mailer (either yours or mine) messed up - >probably with the tabs. Attached is a version as an attachment which >should survive this (win32_tty.patch). > >This patch fixes a longstanding issue. Anybody who has a \dev directory >on the drive that happens to be current when executing psql will get >broken password authentication without any error msg. I beleive this bug >is responsible for most, if not all, the reports of this kind of issue >we've seen on win32. > >The only reason we don't se eit all the time is that c:\dev isn't a very >common directory on win32. But I'm sure several packages doing "I wanna >look like unix" stuff creates one (cygwin, which breaks a lot of other >things, doesn't though - they stick thereis in the cygwin directory. but >there are others) > > >Now, this bug can in theory affect all platforms not just win32 - any >platform where /dev/tty is not a file, or when it doesn't exist but can >be created (in which case it will be created the first time you run >psql, and then it'll be used later). I've attached a second version of >the patch (alternate_tty.patch) which I think could help in this case. >But I haven't tested it on != win32. >(Specifically, it's bad that we open /dev/tty for writing even if we >failed it for reading (that will create a new file), and that we don't >check if it's a tty at all). > > >Since this bug is fairly bad for win32, please apply whichever version >of this patch you prefer to HEAD and also to both 8.0 and 8.1 branches. > > > > Maybe we should stat the file and check that it's actually a character special device. cheers andrew
> >>So yes, it looks like this patch will be needed. A very good catch, > >>Robert! This one has been annoying me for a long time! > >> > >>Tom - if you're unsure the patch fixes the problem, I'll > try to test > >>it soonest. But the problem definitly exists! > >> > >> > > > >I have now tested the patch, and it does work. Didn't apply cleanly, > >most likely because the mailer (either yours or mine) messed up - > >probably with the tabs. Attached is a version as an attachment which > >should survive this (win32_tty.patch). > > > >This patch fixes a longstanding issue. Anybody who has a > \dev directory > >on the drive that happens to be current when executing psql will get > >broken password authentication without any error msg. I beleive this > >bug is responsible for most, if not all, the reports of this kind of > >issue we've seen on win32. > > > >The only reason we don't se eit all the time is that c:\dev isn't a > >very common directory on win32. But I'm sure several > packages doing "I > >wanna look like unix" stuff creates one (cygwin, which > breaks a lot of > >other things, doesn't though - they stick thereis in the cygwin > >directory. but there are others) > > > > > >Now, this bug can in theory affect all platforms not just > win32 - any > >platform where /dev/tty is not a file, or when it doesn't > exist but can > >be created (in which case it will be created the first time you run > >psql, and then it'll be used later). I've attached a second > version of > >the patch (alternate_tty.patch) which I think could help in > this case. > >But I haven't tested it on != win32. > >(Specifically, it's bad that we open /dev/tty for writing even if we > >failed it for reading (that will create a new file), and > that we don't > >check if it's a tty at all). > > > > > >Since this bug is fairly bad for win32, please apply > whichever version > >of this patch you prefer to HEAD and also to both 8.0 and > 8.1 branches. > > > > > > > > > > Maybe we should stat the file and check that it's actually a > character special device. Wouldn't isatty() perform that kind of check already? //Magnus
Andrew Dunstan <andrew@dunslane.net> writes:
> Magnus Hagander wrote:
>> (Specifically, it's bad that we open /dev/tty for writing even if we
>> failed it for reading (that will create a new file), and that we don't
>> check if it's a tty at all).
> Maybe we should stat the file and check that it's actually a character
> special device.
On any sane Unix installation, /dev is not user-writable ... and I've
never heard of one without /dev/tty, either. So the above risk seems
illusory, except on Windows.
regards, tom lane
> >> (Specifically, it's bad that we open /dev/tty for writing > even if we > >> failed it for reading (that will create a new file), and that we > >> don't check if it's a tty at all). > > > Maybe we should stat the file and check that it's actually > a character > > special device. > > On any sane Unix installation, /dev is not user-writable ... > and I've never heard of one without /dev/tty, either. So the > above risk seems illusory, except on Windows. That was kind of my main guess, but I don't know enough different Unix flavours to state it. I'm sure you do :-) Given that, please just go with the win32-only solution. /Magnus
Updated patch applied. I found a use of /dev/tty in psql for command
history that I changed to stderr for Win32.
Thanks for the legwork in finding the cause of this bug. Backpatched to
8.1.X and 8.0.X.
---------------------------------------------------------------------------
Robert Kinberg wrote:
> The current version of psql, pg_dump, ... do not properly prompt the
> user for a password, when the backend database is setup for password
> authentication on a Windows platform. In investigating the code, I
> noticed that the file '/dev/tty' is opened to interact with the console.
> It appears as if on a Windows platform, the actual file /dev/tty is
> opened, instead of the console. I am supplying a patch to address this
> issue on the Windows platform.
>
> *** ./src/port/sprompt.c.orig Wed Feb 22 13:07:43 2006
> --- ./src/port/sprompt.c Wed Feb 22 13:08:14 2006
> ***************
> *** 40,47 ****
> {
> int length;
> char *destination;
> ! FILE *termin,
> ! *termout;
>
> #ifdef HAVE_TERMIOS_H
> struct termios t_orig,
> --- 40,47 ----
> {
> int length;
> char *destination;
> ! FILE *termin = NULL,
> ! *termout = NULL;
>
> #ifdef HAVE_TERMIOS_H
> struct termios t_orig,
> ***************
> *** 63,70 ****
> --- 63,72 ----
> * Do not try to collapse these into one "w+" mode file. Doesn't work
> on
> * some platforms (eg, HPUX 10.20).
> */
> + #ifndef WIN32
> termin = fopen("/dev/tty", "r");
> termout = fopen("/dev/tty", "w");
> + #endif
> if (!termin || !termout)
> {
> if (termin)
>
>
> Let me know what you think.
>
> Robert Kinberg
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
>
--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.161
diff -c -c -r1.161 command.c
*** src/bin/psql/command.c 12 Feb 2006 04:04:32 -0000 1.161
--- src/bin/psql/command.c 3 Mar 2006 23:38:05 -0000
***************
*** 753,760 ****
expand_tilde(&fname);
/* This scrolls off the screen when using /dev/tty */
success = saveHistory(fname ? fname : "/dev/tty");
!
if (success && !quiet && fname)
printf(gettext("Wrote history to file \"%s/%s\".\n"),
pset.dirname ? pset.dirname : ".", fname);
--- 753,763 ----
expand_tilde(&fname);
/* This scrolls off the screen when using /dev/tty */
+ #ifndef WIN32
success = saveHistory(fname ? fname : "/dev/tty");
! #else
! success = saveHistory(fname ? fname : stderr);
! #endif
if (success && !quiet && fname)
printf(gettext("Wrote history to file \"%s/%s\".\n"),
pset.dirname ? pset.dirname : ".", fname);
Index: src/port/sprompt.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/sprompt.c,v
retrieving revision 1.12
diff -c -c -r1.12 sprompt.c
*** src/port/sprompt.c 15 Oct 2005 02:49:51 -0000 1.12
--- src/port/sprompt.c 3 Mar 2006 23:38:06 -0000
***************
*** 40,47 ****
{
int length;
char *destination;
! FILE *termin,
! *termout;
#ifdef HAVE_TERMIOS_H
struct termios t_orig,
--- 40,47 ----
{
int length;
char *destination;
! FILE *termin = NULL,
! *termout = NULL;
#ifdef HAVE_TERMIOS_H
struct termios t_orig,
***************
*** 63,70 ****
--- 63,76 ----
* Do not try to collapse these into one "w+" mode file. Doesn't work on
* some platforms (eg, HPUX 10.20).
*/
+ #ifndef WIN32
+ /*
+ * Some win32 platforms actually have a /dev/tty file, but it isn't
+ * a device file, and it doesn't work as expected, so we avoid trying.
+ */
termin = fopen("/dev/tty", "r");
termout = fopen("/dev/tty", "w");
+ #endif
if (!termin || !termout)
{
if (termin)