GSSAPI support on Windows, attempt 4 #170

Merged
merged 5 commits into from Feb 22, 2014

Projects

None yet

2 participants

@fluggo
fluggo commented Feb 21, 2014

These patches add GSSAPI support on Windows, allow the connection string builder to get the user's Active Directory username in the proper case (and with optional realm), and close a security hole with regards to connection pooling and integrated security.

Guessing the user's Active Directory name requires an additional reference to to the System.DirectoryServices assembly, but from what I can tell, Mono supports this library.

Brian Crowell and others added some commits Nov 12, 2013
Brian Crowell Move responsibility for null-terminating password up to auth methods
The NpgsqlPasswordPacket class assumes that it needs to null-terminate all
passwords that come through, but GSSAPI and SSPI send binary data. I move
the null-termination logic up to the state class, where it can decide
whether a password needs to be null-terminated.
5b2d5c0
Brian Crowell Add GSSAPI support on Windows
For a Windows client to connect to a Linux-hosted PostgreSQL, it needs to
support GSSAPI. Fortunately, that just requires two changes:

* Use the hostname in the SPN
* Ask for the "kerberos" package instead of "negotiate"
110f812
Brian Crowell Guess a case-specific username and optionally include realm for integ…
…rated auth

When PostgreSQL receives our username, it will expect the case to match
Active Directory. For us to guess it correctly requires an LDAP lookup.

While I'm here, I'm also adding an "IncludeRealm" option to the connection
string to add the realm to the username if the include_realm option is
being used on the server side.

One real nasty thing about having the username set upon reading the
property is that it freezes the username at possibly bad times, such as
when you're examining the connection builder in the debugger. I'd fix that
now, but I'm worried about breaking too much.

WARNING: There's a serious bug here where one user could get access to
another user's cached connection if connection pooling is on and integrated
security is being used. This is because the user ID is not in the
connection string until the connection is opened, which is after any
pooling logic. I'll address this in the next patch.
1968ab8
Brian Crowell IMPORTANT: Freeze user name in connection string when using integrate…
…d security

When connection pooling is turned on, connections are cached by connection
string. But when integrated security is turned on, the user's identity
isn't part of the connection string, so two users can get each other's
connections back from the connection pool.

This patch forces the username to appear in the connection string if
integrated security is on, before any pooling takes place.
2eb7f55
@franciscojunior franciscojunior Fix PR #165.
NpgsqlConnectionStringBuilder was missing the Keyword ApplicationName in its valueDescriptions dictionary.
Thanks @pmuessig for the report and directions for the fix.
b66e174
@franciscojunior franciscojunior merged commit 894f3fc into npgsql:master Feb 22, 2014

1 check passed

default Finished TeamCity Build Npgsql :: All : Running
Details
@franciscojunior
Member

Hi, @fluggo !

Would you mind to have a look at this comment: #162 (comment)

User reports that the code is much slower than before (where it was simply fixed by the password value plus one byte more patch).

I think this slow down is caused by the ldap query. Maybe we could make this ldap query optional?
His case doesn't need this ldap as his code worked after fixing the password bug only.

Maybe there are scenarios where the information from the ldap is needed while on others it isn't.

What do you think?

@fluggo
fluggo commented Feb 24, 2014

The LDAP query should always be necessary to determine the correct casing to send to the server. His username probably worked before the query was implemented by the fact that it is all lowercase.

In pull request #172, I've implemented caching the LDAP query results for 30 seconds. That greatly speeds up repeated connection attempts.

@franciscojunior franciscojunior added the bug label Jul 23, 2014
@franciscojunior franciscojunior added this to the 2.2 milestone Jul 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment