Skip to content

Commit 3d49d90

Browse files
ChrisMcKeeChrisMcKee
committed
feat: safestring fettling
fwk * SandboxedSecureString delegate (string inputKey) replaced with BCryptDelegate (ReadOnlySpan<char> inputKey) — same type as the core version * GetBCryptHashFromSecureString is now unsafe to avoid gc copy in `Marshal.PtrToStringUni(sourceStringPointer)`; swapped for `new ReadOnlySpan<char>(sourceStringPointer.ToPointer(), length)` * HashPassword helper changed to ReadOnlySpan<char> add Verify call. - core: encodes both sides to stackalloc byte[60] and calls SecureEquals(ReadOnlySpan<byte>, ReadOnlySpan<byte>) - fwk: calls SecureEquals(SafeUTF8.GetBytes(hash), SafeUTF8.GetBytes(computed))
1 parent 0995cb9 commit 3d49d90

4 files changed

Lines changed: 74 additions & 25 deletions

File tree

‎src/BCrypt.Net/SafeStringExtension.Fwk.cs‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
// */
1919

2020
#if PRE_CORE && SECURESTRING
21-
using System.Runtime.CompilerServices;
2221
using System.Runtime.InteropServices;
2322
using System.Security;
2423

@@ -72,17 +71,32 @@ public static string HashPassword(SecureString inputKey, int workFactor = Defaul
7271
return GetBCryptHashFromSecureString(inputKey, key => HashPassword(key, workFactor));
7372
}
7473

75-
private static string HashPassword(string inputKey, int workFactor = DefaultRounds) =>
76-
HashPassword(inputKey, GenerateSalt(workFactor));
77-
78-
private static string HashPassword(string inputKey, string salt)
74+
/// <summary>
75+
/// Verifies a password against a previously computed BCrypt hash.
76+
/// </summary>
77+
/// <param name="inputKey">The candidate password, provided as a <see cref="SecureString"/>.</param>
78+
/// <param name="hash">The stored BCrypt hash to verify against.</param>
79+
/// <returns><c>true</c> if the password matches the hash; otherwise <c>false</c>.</returns>
80+
/// <exception cref="ArgumentException">Thrown when <paramref name="hash"/> is null or empty.</exception>
81+
/// <exception cref="SaltParseException">Thrown when the stored hash cannot be parsed.</exception>
82+
public static bool Verify(SecureString inputKey, string hash)
7983
{
80-
return CreatePasswordHash(inputKey.AsSpan(), salt.AsSpan());
84+
if (string.IsNullOrEmpty(hash))
85+
throw new ArgumentException("Invalid hash", nameof(hash));
86+
87+
string computed = GetBCryptHashFromSecureString(inputKey, key => HashPassword(key, hash));
88+
return SecureEquals(SafeUTF8.GetBytes(hash), SafeUTF8.GetBytes(computed));
8189
}
8290

83-
private delegate string SandboxedSecureString(string inputKey);
91+
private static string HashPassword(ReadOnlySpan<char> inputKey, int workFactor = DefaultRounds) =>
92+
HashPassword(inputKey, GenerateSalt(workFactor));
93+
94+
private static string HashPassword(ReadOnlySpan<char> inputKey, string salt) =>
95+
CreatePasswordHash(inputKey, salt.AsSpan());
8496

85-
private static string GetBCryptHashFromSecureString(SecureString secureString, SandboxedSecureString func)
97+
private delegate string BCryptDelegate(ReadOnlySpan<char> inputKey);
98+
99+
private static unsafe string GetBCryptHashFromSecureString(SecureString secureString, BCryptDelegate func)
86100
{
87101
if (secureString == null)
88102
throw new ArgumentNullException(nameof(secureString));
@@ -93,7 +107,7 @@ private static string GetBCryptHashFromSecureString(SecureString secureString, S
93107
if (length == 0)
94108
throw new ArgumentException("SecureString cannot be empty", nameof(secureString));
95109

96-
if(!secureString.IsReadOnly())
110+
if (!secureString.IsReadOnly())
97111
secureString.MakeReadOnly();
98112

99113
IntPtr sourceStringPointer = IntPtr.Zero;
@@ -106,14 +120,16 @@ private static string GetBCryptHashFromSecureString(SecureString secureString, S
106120
if (sourceStringPointer == IntPtr.Zero)
107121
throw new InvalidOperationException("Failed to convert SecureString to BSTR");
108122

109-
// Convert BSTR pointer to a managed string
110-
// Note: We need to use Marshal.PtrToStringUni for wide character strings
111-
string inputKey = Marshal.PtrToStringUni(sourceStringPointer);
112-
113-
if (inputKey == null)
114-
throw new InvalidOperationException("Failed to convert BSTR to string");
123+
// Wrap the BSTR directly in a ReadOnlySpan<char> — avoiding a managed string touching the GC heap.
124+
//
125+
// LIFETIME INVARIANT: inputSpan must not outlive sourceStringPointer. The BSTR is zeroed
126+
// and freed in the `finally` block below. The compiler enforces this automatically because
127+
// ReadOnlySpan<char> is a ref struct: it cannot be stored in a field, boxed, or captured
128+
// by an async continuation. Do NOT change BCryptDelegate to an async delegate or convert
129+
// this method to async; breaks the invariant and will error at runtime.
130+
ReadOnlySpan<char> inputSpan = new ReadOnlySpan<char>(sourceStringPointer.ToPointer(), length);
115131

116-
return func(inputKey);
132+
return func(inputSpan);
117133
}
118134
finally
119135
{

‎src/BCrypt.Net/SafeStringExtension.cs‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,30 @@ public static string HashPassword(SecureString inputKey, int workFactor = Defaul
5252
return GetBCryptHashFromSecureString(inputKey, key => HashPassword(key, workFactor));
5353
}
5454

55+
/// <summary>
56+
/// Verifies a password against a previously computed BCrypt hash.
57+
/// </summary>
58+
/// <param name="inputKey">The candidate password, provided as a <see cref="SecureString"/>.</param>
59+
/// <param name="hash">The stored BCrypt hash to verify against.</param>
60+
/// <returns><c>true</c> if the password matches the hash; otherwise <c>false</c>.</returns>
61+
/// <exception cref="ArgumentException">Thrown when <paramref name="hash"/> is null or empty.</exception>
62+
/// <exception cref="SaltParseException">Thrown when the stored hash cannot be parsed.</exception>
63+
public static bool Verify(SecureString inputKey, string hash)
64+
{
65+
if (string.IsNullOrEmpty(hash))
66+
throw new ArgumentException("Invalid hash", nameof(hash));
67+
68+
string computed = GetBCryptHashFromSecureString(inputKey, key => HashPassword(key, hash.AsSpan()));
69+
70+
// bcrypt hashes are at most 60 ASCII characters; encode both sides to bytes for
71+
// constant-time comparison via the existing SecureEquals implementation.
72+
Span<byte> hashBytes = stackalloc byte[60];
73+
Span<byte> computedBytes = stackalloc byte[60];
74+
int hashLen = SafeUTF8.GetBytes(hash, hashBytes);
75+
int computedLen = SafeUTF8.GetBytes(computed, computedBytes);
76+
return SecureEquals(hashBytes[..hashLen], computedBytes[..computedLen]);
77+
}
78+
5579
private static string HashPassword(ReadOnlySpan<char> inputKey, int workFactor = DefaultRounds) =>
5680
HashPassword(inputKey, GenerateSalt(workFactor));
5781

@@ -62,7 +86,8 @@ private static string HashPassword(ReadOnlySpan<char> inputKey, ReadOnlySpan<cha
6286
return new string(outputBuffer[..outputBufferWritten]);
6387
}
6488

65-
private static void HashPassword(ReadOnlySpan<char> inputKey, ReadOnlySpan<char> salt, Span<char> outputBuffer, out int outputBufferWritten) => CreatePasswordHash(inputKey, salt, outputBuffer, out outputBufferWritten);
89+
private static void HashPassword(ReadOnlySpan<char> inputKey, ReadOnlySpan<char> salt, Span<char> outputBuffer, out int outputBufferWritten) =>
90+
CreatePasswordHash(inputKey, salt, outputBuffer, out outputBufferWritten);
6691

6792
private delegate string BCryptDelegate(ReadOnlySpan<char> inputKey);
6893

@@ -75,7 +100,7 @@ private static unsafe string GetBCryptHashFromSecureString(SecureString secureSt
75100
if (length == 0)
76101
throw new ArgumentException("SecureString cannot be empty", nameof(secureString));
77102

78-
if(!secureString.IsReadOnly())
103+
if (!secureString.IsReadOnly())
79104
secureString.MakeReadOnly();
80105

81106
IntPtr sourceStringPointer = IntPtr.Zero;
@@ -88,8 +113,14 @@ private static unsafe string GetBCryptHashFromSecureString(SecureString secureSt
88113
if (sourceStringPointer == IntPtr.Zero)
89114
throw new InvalidOperationException("Failed to convert SecureString to BSTR");
90115

91-
// Convert the BSTR pointer directly to ReadOnlySpan<char>
92-
// Note: This assumes the BSTR is null-terminated & we're working with the actual content
116+
// Wrap the BSTR directly in a ReadOnlySpan<char> — no managed string is created, so the
117+
// password material never touches the GC heap.
118+
//
119+
// LIFETIME INVARIANT: inputSpan must not outlive sourceStringPointer. The BSTR is zeroed
120+
// and freed in the finally block below. The compiler enforces this automatically because
121+
// ReadOnlySpan<char> is a ref struct: it cannot be stored in a field, boxed, or captured
122+
// by an async continuation. Do NOT change BCryptDelegate to an async delegate or convert
123+
// this method to async — doing so would break the invariant without a compile error.
93124
ReadOnlySpan<char> inputSpan = new ReadOnlySpan<char>(sourceStringPointer.ToPointer(), length);
94125

95126
return func(inputSpan);

‎tests/UnitTests/BCrypt.Net.UnitTests.csproj‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
<PropertyGroup>
33
<AssemblyName>BCrypt.Net.UnitTests</AssemblyName>
44
<RootNamespace>BCryptNet.UnitTests</RootNamespace>
5-
<TargetFramework>net10.0</TargetFramework>
5+
<TargetFramework Condition="'$(OS)' != 'Windows_NT'">net10.0</TargetFramework>
6+
<TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">net10.0;net48</TargetFrameworks>
7+
<DebugSymbols>true</DebugSymbols>
8+
<ImplicitUsings>true</ImplicitUsings>
69
<Configurations>Debug;Release</Configurations>
710
<LangVersion>default</LangVersion>
811

‎tests/UnitTests/BCryptTests.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ public void GithubIssue119_WoltLabForumPHPBcrypt()
134134
Assert.True(BCrypt.Verify(pass, hash));
135135
}
136136

137-
138-
139137
private static SecureString AsSecureString(string text)
140138
{
141139
var result = new SecureString();
@@ -146,7 +144,7 @@ private static SecureString AsSecureString(string text)
146144

147145
#if !NET48_OR_GREATER
148146
[Fact()]
149-
public void TestSecureHashPassword()
147+
public void TestSecureHashPasswordFwk()
150148
{
151149
Trace.Write("BCryptSafeString.HashPassword()[Secure]: ");
152150
var sw = Stopwatch.StartNew();
@@ -157,6 +155,8 @@ public void TestSecureHashPassword()
157155
var secureString = AsSecureString(pass);
158156
if(string.IsNullOrEmpty(pass)) continue;
159157
var hash = BCryptSafeString.HashPassword(secureString);
158+
var doesValidateFromSecureString = BCryptSafeString.VerifyPassword(secureString, hash);
159+
Assert.True(doesValidateFromSecureString);
160160
var doesValidate = BCrypt.Verify(pass, hash);
161161
Assert.True(doesValidate);
162162
Trace.Write(".");
@@ -167,7 +167,6 @@ public void TestSecureHashPassword()
167167
Trace.WriteLine("");
168168
}
169169

170-
171170
[Fact()]
172171
public void TestSecureStringHashPassword()
173172
{

0 commit comments

Comments
 (0)