User SecureString for Basic and Proxy passwords (#3806)

This commit changes the BasicAuthenticationCredentials and ProxyPassword types
to use SecureString for holding the passwords in memory, as opposed to as a plain text string.

A plain text string is retrieved from SecureString only at the point at which it is required.
This prevents plain text passwords from being automatically emitted in cases where properties
may be enumerated and captured e.g. logging frameworks.

Closes #2505

Author: russcam

src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

@@ -7,6 +7,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Net.Http;
 using System.Net.Security;
+using System.Security;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 using Elasticsearch.Net.CrossPlatform;
@@ -21,7 +22,7 @@ namespace Elasticsearch.Net
 	public class ConnectionConfiguration : ConnectionConfiguration<ConnectionConfiguration>
 	{
 		private static bool IsCurlHandler { get; } = typeof(HttpClientHandler).Assembly().GetType("System.Net.Http.CurlHandler") != null;
-		
+
 		public static readonly TimeSpan DefaultPingTimeout = TimeSpan.FromSeconds(2);
 		public static readonly TimeSpan DefaultPingTimeoutOnSSL = TimeSpan.FromSeconds(5);
 		public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);
@@ -72,7 +73,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
 		private readonly NameValueCollection _queryString = new NameValueCollection();
 		private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);
 		private readonly ElasticsearchUrlFormatter _urlFormatter;
-		
+
 		private BasicAuthenticationCredentials _basicAuthCredentials;
 		private X509CertificateCollection _clientCertificates;
 		private Action<IApiCallDetails> _completedRequestHandler = DefaultCompletedRequestHandler;
@@ -93,7 +94,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
 		private TimeSpan? _pingTimeout;
 		private bool _prettyJson;
 		private string _proxyAddress;
-		private string _proxyPassword;
+		private SecureString _proxyPassword;
 		private string _proxyUsername;
 		private TimeSpan _requestTimeout;
 		private Func<object, X509Certificate, X509Chain, SslPolicyErrors, bool> _serverCertificateValidationCallback;
@@ -147,7 +148,7 @@ protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection co
 		TimeSpan? IConnectionConfigurationValues.PingTimeout => _pingTimeout;
 		bool IConnectionConfigurationValues.PrettyJson => _prettyJson;
 		string IConnectionConfigurationValues.ProxyAddress => _proxyAddress;
-		string IConnectionConfigurationValues.ProxyPassword => _proxyPassword;
+		SecureString IConnectionConfigurationValues.ProxyPassword => _proxyPassword;
 		string IConnectionConfigurationValues.ProxyUsername => _proxyUsername;
 		NameValueCollection IConnectionConfigurationValues.QueryStringParameters => _queryString;
 		IElasticsearchSerializer IConnectionConfigurationValues.RequestResponseSerializer => UseThisRequestResponseSerializer;
@@ -310,8 +311,16 @@ public T SniffOnConnectionFault(bool sniffsOnConnectionFault = true) =>
 		/// <summary>
 		/// If your connection has to go through proxy, use this method to specify the proxy url
 		/// </summary>
-		public T Proxy(Uri proxyAdress, string username, string password) =>
-			Assign(proxyAdress.ToString(), (a, v) => a._proxyAddress = v)
+		public T Proxy(Uri proxyAddress, string username, string password) =>
+			Assign(proxyAddress.ToString(), (a, v) => a._proxyAddress = v)
+				.Assign(username, (a, v) => a._proxyUsername = v)
+				.Assign(password, (a, v) => a._proxyPassword = v.CreateSecureString());
+
+		/// <summary>
+		/// If your connection has to go through proxy, use this method to specify the proxy url
+		/// </summary>
+		public T Proxy(Uri proxyAddress, string username, SecureString password) =>
+			Assign(proxyAddress.ToString(), (a, v) => a._proxyAddress = v)
 				.Assign(username, (a, v) => a._proxyUsername = v)
 				.Assign(password, (a, v) => a._proxyPassword = v);
 
@@ -369,8 +378,14 @@ public T OnRequestDataCreated(Action<RequestData> handler) =>
 		/// <summary>
 		/// Basic Authentication credentials to send with all requests to Elasticsearch
 		/// </summary>
-		public T BasicAuthentication(string userName, string password) =>
-			Assign(new BasicAuthenticationCredentials { Username = userName, Password = password }, (a, v) => a._basicAuthCredentials = v);
+		public T BasicAuthentication(string username, string password) =>
+			Assign(new BasicAuthenticationCredentials(username, password), (a, v) => a._basicAuthCredentials = v);
+
+		/// <summary>
+		/// Basic Authentication credentials to send with all requests to Elasticsearch
+		/// </summary>
+		public T BasicAuthentication(string username, SecureString password) =>
+			Assign(new BasicAuthenticationCredentials(username, password), (a, v) => a._basicAuthCredentials = v);
 
 		/// <summary>
 		/// Allows for requests to be pipelined. http://en.wikipedia.org/wiki/HTTP_pipelining

src/Elasticsearch.Net/Configuration/IConnectionConfigurationValues.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Net.Security;
+using System.Security;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 
@@ -97,7 +98,7 @@ public interface IConnectionConfigurationValues : IDisposable
 		TimeSpan? KeepAliveTime { get; }
 
 		/// <summary>
-		/// The maximum ammount of time a node is allowed to marked dead
+		/// The maximum amount of time a node is allowed to marked dead
 		/// </summary>
 		TimeSpan? MaxDeadTimeout { get; }
 
@@ -158,7 +159,7 @@ public interface IConnectionConfigurationValues : IDisposable
 		/// <summary>
 		/// The password for the proxy, when configured
 		/// </summary>
-		string ProxyPassword { get; }
+		SecureString ProxyPassword { get; }
 
 		/// <summary>
 		/// The username for the proxy, when configured

src/Elasticsearch.Net/Configuration/RequestConfiguration.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Security;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 
@@ -257,6 +258,15 @@ public RequestConfigurationDescriptor MaxRetries(int retry)
 		}
 
 		public RequestConfigurationDescriptor BasicAuthentication(string userName, string password)
+		{
+			if (Self.BasicAuthenticationCredentials == null)
+				Self.BasicAuthenticationCredentials = new BasicAuthenticationCredentials();
+			Self.BasicAuthenticationCredentials.Username = userName;
+			Self.BasicAuthenticationCredentials.Password = password.CreateSecureString();
+			return this;
+		}
+
+		public RequestConfigurationDescriptor BasicAuthentication(string userName, SecureString password)
 		{
 			if (Self.BasicAuthenticationCredentials == null)
 				Self.BasicAuthenticationCredentials = new BasicAuthenticationCredentials();

src/Elasticsearch.Net/Configuration/Security/BasicAuthenticationCredentials.cs

@@ -1,10 +1,36 @@
-namespace Elasticsearch.Net
+using System.Security;
+
+namespace Elasticsearch.Net
 {
+	/// <summary>
+	/// Credentials for Basic Authentication
+	/// </summary>
 	public class BasicAuthenticationCredentials
 	{
-		public string Password { get; set; }
-		public string Username { get; set; }
+		public BasicAuthenticationCredentials()
+		{
+		}
+
+		public BasicAuthenticationCredentials(string username, string password)
+		{
+			Username = username;
+			Password = password.CreateSecureString();
+		}
 
-		public override string ToString() => $"{Username}:{Password}";
+		public BasicAuthenticationCredentials(string username, SecureString password)
+		{
+			Username = username;
+			Password = password;
+		}
+
+		/// <summary>
+		/// The password with which to authenticate
+		/// </summary>
+		public SecureString Password { get; set; }
+
+		/// <summary>
+		/// The username with which to authenticate
+		/// </summary>
+		public string Username { get; set; }
 	}
 }

src/Elasticsearch.Net/Connection/HttpConnection.cs

@@ -253,7 +253,7 @@ protected virtual void SetBasicAuthenticationIfNeeded(HttpRequestMessage request
 			if (!requestData.Uri.UserInfo.IsNullOrEmpty())
 				userInfo = Uri.UnescapeDataString(requestData.Uri.UserInfo);
 			else if (requestData.BasicAuthorizationCredentials != null)
-				userInfo = requestData.BasicAuthorizationCredentials.ToString();
+				userInfo = $"{requestData.BasicAuthorizationCredentials.Username}:{requestData.BasicAuthorizationCredentials.Password.CreateString()}";
 			if (!userInfo.IsNullOrEmpty())
 			{
 				var credentials = Convert.ToBase64String(Encoding.UTF8.GetBytes(userInfo));

src/Elasticsearch.Net/Connection/SecureStrings.cs

@@ -0,0 +1,51 @@
+using System;
+using System.Runtime.InteropServices;
+using System.Security;
+
+namespace Elasticsearch.Net
+{
+	/// <summary>
+	/// Methods for working with <see cref="SecureString"/>
+	/// </summary>
+	public static class SecureStrings
+	{
+		/// <summary>
+		/// Creates a string from a secure string
+		/// </summary>
+		public static string CreateString(this SecureString secureString)
+		{
+			var num = IntPtr.Zero;
+			if (secureString != null && secureString.Length != 0)
+			{
+				try
+				{
+					num = Marshal.SecureStringToBSTR(secureString);
+					return Marshal.PtrToStringBSTR(num);
+				}
+				finally
+				{
+					if (num != IntPtr.Zero)
+						Marshal.ZeroFreeBSTR(num);
+				}
+			}
+
+			return string.Empty;
+		}
+
+		/// <summary>
+		/// Creates a secure string from a string
+		/// </summary>
+		public static SecureString CreateSecureString(this string plainString)
+		{
+			var secureString = new SecureString();
+
+			if (plainString == null)
+				return secureString;
+
+			foreach (var ch in plainString)
+				secureString.AppendChar(ch);
+
+			return secureString;
+		}
+	}
+}

src/Elasticsearch.Net/Transport/Pipeline/RequestData.cs

@@ -3,6 +3,7 @@
 using System.Collections.Specialized;
 using System.IO;
 using System.Linq;
+using System.Security;
 using System.Security.Cryptography.X509Certificates;
 using Elasticsearch.Net.Extensions;
 
@@ -105,7 +106,7 @@ IMemoryStreamFactory memoryStreamFactory
 		public bool Pipelined { get; }
 		public PostData PostData { get; }
 		public string ProxyAddress { get; }
-		public string ProxyPassword { get; }
+		public SecureString ProxyPassword { get; }
 		public string ProxyUsername { get; }
 		public string RequestMimeType { get; }
 		public TimeSpan RequestTimeout { get; }

src/Tests/Tests/Cat/CatIndices/CatIndicesApiTests.cs

@@ -48,11 +48,9 @@ public CatIndicesApiNotFoundWithSecurityTests(XPackCluster cluster, EndpointUsag
 		{
 			RequestConfiguration = new RequestConfiguration
 			{
-				BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-				{
-					Username = ClusterAuthentication.User.Username,
-					Password = ClusterAuthentication.User.Password,
-				}
+				BasicAuthenticationCredentials = new BasicAuthenticationCredentials(
+					ClusterAuthentication.User.Username,
+					ClusterAuthentication.User.Password)
 			}
 		};
 

src/Tests/Tests/ClientConcepts/Connection/HttpConnectionTests.cs

@@ -70,7 +70,7 @@ private static RequestData CreateRequestData(
 				.EnableHttpCompression(httpCompression);
 
 			if (proxyAddress != null)
-				connectionSettings.Proxy(proxyAddress, null, null);
+				connectionSettings.Proxy(proxyAddress, null, (string)null);
 
 			var requestData = new RequestData(HttpMethod.GET, "/", null, connectionSettings, new PingRequestParameters(),
 				new MemoryStreamFactory())

src/Tests/Tests/XPack/Security/ApiKey/SecurityApiKeyTests.cs

@@ -163,11 +163,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
 							},
 							RequestConfiguration = new RequestConfiguration
 							{
-								BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-								{
-									Username = $"user-{v}",
-									Password = "password"
-								}
+								BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
 							}
 						},
 						(v, d) => d
@@ -192,11 +188,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
 							Expiration = "1d",
 							RequestConfiguration = new RequestConfiguration
 							{
-								BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-								{
-									Username = $"user-{v}",
-									Password = "password"
-								}
+								BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
 							}
 						},
 						(v, d) => d
@@ -218,11 +210,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
 							Name = v,
 							RequestConfiguration = new RequestConfiguration
 							{
-								BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-								{
-									Username = $"user-{v}",
-									Password = "password"
-								}
+								BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
 							}
 						},
 						(v, d) => d
@@ -243,11 +231,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
 							Name = v,
 							RequestConfiguration = new RequestConfiguration
 							{
-								BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-								{
-									Username = $"user-{v}",
-									Password = "password"
-								}
+								BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}","password")
 							}
 						},
 						(v, d) => d

src/Tests/Tests/XPack/Security/Authenticate/AuthenticateApiTests.cs

@@ -51,11 +51,9 @@ public AuthenticateRequestConfigurationApiTests(XPackCluster cluster, EndpointUs
 		{
 			RequestConfiguration = new RequestConfiguration
 			{
-				BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-				{
-					Username = ClusterAuthentication.User.Username,
-					Password = ClusterAuthentication.User.Password
-				}
+				BasicAuthenticationCredentials = new BasicAuthenticationCredentials(
+					ClusterAuthentication.User.Username,
+					ClusterAuthentication.User.Password)
 			}
 		};
 

src/Tests/Tests/XPack/Security/Privileges/ApplicationPrivilegesApiTests.cs

@@ -93,10 +93,7 @@ public ApplicationPrivilegesApiTests(XPackCluster cluster, EndpointUsage usage)
 					{
 						RequestConfiguration = new RequestConfiguration
 						{
-							BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-							{
-								Username = $"user-{v}", Password = $"pass-{v}"
-							}
+							BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", $"pass-{v}")
 						},
 						Application = new[]
 						{
@@ -130,10 +127,7 @@ public ApplicationPrivilegesApiTests(XPackCluster cluster, EndpointUsage usage)
 					{
 						RequestConfiguration = new RequestConfiguration
 						{
-							BasicAuthenticationCredentials = new BasicAuthenticationCredentials
-							{
-								Username = $"user-{v}", Password = $"pass-{v}"
-							}
+							BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", $"pass-{v}")
 						}
 					},
 					(v, d) => d.RequestConfiguration(r=>r.BasicAuthentication($"user-{v}", $"pass-{v}")),