Skip to content

Commit 29eb3a5

Browse files
authored
Use Marshal.SecureStringToGlobalAllocUnicode (#3825)
* Use Marshal.SecureStringToGlobalAllocUnicode Relates: #3806 (comment) This commit updates the SecureStrings implementation to use the cross platform Marshal methods that deal with unmanaged unicode strings. Implement IDisposable on BasicAuthenticationCredentials and dispose of SecureString instances when ConnectionConfiguration is disposed.
1 parent b6d9cf6 commit 29eb3a5

File tree

4 files changed

+49
-15
lines changed

4 files changed

+49
-15
lines changed

src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,8 @@ protected virtual void DisposeManagedResources()
513513
_connectionPool?.Dispose();
514514
_connection?.Dispose();
515515
_semaphore?.Dispose();
516+
_proxyPassword?.Dispose();
517+
_basicAuthCredentials?.Dispose();
516518
}
517519
}
518520
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
using System.Security;
1+
using System;
2+
using System.Security;
23

34
namespace Elasticsearch.Net
45
{
56
/// <summary>
67
/// Credentials for Basic Authentication
78
/// </summary>
8-
public class BasicAuthenticationCredentials
9+
public class BasicAuthenticationCredentials : IDisposable
910
{
1011
public BasicAuthenticationCredentials()
1112
{
@@ -32,5 +33,7 @@ public BasicAuthenticationCredentials(string username, SecureString password)
3233
/// The username with which to authenticate
3334
/// </summary>
3435
public string Username { get; set; }
36+
37+
public void Dispose() => Password?.Dispose();
3538
}
3639
}

src/Elasticsearch.Net/Connection/SecureStrings.cs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,21 @@ public static class SecureStrings
1414
/// </summary>
1515
public static string CreateString(this SecureString secureString)
1616
{
17+
if (secureString == null || secureString.Length == 0)
18+
return string.Empty;
19+
1720
var num = IntPtr.Zero;
18-
if (secureString != null && secureString.Length != 0)
21+
22+
try
1923
{
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-
}
24+
num = Marshal.SecureStringToGlobalAllocUnicode(secureString);
25+
return Marshal.PtrToStringUni(num);
26+
}
27+
finally
28+
{
29+
if (num != IntPtr.Zero)
30+
Marshal.ZeroFreeGlobalAllocUnicode(num);
3031
}
31-
32-
return string.Empty;
3332
}
3433

3534
/// <summary>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using System.Threading.Tasks;
2+
using Elastic.Xunit.XunitPlumbing;
3+
using Elasticsearch.Net;
4+
using FluentAssertions;
5+
6+
namespace Tests.ClientConcepts.Connection
7+
{
8+
public class SecureStringsTests
9+
{
10+
[U]
11+
public void CreateStringMatchesOriginalString()
12+
{
13+
var password = "password";
14+
var secureString = password.CreateSecureString();
15+
var count = 100;
16+
var tasks = new Task<string>[count];
17+
18+
for (var i = 0; i < count; i++)
19+
tasks[i] = new Task<string>(() => secureString.CreateString());
20+
21+
for (var i = 0; i < count; i++)
22+
tasks[i].Start();
23+
24+
Task.WaitAll(tasks);
25+
26+
for (var i = 0; i < count; i++)
27+
tasks[i].Result.Should().Be(password);
28+
}
29+
}
30+
}

0 commit comments

Comments
 (0)