Обсуждение: 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

> 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