Skip to content

Commit d835462

Browse files
russcamrusscam
authored
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
1 parent a3efebf commit d835462

File tree

12 files changed

+133
-55
lines changed

12 files changed

+133
-55
lines changed

‎src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Diagnostics.CodeAnalysis;
88
using System.Net.Http;
99
using System.Net.Security;
10+
using System.Security;
1011
using System.Security.Cryptography.X509Certificates;
1112
using System.Threading;
1213
using Elasticsearch.Net.CrossPlatform;
@@ -21,7 +22,7 @@ namespace Elasticsearch.Net
2122
public class ConnectionConfiguration : ConnectionConfiguration<ConnectionConfiguration>
2223
{
2324
private static bool IsCurlHandler { get; } = typeof(HttpClientHandler).Assembly().GetType("System.Net.Http.CurlHandler") != null;
24-
25+
2526
public static readonly TimeSpan DefaultPingTimeout = TimeSpan.FromSeconds(2);
2627
public static readonly TimeSpan DefaultPingTimeoutOnSSL = TimeSpan.FromSeconds(5);
2728
public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);
@@ -72,7 +73,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
7273
private readonly NameValueCollection _queryString = new NameValueCollection();
7374
private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);
7475
private readonly ElasticsearchUrlFormatter _urlFormatter;
75-
76+
7677
private BasicAuthenticationCredentials _basicAuthCredentials;
7778
private X509CertificateCollection _clientCertificates;
7879
private Action<IApiCallDetails> _completedRequestHandler = DefaultCompletedRequestHandler;
@@ -93,7 +94,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
9394
private TimeSpan? _pingTimeout;
9495
private bool _prettyJson;
9596
private string _proxyAddress;
96-
private string _proxyPassword;
97+
private SecureString _proxyPassword;
9798
private string _proxyUsername;
9899
private TimeSpan _requestTimeout;
99100
private Func<object, X509Certificate, X509Chain, SslPolicyErrors, bool> _serverCertificateValidationCallback;
@@ -147,7 +148,7 @@ protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection co
147148
TimeSpan? IConnectionConfigurationValues.PingTimeout => _pingTimeout;
148149
bool IConnectionConfigurationValues.PrettyJson => _prettyJson;
149150
string IConnectionConfigurationValues.ProxyAddress => _proxyAddress;
150-
string IConnectionConfigurationValues.ProxyPassword => _proxyPassword;
151+
SecureString IConnectionConfigurationValues.ProxyPassword => _proxyPassword;
151152
string IConnectionConfigurationValues.ProxyUsername => _proxyUsername;
152153
NameValueCollection IConnectionConfigurationValues.QueryStringParameters => _queryString;
153154
IElasticsearchSerializer IConnectionConfigurationValues.RequestResponseSerializer => UseThisRequestResponseSerializer;
@@ -310,8 +311,16 @@ public T SniffOnConnectionFault(bool sniffsOnConnectionFault = true) =>
310311
/// <summary>
311312
/// If your connection has to go through proxy, use this method to specify the proxy url
312313
/// </summary>
313-
public T Proxy(Uri proxyAdress, string username, string password) =>
314-
Assign(proxyAdress.ToString(), (a, v) => a._proxyAddress = v)
314+
public T Proxy(Uri proxyAddress, string username, string password) =>
315+
Assign(proxyAddress.ToString(), (a, v) => a._proxyAddress = v)
316+
.Assign(username, (a, v) => a._proxyUsername = v)
317+
.Assign(password, (a, v) => a._proxyPassword = v.CreateSecureString());
318+
319+
/// <summary>
320+
/// If your connection has to go through proxy, use this method to specify the proxy url
321+
/// </summary>
322+
public T Proxy(Uri proxyAddress, string username, SecureString password) =>
323+
Assign(proxyAddress.ToString(), (a, v) => a._proxyAddress = v)
315324
.Assign(username, (a, v) => a._proxyUsername = v)
316325
.Assign(password, (a, v) => a._proxyPassword = v);
317326

@@ -369,8 +378,14 @@ public T OnRequestDataCreated(Action<RequestData> handler) =>
369378
/// <summary>
370379
/// Basic Authentication credentials to send with all requests to Elasticsearch
371380
/// </summary>
372-
public T BasicAuthentication(string userName, string password) =>
373-
Assign(new BasicAuthenticationCredentials { Username = userName, Password = password }, (a, v) => a._basicAuthCredentials = v);
381+
public T BasicAuthentication(string username, string password) =>
382+
Assign(new BasicAuthenticationCredentials(username, password), (a, v) => a._basicAuthCredentials = v);
383+
384+
/// <summary>
385+
/// Basic Authentication credentials to send with all requests to Elasticsearch
386+
/// </summary>
387+
public T BasicAuthentication(string username, SecureString password) =>
388+
Assign(new BasicAuthenticationCredentials(username, password), (a, v) => a._basicAuthCredentials = v);
374389

375390
/// <summary>
376391
/// Allows for requests to be pipelined. http://en.wikipedia.org/wiki/HTTP_pipelining

‎src/Elasticsearch.Net/Configuration/IConnectionConfigurationValues.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Collections.Specialized;
44
using System.Net.Security;
5+
using System.Security;
56
using System.Security.Cryptography.X509Certificates;
67
using System.Threading;
78

@@ -97,7 +98,7 @@ public interface IConnectionConfigurationValues : IDisposable
9798
TimeSpan? KeepAliveTime { get; }
9899

99100
/// <summary>
100-
/// The maximum ammount of time a node is allowed to marked dead
101+
/// The maximum amount of time a node is allowed to marked dead
101102
/// </summary>
102103
TimeSpan? MaxDeadTimeout { get; }
103104

@@ -158,7 +159,7 @@ public interface IConnectionConfigurationValues : IDisposable
158159
/// <summary>
159160
/// The password for the proxy, when configured
160161
/// </summary>
161-
string ProxyPassword { get; }
162+
SecureString ProxyPassword { get; }
162163

163164
/// <summary>
164165
/// The username for the proxy, when configured

‎src/Elasticsearch.Net/Configuration/RequestConfiguration.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Security;
34
using System.Security.Cryptography.X509Certificates;
45
using System.Threading;
56

@@ -257,6 +258,15 @@ public RequestConfigurationDescriptor MaxRetries(int retry)
257258
}
258259

259260
public RequestConfigurationDescriptor BasicAuthentication(string userName, string password)
261+
{
262+
if (Self.BasicAuthenticationCredentials == null)
263+
Self.BasicAuthenticationCredentials = new BasicAuthenticationCredentials();
264+
Self.BasicAuthenticationCredentials.Username = userName;
265+
Self.BasicAuthenticationCredentials.Password = password.CreateSecureString();
266+
return this;
267+
}
268+
269+
public RequestConfigurationDescriptor BasicAuthentication(string userName, SecureString password)
260270
{
261271
if (Self.BasicAuthenticationCredentials == null)
262272
Self.BasicAuthenticationCredentials = new BasicAuthenticationCredentials();

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,36 @@
1-
namespace Elasticsearch.Net
1+
using System.Security;
2+
3+
namespace Elasticsearch.Net
24
{
5+
/// <summary>
6+
/// Credentials for Basic Authentication
7+
/// </summary>
38
public class BasicAuthenticationCredentials
49
{
5-
public string Password { get; set; }
6-
public string Username { get; set; }
10+
public BasicAuthenticationCredentials()
11+
{
12+
}
13+
14+
public BasicAuthenticationCredentials(string username, string password)
15+
{
16+
Username = username;
17+
Password = password.CreateSecureString();
18+
}
719

8-
public override string ToString() => $"{Username}:{Password}";
20+
public BasicAuthenticationCredentials(string username, SecureString password)
21+
{
22+
Username = username;
23+
Password = password;
24+
}
25+
26+
/// <summary>
27+
/// The password with which to authenticate
28+
/// </summary>
29+
public SecureString Password { get; set; }
30+
31+
/// <summary>
32+
/// The username with which to authenticate
33+
/// </summary>
34+
public string Username { get; set; }
935
}
1036
}

‎src/Elasticsearch.Net/Connection/HttpConnection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ protected virtual void SetBasicAuthenticationIfNeeded(HttpRequestMessage request
253253
if (!requestData.Uri.UserInfo.IsNullOrEmpty())
254254
userInfo = Uri.UnescapeDataString(requestData.Uri.UserInfo);
255255
else if (requestData.BasicAuthorizationCredentials != null)
256-
userInfo = requestData.BasicAuthorizationCredentials.ToString();
256+
userInfo = $"{requestData.BasicAuthorizationCredentials.Username}:{requestData.BasicAuthorizationCredentials.Password.CreateString()}";
257257
if (!userInfo.IsNullOrEmpty())
258258
{
259259
var credentials = Convert.ToBase64String(Encoding.UTF8.GetBytes(userInfo));

‎src/Elasticsearch.Net/Connection/SecureStrings.cs‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
using System;
2+
using System.Runtime.InteropServices;
3+
using System.Security;
4+
5+
namespace Elasticsearch.Net
6+
{
7+
/// <summary>
8+
/// Methods for working with <see cref="SecureString"/>
9+
/// </summary>
10+
public static class SecureStrings
11+
{
12+
/// <summary>
13+
/// Creates a string from a secure string
14+
/// </summary>
15+
public static string CreateString(this SecureString secureString)
16+
{
17+
var num = IntPtr.Zero;
18+
if (secureString != null && secureString.Length != 0)
19+
{
20+
try
21+
{
22+
num = Marshal.SecureStringToBSTR(secureString);
23+
return Marshal.PtrToStringBSTR(num);
24+
}
25+
finally
26+
{
27+
if (num != IntPtr.Zero)
28+
Marshal.ZeroFreeBSTR(num);
29+
}
30+
}
31+
32+
return string.Empty;
33+
}
34+
35+
/// <summary>
36+
/// Creates a secure string from a string
37+
/// </summary>
38+
public static SecureString CreateSecureString(this string plainString)
39+
{
40+
var secureString = new SecureString();
41+
42+
if (plainString == null)
43+
return secureString;
44+
45+
foreach (var ch in plainString)
46+
secureString.AppendChar(ch);
47+
48+
return secureString;
49+
}
50+
}
51+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Collections.Specialized;
44
using System.IO;
55
using System.Linq;
6+
using System.Security;
67
using System.Security.Cryptography.X509Certificates;
78
using Elasticsearch.Net.Extensions;
89

@@ -105,7 +106,7 @@ IMemoryStreamFactory memoryStreamFactory
105106
public bool Pipelined { get; }
106107
public PostData PostData { get; }
107108
public string ProxyAddress { get; }
108-
public string ProxyPassword { get; }
109+
public SecureString ProxyPassword { get; }
109110
public string ProxyUsername { get; }
110111
public string RequestMimeType { get; }
111112
public TimeSpan RequestTimeout { get; }

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,9 @@ public CatIndicesApiNotFoundWithSecurityTests(XPackCluster cluster, EndpointUsag
4848
{
4949
RequestConfiguration = new RequestConfiguration
5050
{
51-
BasicAuthenticationCredentials = new BasicAuthenticationCredentials
52-
{
53-
Username = ClusterAuthentication.User.Username,
54-
Password = ClusterAuthentication.User.Password,
55-
}
51+
BasicAuthenticationCredentials = new BasicAuthenticationCredentials(
52+
ClusterAuthentication.User.Username,
53+
ClusterAuthentication.User.Password)
5654
}
5755
};
5856

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private static RequestData CreateRequestData(
7070
.EnableHttpCompression(httpCompression);
7171

7272
if (proxyAddress != null)
73-
connectionSettings.Proxy(proxyAddress, null, null);
73+
connectionSettings.Proxy(proxyAddress, null, (string)null);
7474

7575
var requestData = new RequestData(HttpMethod.GET, "/", null, connectionSettings, new PingRequestParameters(),
7676
new MemoryStreamFactory())

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

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
163163
},
164164
RequestConfiguration = new RequestConfiguration
165165
{
166-
BasicAuthenticationCredentials = new BasicAuthenticationCredentials
167-
{
168-
Username = $"user-{v}",
169-
Password = "password"
170-
}
166+
BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
171167
}
172168
},
173169
(v, d) => d
@@ -192,11 +188,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
192188
Expiration = "1d",
193189
RequestConfiguration = new RequestConfiguration
194190
{
195-
BasicAuthenticationCredentials = new BasicAuthenticationCredentials
196-
{
197-
Username = $"user-{v}",
198-
Password = "password"
199-
}
191+
BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
200192
}
201193
},
202194
(v, d) => d
@@ -218,11 +210,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
218210
Name = v,
219211
RequestConfiguration = new RequestConfiguration
220212
{
221-
BasicAuthenticationCredentials = new BasicAuthenticationCredentials
222-
{
223-
Username = $"user-{v}",
224-
Password = "password"
225-
}
213+
BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}", "password")
226214
}
227215
},
228216
(v, d) => d
@@ -243,11 +231,7 @@ public SecurityApiKeyTests(XPackCluster cluster, EndpointUsage usage) : base(new
243231
Name = v,
244232
RequestConfiguration = new RequestConfiguration
245233
{
246-
BasicAuthenticationCredentials = new BasicAuthenticationCredentials
247-
{
248-
Username = $"user-{v}",
249-
Password = "password"
250-
}
234+
BasicAuthenticationCredentials = new BasicAuthenticationCredentials($"user-{v}","password")
251235
}
252236
},
253237
(v, d) => d

0 commit comments

Comments
 (0)