Reset server MAC after TransformFinalBlock on .NET FW - #1830
Conversation
Call _serverMac.Initialize() after TransformFinalBlock when running on .NET Framework. This ensures the MAC is properly reset, addressing differences in cryptographic API behavior across different versions of mscorlib.dll. Close sshnet#1829
|
|
||
| #if NETFRAMEWORK | ||
| // Ensure MAC is reset on .NET Framework | ||
| _serverMac.Initialize(); |
There was a problem hiding this comment.
I'm just guessing without having looked at the old mscorlib source, but perhaps it ought to be after the get on .Hash i.e. after the FixedTimeEquals check?
There was a problem hiding this comment.
I reviewed the mscorlib source and found that .Hash is not cleared by .Initialize(). It gets cleared in Dispose().
I understand your concern and there's no harm to move .Initialize() after FixedTimeEquals check.
As a reference, the ComputeHash implementation calls .Initialize() at the very end, just before it returns.
The Initialize method name is kind of misleading.
public byte[] ComputeHash(byte[] buffer, int offset, int count)
{
if (buffer == null)
{
throw new ArgumentNullException("buffer");
}
if (offset < 0)
{
throw new ArgumentOutOfRangeException("offset", Environment.GetResourceString("ArgumentOutOfRange_NeedNonNegNum"));
}
if (count < 0 || count > buffer.Length)
{
throw new ArgumentException(Environment.GetResourceString("Argument_InvalidValue"));
}
if (buffer.Length - count < offset)
{
throw new ArgumentException(Environment.GetResourceString("Argument_InvalidOffLen"));
}
if (m_bDisposed)
{
throw new ObjectDisposedException(null);
}
HashCore(buffer, offset, count);
HashValue = HashFinal();
byte[] result = (byte[])HashValue.Clone();
Initialize();
return result;
}There was a problem hiding this comment.
there's no harm to move
.Initialize()after FixedTimeEquals check.
The only minor difference is that .Initialize() will no longer execute if the FixedTimeEquals check fails. This is not a significant concern because an SshConnectionException will be thrown in that scenario.
An alternative approach is to capture the hash into a local variable, invoke .Initialize(), and then perform the FixedTimeEquals check.
Please let me know which option you prefer:
- Move
.Initialize()after theFixedTimeEqualscheck - Capture the hash → call
.Initialize()→ run theFixedTimeEqualscheck - Keep the existing behavior in this PR: call
.Initialize()before theFixedTimeEqualscheck
Call
_serverMac.Initialize()afterTransformFinalBlockwhen running on .NET Framework. This ensures the MAC is properly reset, addressing differences in cryptographic API behavior across different versions of mscorlib.dll.Close #1829