Kerberos brokenness and oops question in 8.1beta2
Hi!
First of all, Kerberos v5 is quite broken in 8.1 beta2. The patch to
allow virtual hosts to be specifed quite efficiently broke everything
case that wasn't using it. I'm working on a patch for this, should be
ready by tomorrow.
(sorry didn't notice earlier, haven't started looking at putting my
kerberos-based systems on 8.1 until just now..)
This is clearly a must-fix.
The second point is an "oops" I introduced in my own kerberos patch
earlier on in 8.1, that I stumbled upon now.
Summary: When talking kerberos in particular you have a service
principal name, which is made out of hostname, service name and realm.
Service name is what's interesting here. It was previously set using
--with-krb-srvnam, and in 8.1 we can set it with a config option in
postgresql.conf.
PostgreSQl uses the calls to krb5_sendauth/krb5_recvauth to
authenticate. These calls take another parameter (that's not strictly in
the kerberos protocol, IIRC, just in the API) called "application name".
If this mismatches, authentication will fail.
Now, in <= 8.0, this application name was set to the same as the service
name (default "postgres", but in an active directory deployment for
example, it would be "POSTGRES"). This is not correct (the app name
doesn't change..), but that's how it has been.
In 8.1, this was changed to be hardcoded to "postgres". In part this was
done to prevent a very very simple way to cause a security issue in the
MIT kerberos libs. This issue has been fixed now, but I forgot all about
it. The other reason is that it's more up to api specs now.
Anyway. This makes it impossible for a 8.1 client to connect to a 8.0
server, or a 8.0 client to a 8.1 server, in any case where the service
name has changed - such as a win32 active directory deployment, but I'm
sure many others as well.
The only real advantage to how it is now is that it's "cleaner". The
argument that it protects against a security hole in MIT KRB5 doesn't
hold any more because there is a patch out, and we can't take
responsibility for people who haven't patched.
So, the bottom line is that I'd like to reverse this part of the patch
out, and go back to using the service name as the application name. I
don't think it's worth to lose the backwards compatibility just to make
it "a tiny bit cleaner".
Comments on this? I'd like to include this in the other kerberos fix I'm
working on, and have this in before beta3 if approved.
//Magnus
"Magnus Hagander" <mha@sollentuna.net> writes:
Anyway. This makes it impossible for a 8.1 client to connect to a 8.0
server, or a 8.0 client to a 8.1 server, in any case where the service
name has changed - such as a win32 active directory deployment, but I'm
sure many others as well.
How important is that really? How many win32 users are likely to be
using Kerberos auth with 8.0?
The only real advantage to how it is now is that it's "cleaner". The
argument that it protects against a security hole in MIT KRB5 doesn't
hold any more because there is a patch out, and we can't take
responsibility for people who haven't patched.
I don't really buy that argument. ISTM we should fix the code to do the
right thing, especially if the right thing is more secure. If I
understood what you said properly, hardwiring it as "postgres" is the
correct thing, and loss of compatibility in marginal cases is just the
price we pay for having done it wrong originally.
regards, tom lane
Anyway. This makes it impossible for a 8.1 client to
connect to a 8.0
server, or a 8.0 client to a 8.1 server, in any case where
the service
name has changed - such as a win32 active directory deployment, but
I'm sure many others as well.How important is that really? How many win32 users are
likely to be using Kerberos auth with 8.0?
Not all that many - especially since it required a recompile to work
with AD. But some, I know of at least a couple who have mailed me about
instructions on how to do it, for example.
I don't know how many other cases changed principal names are used in,
though - we had the functionality to change it in the backend long
before we supported kerberos on windows.
The only real advantage to how it is now is that it's
"cleaner". The
argument that it protects against a security hole in MIT
KRB5 doesn't
hold any more because there is a patch out, and we can't take
responsibility for people who haven't patched.I don't really buy that argument. ISTM we should fix the
code to do the right thing, especially if the right thing is
more secure. If I understood what you said properly,
hardwiring it as "postgres" is the correct thing, and loss of
compatibility in marginal cases is just the price we pay for
having done it wrong originally.
I said it was probably cleaner, which may or may not be the same as
"correct". It's very hard to find good documentation about the
krb5_sendauth/recvauth calls, so I'm not very sure about that - that's
why I'm asking before coding. The best I've found now that I searched
some more states:
"The paramter appl_version is a string describing the application
protocol version which the client is expecting to use for this exchange.
If the server is using a different application protocol, an error will
be returned."
But we already deal with protocol versions outside of this, so there's
not need ot use that functionality. Then again, there is nothing in the
spec that prevents us from using it the way we have previously done
either.
Quick code-check shows that if we set it to NULL instead it disables the
check on MIT Kerberos for to fix exactly this kind of issue, but it
looks like it would cause a crash on Heimdal, so that's not realliy a
good idea either.
The point is I'm having a hard time seeing what the actual gain is in
not changing it back. If the principal name mismatches, we're going to
get rejected anyway, so it's not really a problem there. Even though the
gain in changing it back isn't all that big either, why should we
introduce abackwards-incompatibility if there is no real gain in a
different part of the code.
//Magnus
Import Notes
Resolved by subject fallback
The point is I'm having a hard time seeing what the actual
gain is in not changing it back. If the principal name
mismatches, we're going to get rejected anyway, so it's not
really a problem there. Even though the gain in changing it
back isn't all that big either, why should we introduce
abackwards-incompatibility if there is no real gain in a
different part of the code.
Here's a patch that fixes the big problem and reverts the behaviour of
appl_version to be compatible with 8.0. It's easy enough to isolate the
changes that are around the appl_version - one line in
backend/libpq/auth.c call to krb5_recvauth and one in
interfaces/libpq/fe-auth.c call to krb5_sendauth.
The call in backend/libpq/auth.c to krb5_sname_to_principal in 8.1beta2
was completely broken for a scenario where you *didn't* use virtual
hosts, by setting pg_krb5_server to NULL... The call is needed there as
well.
//Magnus
Attachments:
krb5fix.patchapplication/octet-stream; name=krb5fix.patchDownload
Index: backend/libpq/auth.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.127
diff -c -r1.127 auth.c
*** backend/libpq/auth.c 25 Jul 2005 04:52:31 -0000 1.127
--- backend/libpq/auth.c 8 Oct 2005 14:52:26 -0000
***************
*** 145,169 ****
return STATUS_ERROR;
}
! if (pg_krb_server_hostname)
{
! retval = krb5_sname_to_principal(pg_krb5_context,
! pg_krb_server_hostname, pg_krb_srvnam,
! KRB5_NT_SRV_HST, &pg_krb5_server);
! if (retval)
! {
! ereport(LOG,
! (errmsg("Kerberos sname_to_principal(\"%s\") returned error %d",
! pg_krb_srvnam, retval)));
! com_err("postgres", retval,
! "while getting server principal for service \"%s\"",
! pg_krb_srvnam);
! krb5_kt_close(pg_krb5_context, pg_krb5_keytab);
! krb5_free_context(pg_krb5_context);
! return STATUS_ERROR;
! }
! } else
! pg_krb5_server = NULL;
pg_krb5_initialised = 1;
return STATUS_OK;
--- 145,168 ----
return STATUS_ERROR;
}
! /* If no hostname is specified, pg_krb_server_hostname is already
! NULL. If it's set to blank, force it to NULL. */
! retval = krb5_sname_to_principal(pg_krb5_context,
! (pg_krb_server_hostname && pg_krb_server_hostname[0]=='\0')?NULL:pg_krb_server_hostname,
! pg_krb_srvnam,
! KRB5_NT_SRV_HST, &pg_krb5_server);
! if (retval)
{
! ereport(LOG,
! (errmsg("Kerberos sname_to_principal(\"%s\") returned error %d",
! pg_krb_srvnam, retval)));
! com_err("postgres", retval,
! "while getting server principal for service \"%s\"",
! pg_krb_srvnam);
! krb5_kt_close(pg_krb5_context, pg_krb5_keytab);
! krb5_free_context(pg_krb5_context);
! return STATUS_ERROR;
! }
pg_krb5_initialised = 1;
return STATUS_OK;
***************
*** 194,200 ****
return ret;
retval = krb5_recvauth(pg_krb5_context, &auth_context,
! (krb5_pointer) & port->sock, "postgres",
pg_krb5_server, 0, pg_krb5_keytab, &ticket);
if (retval)
{
--- 193,199 ----
return ret;
retval = krb5_recvauth(pg_krb5_context, &auth_context,
! (krb5_pointer) & port->sock, pg_krb_srvnam,
pg_krb5_server, 0, pg_krb5_keytab, &ticket);
if (retval)
{
Index: interfaces/libpq/fe-auth.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.103
diff -c -r1.103 fe-auth.c
*** interfaces/libpq/fe-auth.c 30 Jun 2005 01:59:20 -0000 1.103
--- interfaces/libpq/fe-auth.c 8 Oct 2005 14:57:47 -0000
***************
*** 280,286 ****
}
retval = krb5_sendauth(pg_krb5_context, &auth_context,
! (krb5_pointer) & sock, "postgres",
pg_krb5_client, server,
AP_OPTS_MUTUAL_REQUIRED,
NULL, 0, /* no creds, use ccache instead */
--- 280,286 ----
}
retval = krb5_sendauth(pg_krb5_context, &auth_context,
! (krb5_pointer) & sock, servicename,
pg_krb5_client, server,
AP_OPTS_MUTUAL_REQUIRED,
NULL, 0, /* no creds, use ccache instead */
Import Notes
Resolved by subject fallback
"Magnus Hagander" <mha@sollentuna.net> writes:
Here's a patch that fixes the big problem and reverts the behaviour of
appl_version to be compatible with 8.0.
Applied with trivial stylistic cleanups.
BTW, the documentation seems a bit broken:
krb_server_hostname (string)
Sets the hostname part of the service principal. This, combined
with krb_srvname, is used to generate the complete service
principal, i.e. krb_server_hostname/krb_server_hostname@REALM.
I suppose one of those last two should be "krb_srvname", but which?
regards, tom lane
I need a comment on this.
---------------------------------------------------------------------------
Tom Lane wrote:
"Magnus Hagander" <mha@sollentuna.net> writes:
Here's a patch that fixes the big problem and reverts the behaviour of
appl_version to be compatible with 8.0.Applied with trivial stylistic cleanups.
BTW, the documentation seems a bit broken:
krb_server_hostname (string)
Sets the hostname part of the service principal. This, combined
with krb_srvname, is used to generate the complete service
principal, i.e. krb_server_hostname/krb_server_hostname@REALM.I suppose one of those last two should be "krb_srvname", but which?
regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073This has been fixed in current CVS:
<varname>krb_srvname</><literal>/</><varname>krb_server_hostname</><literal>@</>REALM.
---------------------------------------------------------------------------
Tom Lane wrote:
"Magnus Hagander" <mha@sollentuna.net> writes:
Here's a patch that fixes the big problem and reverts the behaviour of
appl_version to be compatible with 8.0.Applied with trivial stylistic cleanups.
BTW, the documentation seems a bit broken:
krb_server_hostname (string)
Sets the hostname part of the service principal. This, combined
with krb_srvname, is used to generate the complete service
principal, i.e. krb_server_hostname/krb_server_hostname@REALM.I suppose one of those last two should be "krb_srvname", but which?
regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073