psql with GSS can crash

Started by Zdenek Kotalaalmost 16 years ago6 messages

Zdenek.Kotala@Sun.COM

almost 16 years ago

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or
pghost is not filled. Following code in fe-auth.c pg_GSS_startup()
causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

Zdenek

Magnus Hagander

magnus@hagander.net

almost 16 years ago

In reply to: Zdenek Kotala (#1)

Re: psql with GSS can crash

On Thu, Feb 25, 2010 at 15:04, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or pghost is
not filled. Following code in fe-auth.c pg_GSS_startup() causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

How did you get NULL in there? :-)
There's a default set for that one that's PG_KRB_SRVNAM, so it really
should never come out as NULL, I think...

As for pghost, that certainly seems to be a bug. We check that one in
krb5 and SSPI, but for some reason we seem to be missing it in GSSAPI.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Zdenek Kotala

Zdenek.Kotala@Sun.COM

almost 16 years ago

In reply to: Magnus Hagander (#2)

Re: psql with GSS can crash

Magnus Hagander píše v čt 25. 02. 2010 v 15:17 +0100:

On Thu, Feb 25, 2010 at 15:04, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or pghost is
not filled. Following code in fe-auth.c pg_GSS_startup() causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

How did you get NULL in there? :-)
There's a default set for that one that's PG_KRB_SRVNAM, so it really
should never come out as NULL, I think...

Yeah, you are right. conn->krbsrvname is "postgres" and conn->pghost is
null

As for pghost, that certainly seems to be a bug. We check that one in
krb5 and SSPI, but for some reason we seem to be missing it in GSSAPI.

Yes. The check should be in GSSAPI too.

However what I see in pg_hba.conf is following line:

local all all gss

Gss is used on local unix socket which probably cause a problem that
conn->pghost is not filled when psql tries to connect.

thanks Zdenek

Zdenek

Magnus Hagander

magnus@hagander.net

almost 16 years ago

In reply to: Zdenek Kotala (#3)

1 attachment(s)

Re: psql with GSS can crash

2010/3/1 Zdenek Kotala <Zdenek.Kotala@sun.com>:

Magnus Hagander píše v čt 25. 02. 2010 v 15:17 +0100:

On Thu, Feb 25, 2010 at 15:04, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or pghost is
not filled. Following code in fe-auth.c pg_GSS_startup() causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

How did you get NULL in there? :-)
There's a default set for that one that's PG_KRB_SRVNAM, so it really
should never come out as NULL, I think...

Yeah, you are right. conn->krbsrvname is "postgres" and conn->pghost is
null

Ah, good. We should defentd against that then.

As for pghost, that certainly seems to be a bug. We check that one in
krb5 and SSPI, but for some reason we seem to be missing it in GSSAPI.

Yes. The check should be in GSSAPI too.

However what I see in pg_hba.conf is following line:

local all all gss

Gss is used on local unix socket which probably cause a problem that
conn->pghost is not filled when psql tries to connect.

So there are really two errors - because we should disallow that.

See attached patch - can you confirm it removes the crash with just
the client side applied, and then that it properly rejects GSS with
the server side applied as well?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Attachments:

gss_nohost.patchapplication/octet-stream; name=gss_nohost.patchDownload

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 4e12c78..e9bc525 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -988,6 +988,22 @@ parse_hba_line(List *line, int line_num, HbaLine *parsedline)
 		return false;
 	}
 
+	if (parsedline->conntype == ctLocal &&
+		parsedline->auth_method == uaGSS)
+	{
+		ereport(LOG,
+				(errcode(ERRCODE_CONFIG_FILE_ERROR),
+			 errmsg("gssapi authentication is not supported on local sockets"),
+				 errcontext("line %d of configuration file \"%s\"",
+							line_num, HbaFileName)));
+		return false;
+	}
+	/*
+	 * SSPI authentication can never be enabled on ctLocal connections, because
+	 * it's only supported on Windows, where ctLocal isn't supported.
+	 */
+
+
 	if (parsedline->conntype != ctHostSSL &&
 		parsedline->auth_method == uaCert)
 	{
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 6d35421..87aec04 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -426,6 +426,12 @@ pg_GSS_startup(PGconn *conn)
 	int			maxlen;
 	gss_buffer_desc temp_gbuf;
 
+	if (!conn->pghost)
+	{
+		printfPQExpBuffer(&conn->errorMessage, libpq_gettext("host name must be specified\n"));
+		return STATUS_ERROR;
+	}
+
 	if (conn->gctx)
 	{
 		printfPQExpBuffer(&conn->errorMessage,

Zdenek Kotala

Zdenek.Kotala@Sun.COM

almost 16 years ago

In reply to: Magnus Hagander (#4)

Re: psql with GSS can crash

Magnus Hagander píše v po 01. 03. 2010 v 16:55 +0100:

2010/3/1 Zdenek Kotala <Zdenek.Kotala@sun.com>:

Magnus Hagander píše v čt 25. 02. 2010 v 15:17 +0100:

On Thu, Feb 25, 2010 at 15:04, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or pghost is
not filled. Following code in fe-auth.c pg_GSS_startup() causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

How did you get NULL in there? :-)
There's a default set for that one that's PG_KRB_SRVNAM, so it really
should never come out as NULL, I think...

Yeah, you are right. conn->krbsrvname is "postgres" and conn->pghost is
null

Ah, good. We should defentd against that then.

As for pghost, that certainly seems to be a bug. We check that one in
krb5 and SSPI, but for some reason we seem to be missing it in GSSAPI.

Yes. The check should be in GSSAPI too.

However what I see in pg_hba.conf is following line:

local all all gss

Gss is used on local unix socket which probably cause a problem that
conn->pghost is not filled when psql tries to connect.

So there are really two errors - because we should disallow that.

See attached patch - can you confirm it removes the crash with just
the client side applied, and then that it properly rejects GSS with
the server side applied as well?

I tested it, but I cannot reproduce crash because I cannot setup illegal
combination now ;-). I think it is OK.

Thanks Zdenek

Magnus Hagander

magnus@hagander.net

almost 16 years ago

In reply to: Zdenek Kotala (#5)

Re: psql with GSS can crash

2010/3/7 Zdenek Kotala <Zdenek.Kotala@sun.com>:

Magnus Hagander píše v po 01. 03. 2010 v 16:55 +0100:

2010/3/1 Zdenek Kotala <Zdenek.Kotala@sun.com>:

Magnus Hagander píše v čt 25. 02. 2010 v 15:17 +0100:

On Thu, Feb 25, 2010 at 15:04, Zdenek Kotala <Zdenek.Kotala@sun.com> wrote:

Hi all,

I got following stack:

fffffd7ffed14b70 strlen () + 40
fffffd7ffed71665 snprintf () + e5
fffffd7fff36d088 pg_GSS_startup () + 88
fffffd7fff36d43a pg_fe_sendauth () + 15a
fffffd7fff36e557 PQconnectPoll () + 3b7
fffffd7fff36e152 connectDBComplete () + a2
fffffd7fff36dc32 PQsetdbLogin () + 1b2
000000000041e96d main () + 30d
000000000041302c ???????? ()

It seems that connection is not fully configured and krbsrvname or pghost is
not filled. Following code in fe-auth.c pg_GSS_startup() causes a crash:

440 maxlen = NI_MAXHOST + strlen(conn->krbsrvname) + 2;
441 temp_gbuf.value = (char *) malloc(maxlen);
442 snprintf(temp_gbuf.value, maxlen, "%s@%s",
443 conn->krbsrvname, conn->pghost);
444 temp_gbuf.length = strlen(temp_gbuf.value);

And following code in fe-connect.c fillPGconn() fill NULL value.

571 tmp = conninfo_getval(connOptions, "krbsrvname");
572 conn->krbsrvname = tmp ? strdup(tmp) : NULL;

I think that pg_GSS_startup should sanity the input.

How did you get NULL in there? :-)
There's a default set for that one that's PG_KRB_SRVNAM, so it really
should never come out as NULL, I think...

Yeah, you are right. conn->krbsrvname is "postgres" and conn->pghost is
null

Ah, good. We should defentd against that then.

As for pghost, that certainly seems to be a bug. We check that one in
krb5 and SSPI, but for some reason we seem to be missing it in GSSAPI.

Yes. The check should be in GSSAPI too.

However what I see in pg_hba.conf is following line:

local all all gss

Gss is used on local unix socket which probably cause a problem that
conn->pghost is not filled when psql tries to connect.

So there are really two errors - because we should disallow that.

See attached patch - can you confirm it removes the crash with just
the client side applied, and then that it properly rejects GSS with
the server side applied as well?

I tested it, but I cannot reproduce crash because I cannot setup illegal
combination now ;-). I think it is OK.

Ok, thanks for testing. I've been unable to break it in my testing as
well so - applied, and back-patched.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/