From 44744c21bcacf443835cda4e14e3b645c03d60de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20A=2EP?= <53834183+Jossec101@users.noreply.github.com> Date: Tue, 11 Jul 2023 12:37:24 +0200 Subject: [PATCH] feat(LightningService.cs): add check for correct number of human signatures in multisig wallet test(LightningServiceTests.cs): add test for incorrect number of human signatures in multisig wallet The changes were made to ensure that the number of human signatures matches the number of signatures required for a multisig wallet. This is important for security reasons, as it prevents unauthorized transactions from being processed. The test was added to verify that the system correctly handles cases where the number of human signatures is incorrect. --- src/Services/LightningService.cs | 12 +- .../Services/LightningServiceTests.cs | 238 ++++++++++++++++++ 2 files changed, 245 insertions(+), 5 deletions(-) diff --git a/src/Services/LightningService.cs b/src/Services/LightningService.cs index 17aa15b6..972ecd2f 100644 --- a/src/Services/LightningService.cs +++ b/src/Services/LightningService.cs @@ -192,15 +192,17 @@ public async Task OpenChannel(ChannelOperationRequest channelOperationRequest) try { - var signedPsbtCount = channelOperationRequest.ChannelOperationRequestPsbts.Count( + var humanSignaturesCount = channelOperationRequest.ChannelOperationRequestPsbts.Count( x => channelOperationRequest.Wallet != null && !x.IsFinalisedPSBT && !x.IsInternalWalletPSBT && - (channelOperationRequest.Wallet.IsHotWallet || !x.IsTemplatePSBT)); - if (channelOperationRequest.Wallet != null && signedPsbtCount > channelOperationRequest.Wallet.MofN) + !x.IsTemplatePSBT); + + //If it is a hot wallet, we dont check the number of (human) signatures + if (channelOperationRequest.Wallet != null && !channelOperationRequest.Wallet.IsHotWallet && channelOperationRequest.Wallet != null && humanSignaturesCount != channelOperationRequest.Wallet.MofN -1) { - _logger.LogError("The number of signatures exceeds the value set for this wallet"); - throw new InvalidOperationException("The number of signatures exceeds the value set for this wallet"); + _logger.LogError("The number of human signatures does not match the number of signatures required for this wallet, expected {MofN} but got {HumanSignaturesCount}", channelOperationRequest.Wallet.MofN-1, humanSignaturesCount); + throw new InvalidOperationException("The number of human signatures does not match the number of signatures required for this wallet"); } if (!combinedPSBT.TryGetVirtualSize(out var estimatedVsize)) { diff --git a/test/FundsManager.Tests/Services/LightningServiceTests.cs b/test/FundsManager.Tests/Services/LightningServiceTests.cs index 8aff26ee..ac9d813f 100644 --- a/test/FundsManager.Tests/Services/LightningServiceTests.cs +++ b/test/FundsManager.Tests/Services/LightningServiceTests.cs @@ -1059,6 +1059,244 @@ public async Task OpenChannel_SuccessSingleSigBip39() //TODO Remove hack LightningService.CreateLightningClient = originalCreateLightningClient; } + + /// + /// This tests makes sure that if a multisig wallet is used, the number of signatures is correct. + /// This means that we need in in a m-of-n multisig, m-1 signatures so nodeguard is that last one to sign to avoid leaking signatures with SIGHASH_NONE + /// + [Fact] + public async Task OpenChannel_FailedIncorrectNumberOfHumanSigs() + { + // Arrange + Environment.SetEnvironmentVariable("NBXPLORER_URI", "http://10.0.0.2:38762"); + var dbContextFactory = new Mock>(); + + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(databaseName: "ChannelOpenDb") + .Options; + var context = new ApplicationDbContext(options); + dbContextFactory.Setup(x => x.CreateDbContextAsync(default)).ReturnsAsync(context); + var channelOperationRequestRepository = new Mock(); + var nodeRepository = new Mock(); + + var channelOpReqPsbts = new List(); + var userSignedPSBT = + "cHNidP8BAF4BAAAAAfxbrSOgX+b0TEE/+djT9eYQrMqkbB0oS5eACIYo69ilAQAAAAD/////AYSRNXcAAAAAIgAgknIr2R4V8Bi4hnqXM/qI2ZXEy9MNhs8bc7M8k6KHNCAAAAAATwEENYfPAy8RJCyAAAAB/DvuQjoBjOttImoGYyiO0Pte4PqdeQqzcNAw4Ecw5sgDgI4uHNSCvdBxlpQ8WoEz0WmvhgIra7A4F3FkTsB0RNcQH8zk3jAAAIABAACAAQAAgE8BBDWHzwNWrAP0gAAAAfkIrkpmsP+hqxS1WvDOSPKnAiXLkBCQLWkBr5C5Po+BAlGvFeBbuLfqwYlbP19H/+/s2DIaAu8iKY+J0KIDffBgEGDzoLMwAACAAQAAgAEAAIBPAQQ1h88DfblGjQAAAACA3rcaovFO7X83IvRJXhrQefWfwPOD5bJ72dtvXpkhIAOv6z6lwqcNxKoocpZKXi/xFyrzRmob/tA5tiZlSX/FRhDtAhDIMAAAgAEAAIAAAAAAAAEBKwCUNXcAAAAAIgAg0KnQhQLDgpnwn8miRsBXVMFC0ribpYvNSiY/lUGGzM4iAgLYVMVgz+bATgvrRDQbanlASVXtiUwPt9yCgkQfv2kssUcwRAIgQMEU4f0gB9/Sgiw79s3Ug0BO201upuwiKqoUv6/svesCIGmKmt82DHfnLJsbKD7e2y4xEbc/Z1L/kMMkf4zQXuZLAgEDBAIAAAABBWlSIQLYVMVgz+bATgvrRDQbanlASVXtiUwPt9yCgkQfv2kssSEDAmf/CxGXSG9xiPljcG/e5CXFnnukFn0pJ64Q9U2aNL8hA2/OK1mLPmSxkVJC5GJuM8/inCj45Y6pksEvbHlmsVWpU64iBgLYVMVgz+bATgvrRDQbanlASVXtiUwPt9yCgkQfv2kssRgfzOTeMAAAgAEAAIABAACAAAAAAAAAAAAiBgMCZ/8LEZdIb3GI+WNwb97kJcWee6QWfSknrhD1TZo0vxhg86CzMAAAgAEAAIABAACAAAAAAAAAAAAiBgNvzitZiz5ksZFSQuRibjPP4pwo+OWOqZLBL2x5ZrFVqRjtAhDIMAAAgAEAAIAAAAAAAAAAAAAAAAAAAA=="; + channelOpReqPsbts.Add(new ChannelOperationRequestPSBT() + { + PSBT = userSignedPSBT, + }); + + //Lets add a second signed "human" PSBT + + channelOpReqPsbts.Add(new ChannelOperationRequestPSBT() + { + PSBT = userSignedPSBT, + }); + + var destinationNode = new Node() + { + PubKey = "03485d8dcdd149c87553eeb80586eb2bece874d412e9f117304446ce189955d375", + ChannelAdminMacaroon = "def", + Endpoint = "10.0.0.2" + }; + + var wallet = CreateWallet.MultiSig(_internalWallet); + var operationRequest = new ChannelOperationRequest + { + RequestType = OperationRequestType.Open, + SourceNode = new Node() + { + PubKey = "03b48034270e522e4033afdbe43383d66d426638927b940d09a8a7a0de4d96e807", + ChannelAdminMacaroon = "abc", + Endpoint = "10.0.0.1" + }, + DestNode = destinationNode, + Wallet = wallet, + ChannelOperationRequestPsbts = channelOpReqPsbts, + }; + + channelOperationRequestRepository + .Setup(x => x.GetById(It.IsAny())) + .ReturnsAsync(operationRequest); + + var nodes = new List {destinationNode}; + + nodeRepository + .Setup(x => x.GetAllManagedByNodeGuard()) + .Returns(Task.FromResult(nodes)); + + var lightningClient = Interceptor.For() + .Setup(x => x.GetNodeInfoAsync( + Arg.Ignore(), + Arg.Ignore(), + null, + Arg.Ignore() + )) + .Returns(MockHelpers.CreateAsyncUnaryCall( + new NodeInfo() + { + Node = new LightningNode() + { + Addresses = + { + new NodeAddress() + { + Network = "tcp", + Addr = "10.0.0.2" + } + } + } + })); + + var originalCreateLightningClient = LightningService.CreateLightningClient; + LightningService.CreateLightningClient = (_) => lightningClient; + + lightningClient + .Setup(x => x.ConnectPeerAsync( + Arg.Ignore(), + Arg.Ignore(), + null, + Arg.Ignore() + )) + .Returns(MockHelpers.CreateAsyncUnaryCall(new ConnectPeerResponse())); + + var noneUpdate = new OpenStatusUpdate(); + var chanPendingUpdate = new OpenStatusUpdate + { + ChanPending = new PendingUpdate() + }; + var channelPoint = new ChannelPoint + { + FundingTxidBytes = + ByteString.CopyFromUtf8("e59fa8edcd772213239daef2834d9021d1aecc591d605b426ae32c4bec5fdd7d"), + OutputIndex = 1 + }; + var chanOpenUpdate = new OpenStatusUpdate + { + ChanOpen = new ChannelOpenUpdate() + { + ChannelPoint = channelPoint + } + }; + + var psbtFundUpdate = new OpenStatusUpdate + { + PsbtFund = new ReadyForPsbtFunding() + { + Psbt = ByteString.FromBase64(userSignedPSBT) + } + }; + lightningClient + .Setup(x => x.OpenChannel( + Arg.Ignore(), + Arg.Ignore(), + null, + Arg.Ignore() + )) + .Returns(MockHelpers.CreateAsyncServerStreamingCall( + new List() + { + noneUpdate, + chanPendingUpdate, + psbtFundUpdate, + chanOpenUpdate + })); + + channelOperationRequestRepository + .Setup(x => x.Update(It.IsAny())) + .Returns((true, "")); + + var userSignedPsbtParsed = PSBT.Parse(userSignedPSBT, Network.RegTest); + var utxoChanges = new UTXOChanges(); + var input = userSignedPsbtParsed.Inputs[0]; + var utxoList = new List() + { + new UTXO() + { + Outpoint = input.PrevOut, + Index = 0, + ScriptPubKey = input.WitnessUtxo.ScriptPubKey, + KeyPath = new KeyPath("0/0"), + }, + }; + + utxoChanges.Confirmed = new UTXOChange() {UTXOs = utxoList}; + + var channelOperationRequestPsbtRepository = new Mock(); + channelOperationRequestPsbtRepository + .Setup(x => x.AddAsync(It.IsAny())) + .ReturnsAsync((true, "")); + + lightningClient + .Setup(x => x.FundingStateStep( + Arg.Ignore(), + Arg.Ignore(), + null, + default)) + .Returns(new FundingStateStepResp()); + + lightningClient + .Setup(x => x.FundingStateStepAsync( + Arg.Ignore(), + Arg.Ignore(), + null, + default)) + .Returns(MockHelpers.CreateAsyncUnaryCall(new FundingStateStepResp())); + + // Mock channel repository + var channelRepository = new Mock(); + + channelRepository + .Setup(x => x.GetByChanId(It.IsAny())) + .ReturnsAsync(() => null); + + //Mock List channels async + var listChannelsResponse = new ListChannelsResponse + { + Channels = + { + new Lnrpc.Channel + { + Active = true, + RemotePubkey = "03b48034270e522e4033afdbe43383d66d426638927b940d09a8a7a0de4d96e807", + ChannelPoint = + $"{LightningHelper.DecodeTxId(channelPoint.FundingTxidBytes)}:{channelPoint.OutputIndex}", + ChanId = 124, + Capacity = 1000, + LocalBalance = 100, + RemoteBalance = 900 + } + } + }; + + lightningClient + .Setup(x => x.ListChannelsAsync( + Arg.Ignore(), + Arg.Ignore(), + null, + default)) + .Returns(MockHelpers.CreateAsyncUnaryCall(listChannelsResponse)); + + var lightningService = new LightningService(_logger, + channelOperationRequestRepository.Object, + nodeRepository.Object, + dbContextFactory.Object, + channelOperationRequestPsbtRepository.Object, + channelRepository.Object, + null, + GetNBXplorerServiceFullyMocked(utxoChanges).Object, + null); + + // Act + var act = async () => await lightningService.OpenChannel(operationRequest); + + // Assert + await act.Should().ThrowAsync(); + + //TODO Remove hack + LightningService.CreateLightningClient = originalCreateLightningClient; + } [Fact] public async Task OpenChannel_SuccessSingleSig()