Fix TLS decryption #221
Merged
Fix TLS decryption #221
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
According to [MSDN](https://learn.microsoft.com/en-us/windows/win32/secauthn/decryptmessage--general), encrypt and decrypt message functions can be called from different threads simultaneously. During the dev testing, we confirmed that these methods are called from different threads. But our implementation has no synchronization and we can not guarantee that it doesn't break during the connection. To fix this, we wrapped SppiContext into a Mutex and implemented all needed methods and traits for this wrapper.
According to the [MSDN](https://learn.microsoft.com/en-us/windows/win32/secauthn/decryptmessage--schannel): "The encrypted message is decrypted in place, overwriting the original contents of its buffer." Our previous implementation had allocated new buffers for the decrypted data. It had caused a memory leak because the caller didn't expect new allocations. This problem is fixed by introducing a special `DecryptBuffer` type that uses only references to the provided buffers without any vectors and data cloning.
There can be a situation when `mstsc` can pass only some part of the TLS packet as an input buffer. Previously, we passed such buffers to the `rustls` and were waiting for the rest on the next decrypt function call. But after such n-step packet decryption, we may get a situation when the decrypted data buffer is larger than the encrypted one. It makes it impossible to properly return all decrypted data. Now, the sspi-rs behaves as the original `SChannel`: In a case when it gets the partial TLS message, it'll return `SECBUFFER_MISSING` with a length equal to the needed bytes amount alongside with `SEC_E_INCOMPLETE_MESSAGE` status code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
In this pull request, I have fixed the decryption error. This error used to look like this:
More information you can read in those four pull requests description:
I checked the connection in the following scenarios:
Docs & references:
closes #153
cc @awakecoding