From 5edac4816b80dc60468df819bc6dc9952ea9183f Mon Sep 17 00:00:00 2001 From: Brian Crowell Date: Tue, 12 Nov 2013 11:26:35 -0600 Subject: [PATCH 1/4] 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. --- src/Npgsql/NpgsqlPasswordPacket.cs | 12 ++++++------ src/Npgsql/NpgsqlState.cs | 15 +++++++++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Npgsql/NpgsqlPasswordPacket.cs b/src/Npgsql/NpgsqlPasswordPacket.cs index 270f768e4e..6ab647ec9f 100644 --- a/src/Npgsql/NpgsqlPasswordPacket.cs +++ b/src/Npgsql/NpgsqlPasswordPacket.cs @@ -61,19 +61,19 @@ public override void WriteToStream(Stream outputStream) { case ProtocolVersion.Version2: // Write the size of the packet. - // 4 + (passwordlength + 1) -> Int32 + NULL terminated string. - // output_stream.Write(BitConverter.GetBytes(IPAddress.HostToNetworkOrder(4 + (password.Length + 1))), 0, 4); + // 4 + (passwordlength) -> Int32 + Byte string. Null termination is done before we get here. + // output_stream.Write(BitConverter.GetBytes(IPAddress.HostToNetworkOrder(4 + (password.Length))), 0, 4); outputStream - .WriteInt32(4 + password.Length + 1) - .WriteBytesNullTerminated(password); + .WriteInt32(4 + password.Length) + .WriteBytes(password); break; case ProtocolVersion.Version3: outputStream .WriteBytes((Byte)ASCIIBytes.p) - .WriteInt32(4 + password.Length + 1) - .WriteBytesNullTerminated(password); + .WriteInt32(4 + password.Length) + .WriteBytes(password); break; } diff --git a/src/Npgsql/NpgsqlState.cs b/src/Npgsql/NpgsqlState.cs index 434401a70b..17fa28ea75 100644 --- a/src/Npgsql/NpgsqlState.cs +++ b/src/Npgsql/NpgsqlState.cs @@ -445,6 +445,13 @@ internal bool CheckForContextSocketAvailability (NpgsqlConnector context, Select return socketPoolResponse || context.Socket.Poll (1000000 * secondsToWait, selectMode); } + static byte[] NullTerminateArray(byte[] input) { + byte[] output = new byte[input.Length + 1]; + input.CopyTo(output, 0); + + return output; + } + protected IEnumerable ProcessBackendResponses_Ver_2(NpgsqlConnector context) { NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "ProcessBackendResponses"); @@ -498,7 +505,7 @@ protected IEnumerable ProcessBackendResponses_Ver_2(Npgsq NpgsqlEventLog.LogMsg(resman, "Log_AuthenticationClearTextRequest", LogLevel.Debug); // Send the PasswordPacket. ChangeState(context, NpgsqlStartupState.Instance); - context.Authenticate(context.Password); + context.Authenticate(NullTerminateArray(context.Password)); break; case AuthenticationRequestType.AuthenticationMD5Password: NpgsqlEventLog.LogMsg(resman, "Log_AuthenticationMD5Request", LogLevel.Debug); @@ -547,7 +554,7 @@ protected IEnumerable ProcessBackendResponses_Ver_2(Npgsq sb.Append(b.ToString("x2")); } - context.Authenticate(BackendEncoding.UTF8Encoding.GetBytes(sb.ToString())); + context.Authenticate(NullTerminateArray(BackendEncoding.UTF8Encoding.GetBytes(sb.ToString()))); break; default: @@ -737,7 +744,7 @@ protected IEnumerable ProcessBackendResponses_Ver_3(Npgsq // Send the PasswordPacket. ChangeState(context, NpgsqlStartupState.Instance); - context.Authenticate(context.Password); + context.Authenticate(NullTerminateArray(context.Password)); break; case AuthenticationRequestType.AuthenticationMD5Password: @@ -784,7 +791,7 @@ protected IEnumerable ProcessBackendResponses_Ver_3(Npgsq sb.Append(b.ToString("x2")); } - context.Authenticate(BackendEncoding.UTF8Encoding.GetBytes(sb.ToString())); + context.Authenticate(NullTerminateArray(BackendEncoding.UTF8Encoding.GetBytes(sb.ToString()))); break; #if WINDOWS && UNMANAGED From 02505d397e389674c7122b05fbd6e4bb617eb8dd Mon Sep 17 00:00:00 2001 From: Brian Crowell Date: Tue, 12 Nov 2013 11:28:18 -0600 Subject: [PATCH 2/4] 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" --- src/Npgsql/NpgsqlState.cs | 19 ++++++++++++++++++- src/Npgsql/SSPIHandler.cs | 4 ++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/Npgsql/NpgsqlState.cs b/src/Npgsql/NpgsqlState.cs index 17fa28ea75..321f748f35 100644 --- a/src/Npgsql/NpgsqlState.cs +++ b/src/Npgsql/NpgsqlState.cs @@ -796,13 +796,30 @@ protected IEnumerable ProcessBackendResponses_Ver_3(Npgsq break; #if WINDOWS && UNMANAGED + case AuthenticationRequestType.AuthenticationGSS: + { + if (context.IntegratedSecurity) + { + // For SSPI we have to get the IP-Address (hostname doesn't work) + context.SSPI = new SSPIHandler(context.Host, "POSTGRES", true); + ChangeState(context, NpgsqlStartupState.Instance); + context.Authenticate(context.SSPI.Continue(null)); + break; + } + else + { + // TODO: correct exception + throw new Exception(); + } + } + case AuthenticationRequestType.AuthenticationSSPI: { if (context.IntegratedSecurity) { // For SSPI we have to get the IP-Address (hostname doesn't work) string ipAddressString = ((IPEndPoint)context.Socket.RemoteEndPoint).Address.ToString(); - context.SSPI = new SSPIHandler(ipAddressString, "POSTGRES"); + context.SSPI = new SSPIHandler(ipAddressString, "POSTGRES", false); ChangeState(context, NpgsqlStartupState.Instance); context.Authenticate(context.SSPI.Continue(null)); break; diff --git a/src/Npgsql/SSPIHandler.cs b/src/Npgsql/SSPIHandler.cs index f61e3efe73..83de35ec27 100644 --- a/src/Npgsql/SSPIHandler.cs +++ b/src/Npgsql/SSPIHandler.cs @@ -118,7 +118,7 @@ ref SecHandle phContext private SecHandle sspictx; private bool sspictx_set; - public SSPIHandler(string pghost, string krbsrvname) + public SSPIHandler(string pghost, string krbsrvname, bool useGssapi) { if (pghost == null) throw new ArgumentNullException("pghost"); @@ -129,7 +129,7 @@ public SSPIHandler(string pghost, string krbsrvname) SecHandle expire; int status = AcquireCredentialsHandle( "", - "negotiate", + useGssapi ? "kerberos" : "negotiate", SECPKG_CRED_OUTBOUND, IntPtr.Zero, IntPtr.Zero, From 23a931d493d4abefc114ff8b646e023ea704d8db Mon Sep 17 00:00:00 2001 From: Brian Crowell Date: Tue, 12 Nov 2013 12:07:45 -0600 Subject: [PATCH 3/4] Guess a case-specific username and optionally include realm for integrated 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. --- src/Npgsql.csproj | 1 + src/Npgsql/NpgsqlConnectionStringBuilder.cs | 34 ++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/Npgsql.csproj b/src/Npgsql.csproj index 0eef897b71..4a92fcb7ca 100644 --- a/src/Npgsql.csproj +++ b/src/Npgsql.csproj @@ -146,6 +146,7 @@ + diff --git a/src/Npgsql/NpgsqlConnectionStringBuilder.cs b/src/Npgsql/NpgsqlConnectionStringBuilder.cs index e85642b8cd..4e5b4801ba 100644 --- a/src/Npgsql/NpgsqlConnectionStringBuilder.cs +++ b/src/Npgsql/NpgsqlConnectionStringBuilder.cs @@ -31,6 +31,7 @@ using System; using System.Collections.Generic; using System.Data.Common; +using System.DirectoryServices.AccountManagement; using System.Reflection; using System.Resources; using System.Text; @@ -69,6 +70,7 @@ static NpgsqlConnectionStringBuilder() defaults.Add(Keywords.PreloadReader, false); defaults.Add(Keywords.UseExtendedTypes, false); defaults.Add(Keywords.IntegratedSecurity, false); + defaults.Add(Keywords.IncludeRealm, false); defaults.Add(Keywords.Compatible, THIS_VERSION); defaults.Add(Keywords.ApplicationName, string.Empty); } @@ -276,9 +278,17 @@ public string UserName { if ((_integrated_security) && (String.IsNullOrEmpty(_username))) { - System.Security.Principal.WindowsIdentity identity = - System.Security.Principal.WindowsIdentity.GetCurrent(); - _username = identity.Name.Split('\\')[1]; + string[] usernameParts = UserPrincipal.Current.UserPrincipalName.Split('@'); + + // With the realm split out, decide whether to add it back + if (_includeRealm) + { + _username = usernameParts[0] + "@" + usernameParts[1].ToUpperInvariant(); + } + else + { + _username = usernameParts[0]; + } } return _username; } @@ -402,6 +412,13 @@ public bool IntegratedSecurity set { SetValue(GetKeyName(Keywords.IntegratedSecurity), value); } } + private bool _includeRealm; + public bool IncludeRealm + { + get { return _includeRealm; } + set { SetValue(GetKeyName(Keywords.IncludeRealm), value); } + } + private Version _compatible; private static readonly Version THIS_VERSION = @@ -486,6 +503,8 @@ private static Keywords GetKey(string key) return Keywords.UseExtendedTypes; case "INTEGRATED SECURITY": return Keywords.IntegratedSecurity; + case "INCLUDEREALM": + return Keywords.IncludeRealm; case "COMPATIBLE": return Keywords.Compatible; case "APPLICATIONNAME": @@ -543,6 +562,8 @@ internal static string GetKeyName(Keywords keyword) return "USEEXTENDEDTYPES"; case Keywords.IntegratedSecurity: return "INTEGRATED SECURITY"; + case Keywords.IncludeRealm: + return "INCLUDEREALM"; case Keywords.Compatible: return "COMPATIBLE"; default: @@ -702,6 +723,9 @@ private void SetValue(Keywords keyword, object value) case Keywords.IntegratedSecurity: this._integrated_security = ToIntegratedSecurity(value); break; + case Keywords.IncludeRealm: + this._includeRealm = ToBoolean(value); + break; case Keywords.Compatible: Version ver = new Version(value.ToString()); if (ver > THIS_VERSION) @@ -731,6 +755,7 @@ private void SetValue(Keywords keyword, object value) case Keywords.SSL: case Keywords.Pooling: case Keywords.SyncNotification: + case Keywords.IncludeRealm: exception_template = resman.GetString("Exception_InvalidBooleanKeyVal"); break; case Keywords.Protocol: @@ -789,7 +814,8 @@ public enum Keywords UseExtendedTypes, IntegratedSecurity, Compatible, - ApplicationName + ApplicationName, + IncludeRealm } public enum SslMode From 26195b5587b03fd61819946457ce00be3a3c391c Mon Sep 17 00:00:00 2001 From: Brian Crowell Date: Tue, 12 Nov 2013 12:10:49 -0600 Subject: [PATCH 4/4] IMPORTANT: Freeze user name in connection string when using integrated 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. --- src/Npgsql/NpgsqlConnection.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Npgsql/NpgsqlConnection.cs b/src/Npgsql/NpgsqlConnection.cs index f8ef238fc4..5e76856dc8 100644 --- a/src/Npgsql/NpgsqlConnection.cs +++ b/src/Npgsql/NpgsqlConnection.cs @@ -1035,6 +1035,13 @@ private void LoadConnectionStringBuilder(string connectionString) cache[connectionString] = settings; } + // Clone the settings, because if Integrated Security is enabled, user ID can be different + settings = settings.Clone(); + + // Set the UserName explicitly to freeze any Integrated Security-determined names + if(settings.IntegratedSecurity) + settings.UserName = settings.UserName; + RefreshConnectionString(); LogConnectionString(); }