Code Monkey home page Code Monkey logo

Comments (34)

zbalkan avatar zbalkan commented on May 26, 2024 1

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024 1

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024 1

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024 1

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

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024 1

I'll test it this evening.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024 1

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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;

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

The first one is probably this:

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

Or this:
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.

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.

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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.

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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.

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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?

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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 👍

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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

from h.pipes.

zbalkan avatar zbalkan commented on May 26, 2024

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

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

image

from h.pipes.

HavenDV avatar HavenDV commented on May 26, 2024

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

from h.pipes.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.