Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disposal deadlock #153

Merged
merged 1 commit into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 43 additions & 41 deletions Core Modules/WalletConnectSharp.Crypto/Crypto.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
public class Crypto : ICrypto
{
private readonly string CRYPTO_CLIENT_SEED = $"client_ed25519_seed";

private const string MULTICODEC_ED25519_ENCODING = "base58btc";
private const string MULTICODEC_ED25519_BASE = "z";
private const string MULTICODEC_ED25519_HEADER = "K36";
Expand All @@ -48,7 +48,7 @@
private const int TYPE_LENGTH = 1;
private const int IV_LENGTH = 12;
private const int KEY_LENGTH = 32;

/// <summary>
/// The name of the crypto module
/// </summary>
Expand All @@ -71,19 +71,20 @@
return "walletconnectsharp";
}
}

/// <summary>
/// The current KeyChain this crypto module instance is using
/// </summary>
public IKeyChain KeyChain { get; private set; }

/// <summary>
/// The current storage module this crypto module instance is using
/// </summary>
public IKeyValueStorage Storage { get; private set; }

private bool _initialized;
private bool _newStorage;
protected bool Disposed;

/// <summary>
/// Create a new instance of the crypto module, with a given storage module.
Expand All @@ -97,7 +98,7 @@
this.KeyChain = new KeyChain(storage);
this.Storage = storage;
}

/// <summary>
/// Create a new instance of the crypto module, with a given keychain.
/// </summary>
Expand Down Expand Up @@ -128,7 +129,7 @@
{
if (_newStorage)
await this.Storage.Init();

await this.KeyChain.Init();
this._initialized = true;
}
Expand Down Expand Up @@ -159,7 +160,7 @@
var options = new KeyGenerationParameters(SecureRandom.GetInstance("SHA256PRNG"), 1);
X25519KeyPairGenerator generator = new X25519KeyPairGenerator();
generator.Init(options);

var keypair = generator.GenerateKeyPair();
var publicKeyData = keypair.Public as X25519PublicKeyParameters;
var privateKeyData = keypair.Private as X25519PrivateKeyParameters;
Expand Down Expand Up @@ -281,7 +282,7 @@

var typeRaw = Bases.Base10.Decode($"{@params.Type}");
var iv = @params.Iv;

byte[] rawIv;
if (iv == null)
{
Expand All @@ -308,7 +309,7 @@
{
byte[] temp = new byte[encoded.Length * 3];
int len = aead.ProcessBytes(encoded, 0, encoded.Length, temp, 0);

if (len > 0)
{
encryptedStream.Write(temp, 0, len);
Expand All @@ -327,7 +328,7 @@
{
if (senderPublicKey == null)
throw new ArgumentException("Missing sender public key for type1 envelope");

return Task.FromResult(Convert.ToBase64String(
typeRaw.Concat(senderPublicKey).Concat(rawIv).Concat(encrypted).ToArray()
));
Expand Down Expand Up @@ -380,10 +381,7 @@
var message = JsonConvert.SerializeObject(payload);
var results = await Encrypt(new EncryptParams()
{
Message = message,
Type = type,
SenderPublicKey = senderPublicKey,
SymKey = symKey
Message = message, Type = type, SenderPublicKey = senderPublicKey, SymKey = symKey
});

return results;
Expand All @@ -398,7 +396,8 @@
/// <param name="options">(optional) Decoding options</param>
/// <typeparam name="T">The type of the IJsonRpcPayload to convert the encoded Json to</typeparam>
/// <returns>The decoded, decrypted and deserialized object of type T from an async task</returns>
public async Task<T> Decode<T>(string topic, string encoded, DecodeOptions options = null) where T : IJsonRpcPayload
public async Task<T> Decode<T>(string topic, string encoded, DecodeOptions options = null)
where T : IJsonRpcPayload
{
this.IsInitialized();
var @params = ValidateDecoding(encoded, options);
Expand Down Expand Up @@ -430,7 +429,8 @@
var slice3 = slice2 + IV_LENGTH;
var senderPublicKey = new ArraySegment<byte>(bytes, slice1, KEY_LENGTH);
var iv = new ArraySegment<byte>(bytes, slice2, IV_LENGTH);
var @sealed = new ArraySegment<byte>(bytes, slice3, bytes.Length - (TYPE_LENGTH + KEY_LENGTH + IV_LENGTH));
var @sealed =
new ArraySegment<byte>(bytes, slice3, bytes.Length - (TYPE_LENGTH + KEY_LENGTH + IV_LENGTH));

return new EncodingParams()
{
Expand All @@ -446,12 +446,7 @@
var iv = new ArraySegment<byte>(bytes, slice1, IV_LENGTH);
var @sealed = new ArraySegment<byte>(bytes, slice2, bytes.Length - (IV_LENGTH + TYPE_LENGTH));

return new EncodingParams()
{
Type = typeRaw,
Sealed = @sealed.ToArray(),
Iv = iv.ToArray()
};
return new EncodingParams() { Type = typeRaw, Sealed = @sealed.ToArray(), Iv = iv.ToArray() };
}
}

Expand Down Expand Up @@ -493,12 +488,7 @@
signer.BlockUpdate(data, 0, data.Length);

var signature = signer.GenerateSignature();
return EncodeJwt(new IridiumJWTSigned()
{
Header = header,
Payload = payload,
Signature = signature
});
return EncodeJwt(new IridiumJWTSigned() { Header = header, Payload = payload, Signature = signature });
}

/// <summary>
Expand All @@ -516,8 +506,8 @@

private string EncodeJwt(IridiumJWTSigned data)
{
return string.Join(JWT_DELIMITER,
EncodeJson(data.Header),
return string.Join(JWT_DELIMITER,
EncodeJson(data.Header),
EncodeJson(data.Payload),
EncodeSig(data.Signature)
);
Expand Down Expand Up @@ -549,7 +539,7 @@
private Ed25519PrivateKeyParameters KeypairFromSeed(byte[] seed)
{
return new Ed25519PrivateKeyParameters(seed);

/*var options = new KeyCreationParameters()
{
ExportPolicy = KeyExportPolicies.AllowPlaintextExport
Expand Down Expand Up @@ -578,10 +568,8 @@
{
if (!this._initialized)
{
throw WalletConnectException.FromType(ErrorType.NOT_INITIALIZED, new Dictionary<string, object>()
{
{ "Name", Name }
});
throw WalletConnectException.FromType(ErrorType.NOT_INITIALIZED,
new Dictionary<string, object>() { { "Name", Name } });
}
}

Expand Down Expand Up @@ -616,7 +604,7 @@
{
var keyB = PublicKey.Import(KeyAgreementAlgorithm.X25519, publicKeyB.HexToByteArray(),
KeyBlobFormat.RawPublicKey);

var options = new SharedSecretCreationParameters
{
ExportPolicy = KeyExportPolicies.AllowPlaintextArchiving
Expand All @@ -632,7 +620,7 @@
generator.Init(new HkdfParameters(secretKey, Array.Empty<byte>(), Array.Empty<byte>()));

byte[] key = new byte[32];
generator.GenerateBytes(key, 0,32);
generator.GenerateBytes(key, 0, 32);

return key;
}
Expand All @@ -644,14 +632,14 @@
var iv = param.Iv;
var type = int.Parse(Bases.Base10.Encode(param.Type));
var isType1 = type == TYPE_1;

var aead = new ChaCha20Poly1305();
aead.Init(false, new ParametersWithIV(new KeyParameter(symKey.HexToByteArray()), iv));

using MemoryStream rawDecrypted = new MemoryStream();
byte[] temp = new byte[@sealed.Length];
int len = aead.ProcessBytes(@sealed, 0, @sealed.Length, temp, 0);

if (len > 0)
{
rawDecrypted.Write(temp, 0, len);
Expand All @@ -663,7 +651,7 @@
{
rawDecrypted.Write(temp, 0, len);
}

return Encoding.UTF8.GetString(rawDecrypted.ToArray());
}

Expand All @@ -674,7 +662,7 @@
{
seed = await this.KeyChain.Get(CRYPTO_CLIENT_SEED);
}
catch (Exception e)

Check warning on line 665 in Core Modules/WalletConnectSharp.Crypto/Crypto.cs

View workflow job for this annotation

GitHub Actions / build (unit-tests)

The variable 'e' is declared but never used

Check warning on line 665 in Core Modules/WalletConnectSharp.Crypto/Crypto.cs

View workflow job for this annotation

GitHub Actions / build (integration-tests)

The variable 'e' is declared but never used
{
byte[] seedRaw = new byte[32];
RandomNumberGenerator.Fill(seedRaw);
Expand All @@ -687,7 +675,21 @@

public void Dispose()
{
KeyChain?.Dispose();
Dispose(true);
GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
if (Disposed)
return;

if (disposing)
{
KeyChain?.Dispose();
}

Disposed = true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
private string _url;
private bool _registering;
private Guid _context;
protected bool Disposed;

/// <summary>
/// The Open timeout
Expand Down Expand Up @@ -98,7 +99,7 @@
{
if (!Validation.IsWsUrl(url))
throw new ArgumentException("Provided URL is not compatible with WebSocket connection: " + url);

_context = Guid.NewGuid();
this._url = url;
}
Expand Down Expand Up @@ -178,7 +179,7 @@
{
if (socket == null)
return;

socket.MessageReceived.Subscribe(OnPayload);
socket.DisconnectionHappened.Subscribe(OnDisconnect);

Expand All @@ -191,15 +192,15 @@
{
if (obj.Exception != null)
this.ErrorReceived?.Invoke(this, obj.Exception);

OnClose(obj);
}

private void OnClose(DisconnectionInfo obj)
{
if (this._socket == null)
return;

//_socket.Dispose();
this._socket = null;
this._registering = false;
Expand All @@ -221,7 +222,7 @@
}

if (string.IsNullOrWhiteSpace(json)) return;

//Console.WriteLine($"[{Name}] Got payload {json}");

this.PayloadReceived?.Invoke(this, json);
Expand All @@ -237,8 +238,9 @@
throw new IOException("Connection already closed");

await _socket.Stop(WebSocketCloseStatus.NormalClosure, "Close Invoked");

OnClose(new DisconnectionInfo(DisconnectionType.Exit, WebSocketCloseStatus.Empty, "Close Invoked", null, null));

OnClose(new DisconnectionInfo(DisconnectionType.Exit, WebSocketCloseStatus.Empty, "Close Invoked", null,
null));
}

/// <summary>
Expand Down Expand Up @@ -303,32 +305,37 @@
}
}

/// <summary>
/// Dispose this websocket connection. Will automatically Close this
/// websocket if still connected.
/// </summary>
public async void Dispose()

Check warning on line 308 in Core Modules/WalletConnectSharp.Network.Websocket/WebsocketConnection.cs

View workflow job for this annotation

GitHub Actions / build (unit-tests)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 308 in Core Modules/WalletConnectSharp.Network.Websocket/WebsocketConnection.cs

View workflow job for this annotation

GitHub Actions / build (unit-tests)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 308 in Core Modules/WalletConnectSharp.Network.Websocket/WebsocketConnection.cs

View workflow job for this annotation

GitHub Actions / build (integration-tests)

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
if (Connected)
Dispose(true);
GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
if (Disposed)
return;

if (disposing)
{
await Close();
_socket.Dispose();
}

Disposed = true;
}

private string addressNotFoundError = "getaddrinfo ENOTFOUND";
private string connectionRefusedError = "connect ECONNREFUSED";
private const string AddressNotFoundError = "getaddrinfo ENOTFOUND";
private const string ConnectionRefusedError = "connect ECONNREFUSED";

private void OnError<T>(IJsonRpcPayload ogPayload, Exception e)
{
var exception = e.Message.Contains(addressNotFoundError) || e.Message.Contains(connectionRefusedError)
? new IOException("Unavailable WS RPC url at " + _url) : e;
var exception = e.Message.Contains(AddressNotFoundError) || e.Message.Contains(ConnectionRefusedError)
? new IOException("Unavailable WS RPC url at " + _url)
: e;

var message = exception.Message;
var payload = new JsonRpcResponse<T>(ogPayload.Id, new Error()
{
Code = exception.HResult,
Data = null,
Message = message
}, default(T));
var payload = new JsonRpcResponse<T>(ogPayload.Id,
new Error() { Code = exception.HResult, Data = null, Message = message }, default(T));

//Trigger the payload event, converting the new JsonRpcResponse object to JSON string
this.PayloadReceived?.Invoke(this, JsonConvert.SerializeObject(payload));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
using System;
using System.Threading.Tasks;
using WalletConnectSharp.Common;
using WalletConnectSharp.Network.Models;

namespace WalletConnectSharp.Network
{
/// <summary>
/// The interface that represents a JSON RPC provider
/// </summary>
public interface IJsonRpcProvider : IBaseJsonRpcProvider
public interface IJsonRpcProvider : IBaseJsonRpcProvider, IModule
{
event EventHandler<JsonRpcPayload> PayloadReceived;

Expand All @@ -18,7 +19,7 @@ public interface IJsonRpcProvider : IBaseJsonRpcProvider
event EventHandler<Exception> ErrorReceived;

event EventHandler<string> RawMessageReceived;

/// <summary>
/// Connect this provider to the given URL
/// </summary>
Expand Down
Loading
Loading