CryptoFormatter

[HavenDV]

A wrapper for other formatter that encrypts/decrypts data

At the moment, my knowledge is enough only to implement the simplest XOR encryption, which makes little sense

[zbalkan]

I use some encryption with my software to encrypt license keys. I am working on using the same encryption methods to encrypt every message. If I manage to make it work stable, I'll create a PR.

[HavenDV]

I have merged your code and will adapt it a little according to my vision of the project. I need a little explanation about KeyPair.ValidatePublicKey:
The current implementation does not refer to class fields and can be a static method. Or you have an error, because the out variable and the publicKey field have the same name, and it is the out variable that is visible inside the method.

[zbalkan]

Thank you for the review. I will update my repo and work on that part.

[HavenDV]

Please wait a little. I have already included your code in the main branch, I will change it a little and you will just provide a new PR on top of the master branch.

[HavenDV]

I rewrote your code a bit, simplifying the end-use to:

await using var server = new PipeServer<MyMessage>(pipeName, formatter: new InfernoFormatter(new SystemTextJsonFormatter()));
server.EnableEncryption();

await using var client = new PipeClient<MyMessage>(pipeName, formatter: new InfernoFormatter(new SystemTextJsonFormatter()));
client.EnableEncryption();

Please also study the code https://github.com/HavenDV/H.Pipes/tree/master/src/libs/H.Formatters.Inferno for errors in encryption.

[HavenDV]

There is still a potential problem here:
If the client sends messages immediately after ConnectAsync, then the message may arrive unencrypted.
So far, I see a solution to this problem in changing the InfernoFormatter to IAsyncFormatter and waiting for the key via while (Key == null) await Task.Delay;

[HavenDV]

There is also a problem when the server has more than 1 connection, since the Formatter is common for all connections.

[zbalkan]

My take was if the sent message is not a public key, ignore it. If it's a public key, then use it to calculate shared key. Afterwards, go on encrypted. But it's better to handle it in the library, you are right.

In the Validation, I'll add a try catch and keep the length as a check step. I plan to ensure the sent message is a valid public key by using either dotnet core Cryptography library or Inferno. Since try-catch is costly, the precheck of length is still a valid step.

And I think, client can send its public key only after it gets the server public key, or vice versa.

[HavenDV]

I have changed the way for key exchange. Now the client and the server create a separate byte[] channel for this using the SingleConnection server and client.

[zbalkan]

There is also a problem when the server has more than 1 connection, since the Formatter is common for all connections.

You are absolutely right. An instance of format is needed for each connection. I only created one KeyPair for each, but a Formatter is also needed for each. A dictionary might not suffice, I believe, there might be another class with Client Id, KeyPair and Formatter. There may be more ways to improve the quality.

[HavenDV]

I have simplified the end-use to:

await using var server = new PipeServer<MyMessage>(pipeName, formatter: new SystemTextJsonFormatter());
server.EnableEncryption();

await using var client = new PipeClient<MyMessage>(pipeName, formatter: new SystemTextJsonFormatter());
client.EnableEncryption();

solving the problem with multiple connections at the same time - I just create an InfernoFormatter for each connection after I get the key.

[HavenDV]

I added PipeConnectionExtensions.WaitExchangeAsync. It applies to any connection and will wait for the InfernoFormatter to be active.

[HavenDV]

I am going to sleep. It will be great if you test this and provide feedback. Thank you again for the your work and the provided PRs

[zbalkan]

I'll test it this evening.

[zbalkan]

There's a Heisenbug. It happens when not debugging but when you search for it, it does not occur.

Currently there are two errors I could see but could not diagnose. First one is about serialization / deserialization. There's a byte array sent to a JSON converter, that I could not spot. The second is premature cancelation of a task.

[HavenDV]

The current challenge is to recreate the problems in the tests so that they can be fixed.

[zbalkan]

The first one is probably this:

H.Pipes/src/libs/H.Formatters.Inferno/PipeClientExtensions.cs

Line 31 in 8bf9084

var client = new SingleConnectionPipeClient<byte[]>(pipeName, args.Connection.ServerName, formatter: args.Connection.Formatter);

Or this:

H.Pipes/src/libs/H.Formatters.Inferno/PipeClientExtensions.cs

Line 37 in 8bf9084

var response = await client.WaitMessageAsync(cancellationToken: cancellationToken).ConfigureAwait(false);

It sends a byte array but uses Formatter. Byte array cannot be (de)serialized as json. The exception was something like "0x17 is not a valid JSON".

And the cancelation is caused by the same thing because here it has 10 seconds to communicate but could not succeed for reasons.

H.Pipes/src/libs/H.Formatters.Inferno/PipeClientExtensions.cs

Line 28 in 8bf9084

using var source = new CancellationTokenSource(TimeSpan.FromSeconds(10));

Since the other one did not succeed for non-alphanumeric characters in byte array that could not be (de)serialized, this line will wait and probably cause the timeout.

H.Pipes/src/libs/H.Formatters.Inferno/PipeConnectionExtensions.cs

Line 25 in 8bf9084

await Task.Delay(TimeSpan.FromMilliseconds(1), cancellationToken).ConfigureAwait(false);

[HavenDV]

The timeout is needed to know about problems, instead of the method just waiting forever.

[HavenDV]

I am using serialization byte[] to json in tests. This works correctly for Newtonsoft.Json/System.Json.

[HavenDV]

I can use BinaryFormatter, but wouldn't that be another problem?

[HavenDV]

WaitExchangeAsync will wait forever unless you pass a CancellationToken. I can add an extra timeout there if needed.

[zbalkan]

I am using serialization byte[] to json in tests. This works correctly for Newtonsoft.Json/System.Json.

It's probably the byte[] here has unreadable characters. I used a simple LINQ call chain that casts each byte as char and concatenate as a string. But Inferno has its own method for this, converting it to a Base16, Base32 or Base64 string. Probably, that's for the best and allow a safe JSON conversion.

[zbalkan]

WaitExchangeAsync will wait forever unless you pass a CancellationToken. I can add an extra timeout there if needed.

Since the attempt to send a public key fails, it will always hit the timeout. So yes, having a timeout here is the correct thing. Without it, it would be hard to understand the problem and also it would hang forever.

[zbalkan]

I have another question. Think of TCP handshake. Would it be better to have this key share like that?

1. Client sends the public key, waits until it gets a public key. Ignores any other message. Client shuts down if no response is received.
2. When server receives a public key, it sends its newly created public key for that pipe. If no valid public key received in a timely manner, disposes the pipe.
3. When both successfully shared, then they start communicating.

[HavenDV]

It's probably the byte[] here has unreadable characters. I used a simple LINQ call chain that casts each byte as char and concatenate as a string. But Inferno has its own method for this, converting it to a Base16, Base32 or Base64 string. Probably, that's for the best and allow a safe JSON conversion.

Can you grab the data that is throwing the exception?
I suspect that this is still a problem with the Formatter you are using - MessagePack

[HavenDV]

byte[] serialization should not depend on content in any way

[HavenDV]

I have another question. Think of TCP handshake. Would it be better to have this key share like that?

1. Client sends the public key, waits until it gets a public key. Ignores any other message. Client shuts down if no response is received.
2. When server receives a public key, it sends its newly created public key for that pipe. If no valid public key received in a timely manner, disposes the pipe.
3. When both successfully shared, then they start communicating.

Yes, I think we need to add Disconnect if an error occurs or timed out while retrieving the key. Both client-side and non-server-side.

[zbalkan]

It's probably the byte[] here has unreadable characters. I used a simple LINQ call chain that casts each byte as char and concatenate as a string. But Inferno has its own method for this, converting it to a Base16, Base32 or Base64 string. Probably, that's for the best and allow a safe JSON conversion.

Can you grab the data that is throwing the exception? I suspect that this is still a problem with the Formatter you are using - MessagePack

Well, I only used the sample project you created. I did not want to make it more complicated.

[HavenDV]

I cannot repeat the behavior you are talking about. Perhaps you have sent special characters or are you using not the most recent code from the master branch?

[zbalkan]

Last night, I rebased my repo with the - then- latest state of the repo. It happened twice. But when I tried to debug it, Iit did not reoccur. I'll rebase with the latest version and try again. But for now, it seems it's OK. If I manage to find it, I'll create a new issue with steps to reproduce. If it cannot be reproduced, then I'll assume it does not exist 👍

[zbalkan]

Hi,

I managed to find it. Here are the steps to reproduce:

1. Start an instance and type server.
2. Start an instance and type client.
3. Start another instance and type client.
4. Voila! Get an exception: Exception: System.Text.Json.JsonException: '...' is an invalid start of a value.

Enter mode('server' or 'client'):
client
Running in CLIENT mode. PipeName: named_pipe_test_server
Enter 'q' to exit
Client connecting...
Connected to server
Exception: System.Text.Json.JsonException: '0xEF' is an invalid start of a value. Path: $ | LineNumber: 0 | BytePositionInLine: 0.
 ---> System.Text.Json.JsonReaderException: '0xEF' is an invalid start of a value. LineNumber: 0 | BytePositionInLine: 0.
   at System.Text.Json.ThrowHelper.ThrowJsonReaderException(Utf8JsonReader& json, ExceptionResource resource, Byte nextByte, ReadOnlySpan`1 bytes)
   at System.Text.Json.Utf8JsonReader.ConsumeValue(Byte marker)
   at System.Text.Json.Utf8JsonReader.ReadFirstToken(Byte first)
   at System.Text.Json.Utf8JsonReader.ReadSingleSegment()
   at System.Text.Json.Utf8JsonReader.Read()
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, JsonReaderException ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
   at H.Formatters.SystemTextJsonFormatter.DeserializeInternal[T](Byte[] bytes) in D:\Repos\H.Pipes\src\libs\H.Formatters.System.Text.Json\SystemTextJsonFormatter.cs:line 27
   at H.Formatters.FormatterBase.Deserialize[T](Byte[] bytes) in D:\Repos\H.Pipes\src\libs\H.Formatters\FormatterBase.cs:line 39
   at H.Pipes.PipeConnection`1.<Start>b__37_0(CancellationToken cancellationToken) in D:\Repos\H.Pipes\src\libs\H.Pipes\PipeConnection.cs:line 124

[zbalkan]

I believe some unit tests are needed for this library but I cannot decide what is a unit.

[HavenDV]

[HavenDV]

In addition, as I understand it, the exception occurs after the keys are received and the encrypted connection is established.