GSSAPI support on Windows, attempt 1#95
Closed
fluggo wants to merge 4 commits into
Closed
Conversation
added 4 commits
November 12, 2013 11:26
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.
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"
…rated auth When PostgreSQL receives our username, it will expect the case to match Active Directory. For us to guess it correctly requires a lookup into Active Directory. .NET makes that easy; I don't know how well this ports to Mono. 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. WARNING: There's a serious bug here where one user could get access to another user's cached connection is 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.
…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.
Member
|
Hi, Brian!! Would you mind to rebase your pull request? Also, Npgsql needs to be compatible with .net 2.0. And we got this error when compiling your pr with .net 2.0: Npgsql\NpgsqlConnectionStringBuilder.cs(34, 14): error CS0234: The type or namespace name 'DirectoryServices' does not exist in the namespace 'System' (are you missing an assembly reference?) Thanks in advance. |
Author
|
That error-- the assembly for System.DirectoryServices.AccountManagement unfortunately was introduced in .NET 3.5. I'm going to have to go back and rewrite that with lower-level code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.AccountManagement library, but from what I can tell, Mono supports this library. If that's a deal breaker, the 23a931d commit can be omitted, but 26195b5 is critical.