BUG #8139: initdb: Misleading error message when current user not in /etc/passwd
The following bug has been logged on the website:
Bug reference: 8139
Logged by: Nicolas Marchildon
Email address: nicolas@marchildon.net
PostgreSQL version: 9.2.4
Operating system: RHEL 6
Description:
Running initdb while logged in as a user that has no entry in /etc/passwd,
which happens when authenticating with Kerberos, and missing sssd-client
prints a misleading error message:
"initdb: could not obtain information about current user: Success"
The misleading part is the "Success". This comes from errno:
pw = getpwuid(geteuid());
if (!pw)
{
fprintf(stderr,
_("%s: could not obtain information about current
user: %s\n"),
progname, strerror(errno));
exit(1);
}
The man page says:
RETURN VALUE
The getpwnam() and getpwuid() functions return a pointer to a
passwd
structure, or NULL if the matching entry is not found or an
error
occurs. If an error occurs, errno is set appropriately. If one
wants
to check errno after the call, it should be set to zero before
the
call.
First, initdb's get_id function does not set errno to zero, which is a bug.
Second, when the return value is NULL, it should only print strerror(errno)
when errno != 0.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
nicolas@marchildon.net writes:
"initdb: could not obtain information about current user: Success"
The misleading part is the "Success". This comes from errno:
pw = getpwuid(geteuid());
if (!pw)
{
fprintf(stderr,
_("%s: could not obtain information about current
user: %s\n"),
progname, strerror(errno));
exit(1);
}
The man page says:
RETURN VALUE
The getpwnam() and getpwuid() functions return a pointer to a
passwd
structure, or NULL if the matching entry is not found or an
error
occurs. If an error occurs, errno is set appropriately. If one
wants
to check errno after the call, it should be set to zero before
the
call.
AFAICS, getpwuid is not honoring its specification here: it failed to
set errno. I don't see that suppressing the strerror result would add
anything much.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Tom Lane wrote:
nicolas@marchildon.net writes:
The man page says:
RETURN VALUE
The getpwnam() and getpwuid() functions return a pointer to a
passwd
structure, or NULL if the matching entry is not found or an
error
occurs. If an error occurs, errno is set appropriately. If one
wants
to check errno after the call, it should be set to zero before
the
call.AFAICS, getpwuid is not honoring its specification here: it failed to
set errno. I don't see that suppressing the strerror result would add
anything much.
Well, in this case no error occured, but no matching entry was found.
The wording in the manpage is explicit that there not being an entry is
not an error.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Tue, May 7, 2013 at 02:19:05PM -0400, Alvaro Herrera wrote:
Tom Lane wrote:
nicolas@marchildon.net writes:
The man page says:
RETURN VALUE
The getpwnam() and getpwuid() functions return a pointer to a
passwd
structure, or NULL if the matching entry is not found or an
error
occurs. If an error occurs, errno is set appropriately. If one
wants
to check errno after the call, it should be set to zero before
the
call.AFAICS, getpwuid is not honoring its specification here: it failed to
set errno. I don't see that suppressing the strerror result would add
anything much.Well, in this case no error occured, but no matching entry was found.
The wording in the manpage is explicit that there not being an entry is
not an error.
I have developed the attached patch to fix this and another instance I
saw.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
passwd.difftext/x-diff; charset=us-asciiDownload+7-7
Bruce Momjian wrote:
On Tue, May 7, 2013 at 02:19:05PM -0400, Alvaro Herrera wrote:
I have developed the attached patch to fix this and another instance I
saw.
Remember to set errno = 0 before calling the getpw* function; at least
in initdb it would be meaningful. And in the pg_upgrade case, it seems
better to abort the upgrade if this call doesn't work. (Also, there are
other uses of getpwuid/getpwnam elsewhere. Not sure we want to worry
too much about them.)
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Bruce Momjian <bruce@momjian.us> writes:
I have developed the attached patch to fix this and another instance I
saw.
I think you need to re-read the part of the man page you quoted,
and set errno to zero beforehand, if you're going to rely on it
being zero after a non-error return.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Wed, Dec 4, 2013 at 04:03:39PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
On Tue, May 7, 2013 at 02:19:05PM -0400, Alvaro Herrera wrote:
I have developed the attached patch to fix this and another instance I
saw.Remember to set errno = 0 before calling the getpw* function; at least
in initdb it would be meaningful. And in the pg_upgrade case, it seems
better to abort the upgrade if this call doesn't work. (Also, there are
other uses of getpwuid/getpwnam elsewhere. Not sure we want to worry
too much about them.)
Wow, that is an odd API. Once I started to look at all the errno
clearing necessary, I realized that the function in bin/scripts/common.c
could be moved to src/port and generalized to reduce code duplication.
Updated patch attached, with centralized checking for errno and more
consistent behavior for username lookups.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
passwd.difftext/x-diff; charset=us-asciiDownload+176-195
Bruce Momjian <bruce@momjian.us> writes:
Updated patch attached, with centralized checking for errno and more
consistent behavior for username lookups.
This bit is not good:
+ errno = 0; /* clear errno before call */
+ pw = getpwuid(geteuid());
+ if (!pw)
+ {
+ *errstr = psprintf(_("effective user id %d lookup failure: %s"),
+ (int) geteuid(), errno ? strerror(errno) :
+ _("user does not exist"));The second call of geteuid() could clobber errno before strerror can see it.
You should compute geteuid() just once and keep it in a variable.
Also, I don't think that error message meets our style guidelines.
Maybe "failed to look up user id %d: ..." ?
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Bruce Momjian wrote:
return STATUS_ERROR;
}! user_name = get_user_name(&errstr);
! if (!user_name)
{
! ereport(LOG, (errmsg("%s\n", errstr)));
! pfree(errstr);
return STATUS_ERROR;
}
The message is already translated by get_user_name, so I think this
should use errmsg_internal() instead of errmsg(). Also, why do you add
a newline?
Not clear whether the new file should be in src/port or src/common.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Mon, Dec 9, 2013 at 04:35:56PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Updated patch attached, with centralized checking for errno and more
consistent behavior for username lookups.This bit is not good:
+ errno = 0; /* clear errno before call */ + pw = getpwuid(geteuid()); + if (!pw) + { + *errstr = psprintf(_("effective user id %d lookup failure: %s"), + (int) geteuid(), errno ? strerror(errno) : + _("user does not exist"));The second call of geteuid() could clobber errno before strerror can see it.
You should compute geteuid() just once and keep it in a variable.
Good point --- fixed.
Also, I don't think that error message meets our style guidelines.
Maybe "failed to look up user id %d: ..." ?
OK, updated.
Updated patch attached to Alvaro's email reply.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Mon, Dec 9, 2013 at 06:45:39PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
return STATUS_ERROR;
}! user_name = get_user_name(&errstr);
! if (!user_name)
{
! ereport(LOG, (errmsg("%s\n", errstr)));
! pfree(errstr);
return STATUS_ERROR;
}The message is already translated by get_user_name, so I think this
should use errmsg_internal() instead of errmsg(). Also, why do you add
a newline?
OK, done.
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.
Updated patch attached, with Tom's requested changes.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
passwd.difftext/x-diff; charset=us-asciiDownload+177-195
On Mon, Dec 9, 2013 at 07:47:34PM -0500, Bruce Momjian wrote:
On Mon, Dec 9, 2013 at 06:45:39PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
return STATUS_ERROR;
}! user_name = get_user_name(&errstr);
! if (!user_name)
{
! ereport(LOG, (errmsg("%s\n", errstr)));
! pfree(errstr);
return STATUS_ERROR;
}The message is already translated by get_user_name, so I think this
should use errmsg_internal() instead of errmsg(). Also, why do you add
a newline?OK, done.
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.Updated patch attached, with Tom's requested changes.
Patch applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On 12/9/13, 7:47 PM, Bruce Momjian wrote:
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.
It's not for portability, though, is it?
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Dec 20, 2013 at 10:46:43AM -0500, Peter Eisentraut wrote:
On 12/9/13, 7:47 PM, Bruce Momjian wrote:
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.It's not for portability, though, is it?
Well, neither is sprompt.c, but that has a lot of port-specific code in
it, so I used that as a guide.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Bruce Momjian wrote:
On Fri, Dec 20, 2013 at 10:46:43AM -0500, Peter Eisentraut wrote:
On 12/9/13, 7:47 PM, Bruce Momjian wrote:
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.It's not for portability, though, is it?
Well, neither is sprompt.c, but that has a lot of port-specific code in
it, so I used that as a guide.
src/common was created much later than sprompt.c was written. I would
have thought that the consideration would have been that sprompt.c
should eventually be moved to src/common; not that it would serve as a
precedent for anything.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Tue, Jan 7, 2014 at 06:40:14PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
On Fri, Dec 20, 2013 at 10:46:43AM -0500, Peter Eisentraut wrote:
On 12/9/13, 7:47 PM, Bruce Momjian wrote:
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.It's not for portability, though, is it?
Well, neither is sprompt.c, but that has a lot of port-specific code in
it, so I used that as a guide.src/common was created much later than sprompt.c was written. I would
have thought that the consideration would have been that sprompt.c
should eventually be moved to src/common; not that it would serve as a
precedent for anything.
Are we not moving items over to common where appropriate? Are we
worried about bring external applications?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Bruce Momjian wrote:
On Tue, Jan 7, 2014 at 06:40:14PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
On Fri, Dec 20, 2013 at 10:46:43AM -0500, Peter Eisentraut wrote:
On 12/9/13, 7:47 PM, Bruce Momjian wrote:
Not clear whether the new file should be in src/port or src/common.
Agreed. It isn't designed to add missing OS functionality, but it is
mostly OS-specific code.It's not for portability, though, is it?
Well, neither is sprompt.c, but that has a lot of port-specific code in
it, so I used that as a guide.src/common was created much later than sprompt.c was written. I would
have thought that the consideration would have been that sprompt.c
should eventually be moved to src/common; not that it would serve as a
precedent for anything.Are we not moving items over to common where appropriate?
I don't think we're moving code from src/port to src/common just for the
heck of it; but ISTM if we're adding new code which belongs to
src/common, and there's a natural file for it in src/port which should
arguably also be in src/common, then it makes sense to put both the new
code and the old file together in src/common.
Note that nothing in src/port should depend on stuff in src/common. As
I see it, src/port is very bare-bones stuff which src/common builds on
top of. It might also make sense, in some cases, to consider low-level
routines in src/port that are used by higher level routines in
src/common.
Are we worried about bring external applications?
Please rephrase. What are we worried about?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Jan 10, 2014 at 05:23:43PM -0300, Alvaro Herrera wrote:
Are we not moving items over to common where appropriate?
I don't think we're moving code from src/port to src/common just for the
heck of it; but ISTM if we're adding new code which belongs to
src/common, and there's a natural file for it in src/port which should
arguably also be in src/common, then it makes sense to put both the new
code and the old file together in src/common.Note that nothing in src/port should depend on stuff in src/common. As
I see it, src/port is very bare-bones stuff which src/common builds on
top of. It might also make sense, in some cases, to consider low-level
routines in src/port that are used by higher level routines in
src/common.
OK, C file moved from /port to /common.
Are we worried about bring external applications?
Please rephrase. What are we worried about?
I was asking if we did not move old functions from port to common
because of concern about breaking 3rd party applications.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, 2014-01-10 at 18:04 -0500, Bruce Momjian wrote:
OK, C file moved from /port to /common.
Please fix the new compiler warnings in pg_upgrade.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Jan 10, 2014 at 08:49:18PM -0500, Peter Eisentraut wrote:
On Fri, 2014-01-10 at 18:04 -0500, Bruce Momjian wrote:
OK, C file moved from /port to /common.
Please fix the new compiler warnings in pg_upgrade.
Thanks, fixed.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
