From 617b6ce55808d1881d09738371bdf703949c9040 Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Mon, 28 Oct 2024 19:26:23 -0400 Subject: [PATCH 01/11] Switch to CBC --- src/OSDP.Net/SecureChannel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OSDP.Net/SecureChannel.cs b/src/OSDP.Net/SecureChannel.cs index 97cdecd2..10317cfc 100644 --- a/src/OSDP.Net/SecureChannel.cs +++ b/src/OSDP.Net/SecureChannel.cs @@ -219,7 +219,7 @@ private static Aes CreateKeyAlgorithm() throw new Exception("Unable to create key algorithm"); } - keyAlgorithm.Mode = CipherMode.ECB; + keyAlgorithm.Mode = CipherMode.CBC; keyAlgorithm.KeySize = 128; keyAlgorithm.BlockSize = 128; keyAlgorithm.Padding = PaddingMode.Zeros; From 9bbb6b7cf4d28f7120d90150df3abb8a71686bdf Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Mon, 28 Oct 2024 19:36:34 -0400 Subject: [PATCH 02/11] Fix shutdown and restore ECB for initializing connections --- src/Console/Program.cs | 9 ++++++++- src/OSDP.Net/SecureChannel.cs | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Console/Program.cs b/src/Console/Program.cs index a6dc093f..92ad53c5 100644 --- a/src/Console/Program.cs +++ b/src/Console/Program.cs @@ -163,7 +163,14 @@ private static async Task Main() RegisterEvents(); - Application.Run(); + try + { + Application.Run(); + } + catch + { + // ignored + } await _controlPanel.Shutdown(); } diff --git a/src/OSDP.Net/SecureChannel.cs b/src/OSDP.Net/SecureChannel.cs index 10317cfc..97cdecd2 100644 --- a/src/OSDP.Net/SecureChannel.cs +++ b/src/OSDP.Net/SecureChannel.cs @@ -219,7 +219,7 @@ private static Aes CreateKeyAlgorithm() throw new Exception("Unable to create key algorithm"); } - keyAlgorithm.Mode = CipherMode.CBC; + keyAlgorithm.Mode = CipherMode.ECB; keyAlgorithm.KeySize = 128; keyAlgorithm.BlockSize = 128; keyAlgorithm.Padding = PaddingMode.Zeros; From cb20e80a966c2c4f9cfd7cd91d9420aed58ff3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Star=C3=BD?= Date: Wed, 20 Nov 2024 11:49:26 +0100 Subject: [PATCH 03/11] GH183: implement osdp_FMT reply processing - added: Event Handler for receiving osdp_FMT message on CP - added: osdp_FMT message parser --- .../Model/ReplyData/FormattedCardDataTest.cs | 29 ++++ src/OSDP.Net/ControlPanel.cs | 44 +++++++ .../Model/ReplyData/FormattedCardData.cs | 124 ++++++++++++++++++ 3 files changed, 197 insertions(+) create mode 100644 src/OSDP.Net.Tests/Model/ReplyData/FormattedCardDataTest.cs create mode 100644 src/OSDP.Net/Model/ReplyData/FormattedCardData.cs diff --git a/src/OSDP.Net.Tests/Model/ReplyData/FormattedCardDataTest.cs b/src/OSDP.Net.Tests/Model/ReplyData/FormattedCardDataTest.cs new file mode 100644 index 00000000..ea23c72d --- /dev/null +++ b/src/OSDP.Net.Tests/Model/ReplyData/FormattedCardDataTest.cs @@ -0,0 +1,29 @@ +using System; +using NUnit.Framework; +using OSDP.Net.Model.ReplyData; + +namespace OSDP.Net.Tests.Model.ReplyData; + +public class FormattedCardDataTest +{ + [Test] + public void ParseData() + { + var data = new byte[] { 0x05, 0x00, 0x09, 0x74, 0x65, 0x73, 0x74, 0x69, 0x6E, 0x70, 0x75, 0x74 }; + + var formattedCardData = FormattedCardData.ParseData(data); + + Assert.That(formattedCardData.ReaderNumber, Is.EqualTo(5)); + Assert.That(formattedCardData.ReadDirection, Is.EqualTo(ReadDirection.Forward)); + Assert.That(formattedCardData.Lenght, Is.EqualTo(9)); + Assert.That(formattedCardData.Data, Is.EqualTo("testinput")); + } + + [Test] + public void BuildData() + { + var formattedCardData = new FormattedCardData(5, ReadDirection.Forward, "testinput"); + var buffer = formattedCardData.BuildData(); + Assert.That(BitConverter.ToString(buffer), Is.EqualTo("05-00-09-74-65-73-74-69-6E-70-75-74")); + } +} \ No newline at end of file diff --git a/src/OSDP.Net/ControlPanel.cs b/src/OSDP.Net/ControlPanel.cs index 03c3c8ad..fe50f8a9 100644 --- a/src/OSDP.Net/ControlPanel.cs +++ b/src/OSDP.Net/ControlPanel.cs @@ -1393,6 +1393,11 @@ private void OnReplyReceived(ReplyTracker reply) new RawCardDataReplyEventArgs(reply.ConnectionId, reply.ReplyMessage.Address, RawCardData.ParseData(reply.ReplyMessage.Payload))); break; + case ReplyType.FormattedReaderData: + FormattedCardDataReplyReceived?.Invoke(this, + new FormattedCardDataReplyEventArgs(reply.ConnectionId, reply.ReplyMessage.Address, + FormattedCardData.ParseData(reply.ReplyMessage.Payload))); + break; case ReplyType.ManufactureSpecific: ManufacturerSpecificReplyReceived?.Invoke(this, new ManufacturerSpecificReplyEventArgs(reply.ConnectionId, reply.ReplyMessage.Address, @@ -1463,6 +1468,11 @@ private void OnReplyReceived(ReplyTracker reply) /// public event EventHandler RawCardDataReplyReceived; + /// + /// Occurs when formatted card data reply is received. + /// + public event EventHandler FormattedCardDataReplyReceived; + /// /// Occurs when manufacturer specific reply is received. /// @@ -1743,6 +1753,40 @@ public RawCardDataReplyEventArgs(Guid connectionId, byte address, RawCardData ra public RawCardData RawCardData { get; } } + /// + /// The formatted card data reply has been received. + /// + public class FormattedCardDataReplyEventArgs : EventArgs + { + /// + /// Initializes a new instance of the class. + /// + /// Identify the connection for communicating to the device. + /// Address assigned to the device. + /// A formatted card data reply. + public FormattedCardDataReplyEventArgs(Guid connectionId, byte address, FormattedCardData formattedCardData) + { + ConnectionId = connectionId; + Address = address; + FormattedCardData = formattedCardData; + } + + /// + /// Identify the connection for communicating to the device. + /// + public Guid ConnectionId { get; } + + /// + /// Address assigned to the device. + /// + public byte Address { get; } + + /// + /// A formatted card data reply. + /// + public FormattedCardData FormattedCardData { get; } + } + /// /// The manufacture specific reply has been received. /// diff --git a/src/OSDP.Net/Model/ReplyData/FormattedCardData.cs b/src/OSDP.Net/Model/ReplyData/FormattedCardData.cs new file mode 100644 index 00000000..52afe043 --- /dev/null +++ b/src/OSDP.Net/Model/ReplyData/FormattedCardData.cs @@ -0,0 +1,124 @@ +using System; +using System.Linq; +using System.Text; + +using OSDP.Net.Messages; +using OSDP.Net.Messages.SecureChannel; + +namespace OSDP.Net.Model.ReplyData; + +/// +/// A formated card data reply. +/// +public class FormattedCardData : PayloadData +{ + /// + /// Creates a new instance of FormattedCardData. The parameters passed here are + /// defined in OSDP spec for osdp_FMT response + /// + /// Reader number + /// Read direction + /// Data + public FormattedCardData(byte readerNumber, ReadDirection direction, string data) + { + ReaderNumber = readerNumber; + ReadDirection = direction; + Data = data; + Lenght = (ushort)data.Length; + } + + /// + /// The reader number. + /// + public byte ReaderNumber { get; } + + /// + /// The direction for reading the formatted card data. + /// + public ReadDirection ReadDirection { get; } + + /// + /// The lenght of the data. + /// + public ushort Lenght { get; } + + /// + /// The formatted card data. + /// + public string Data { get; } + + /// + public override byte Code => (byte)ReplyType.FormattedReaderData; + + /// + public override ReadOnlySpan SecurityControlBlock() + { + return SecurityBlock.ReplyMessageWithDataSecurity; + } + + /// + /// Parses the data. + /// + /// The data. + /// FormattedCardData. + public static FormattedCardData ParseData(ReadOnlySpan data) + { + var dataArray = data.ToArray(); + var readerNumber = data[0]; + var reverse = (data[1] & 0x01) == 1; + var numberOfBytes = data[2]; + + if (reverse) + { + dataArray = dataArray.Skip(3).Take(numberOfBytes).Reverse().ToArray(); + } + else + { + dataArray = dataArray.Skip(3).Take(numberOfBytes).ToArray(); + } + + var cardData = Encoding.ASCII.GetString(dataArray); + + return new FormattedCardData(readerNumber, (ReadDirection)data[1], cardData); + } + + /// + public override string ToString(int indent) + { + var padding = new string(' ', indent); + var build = new StringBuilder(); + build.AppendLine($"{padding} Reader Number: {ReaderNumber}"); + build.AppendLine($"{padding}Read Direction: {ReadDirection}"); + build.AppendLine($"{padding} Data Lenght: {Lenght}"); + build.AppendLine($"{padding} Data: {Data}"); + return build.ToString(); + } + + /// + public override byte[] BuildData() + { + var lenght = 3 + Data.Length; + var buffer = new byte[lenght]; + buffer[0] = ReaderNumber; + buffer[1] = (byte)ReadDirection; + buffer[2] = (byte)Data.Length; + Encoding.ASCII.GetBytes(Data).CopyTo(buffer, 3); + + return buffer; + } +} + +/// +/// The direction for reading the formatted card data. +/// +public enum ReadDirection +{ + /// + /// Forward direction. + /// + Forward = 0x00, + /// + /// Reverse direction. + /// + Reverse = 0x01 +} From 156f996658d89dd97926d6f2556deb354ac78f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Star=C3=BD?= Date: Wed, 20 Nov 2024 12:53:33 +0100 Subject: [PATCH 04/11] fix: running test on UNIX machine - fixed line ends to be the same as the line ends of OS on which are tests running --- src/OSDP.Net.Tests/ControlPanelTest.cs | 14 ++++----- .../Model/ReplyData/DeviceCapabilitiesTest.cs | 30 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/OSDP.Net.Tests/ControlPanelTest.cs b/src/OSDP.Net.Tests/ControlPanelTest.cs index 4a241bbf..92e886ae 100644 --- a/src/OSDP.Net.Tests/ControlPanelTest.cs +++ b/src/OSDP.Net.Tests/ControlPanelTest.cs @@ -24,7 +24,7 @@ public async Task DeviceGoesOnlineTest() { // Arrange var mockConnection = new MockConnection(); - + var panel = new ControlPanel(NullLoggerFactory.Instance); Guid id = panel.StartConnection(mockConnection.Object); panel.AddDevice(id, 0, true, false); @@ -40,7 +40,7 @@ public async Task ShutdownTest() { // Arrange var mockConnection = new MockConnection(); - + var panel = new ControlPanel(NullLoggerFactory.Instance); Guid id = panel.StartConnection(mockConnection.Object); panel.AddDevice(id, 0, true, false); @@ -193,11 +193,11 @@ public async Task ReturnsValidReportTest() var response = await panel.IdReport(id, 0); Assert.That(response.ToString(), Is.EqualTo( - " Vendor Code: 5C-26-23\r\n" + - " Model Number: 25\r\n" + - " Version: 2\r\n" + - " Serial Number: 00-00-E9-2A\r\n" + - "Firmware Version: 3.0.0\r\n" + $" Vendor Code: 5C-26-23{Environment.NewLine}" + + $" Model Number: 25{Environment.NewLine}" + + $" Version: 2{Environment.NewLine}" + + $" Serial Number: 00-00-E9-2A{Environment.NewLine}" + + $"Firmware Version: 3.0.0{Environment.NewLine}" )); } diff --git a/src/OSDP.Net.Tests/Model/ReplyData/DeviceCapabilitiesTest.cs b/src/OSDP.Net.Tests/Model/ReplyData/DeviceCapabilitiesTest.cs index d41c6c3f..6436641b 100644 --- a/src/OSDP.Net.Tests/Model/ReplyData/DeviceCapabilitiesTest.cs +++ b/src/OSDP.Net.Tests/Model/ReplyData/DeviceCapabilitiesTest.cs @@ -26,8 +26,8 @@ public void ThrowsWhenDataWrongLength() [Test] public void ParsesOutFunctionCodes() { - #pragma warning disable CS0618 // Type or member is obsolete - var expectedFuncCodes = new [] +#pragma warning disable CS0618 // Type or member is obsolete + var expectedFuncCodes = new[] { CapabilityFunction.CardDataFormat, CapabilityFunction.ReaderLEDControl, @@ -38,10 +38,10 @@ public void ParsesOutFunctionCodes() CapabilityFunction.CommunicationSecurity, CapabilityFunction.ReceiveBufferSize, CapabilityFunction.Biometrics, - CapabilityFunction.SecurePINEntry, + CapabilityFunction.SecurePINEntry, CapabilityFunction.OSDPVersion }; - #pragma warning restore CS0618 // Type or member is obsolete +#pragma warning restore CS0618 // Type or member is obsolete var actual = DeviceCapabilities.ParseData(_rawCapsFromDennisBrivoKeypad); @@ -64,17 +64,17 @@ public void ToStringTest() { var actual = DeviceCapabilities.ParseData(_rawCapsFromDennisBrivoKeypad.AsSpan().Slice(18, 9)); var expectedText = - " Function: Communication Security\r\n" + - "Supports AES-128: True\r\n" + - "Uses Default Key: True\r\n" + - "\r\n" + - " Function: Receive Buffer Size\r\n" + - " Size: 450\r\n" + - "\r\n" + - " Function: Biometrics\r\n" + - "Compliance: 0\r\n" + - " Number Of: 0\r\n" + - "\r\n"; + $" Function: Communication Security{Environment.NewLine}" + + $"Supports AES-128: True{Environment.NewLine}" + + $"Uses Default Key: True{Environment.NewLine}" + + Environment.NewLine + + $" Function: Receive Buffer Size{Environment.NewLine}" + + $" Size: 450{Environment.NewLine}" + + Environment.NewLine + + $" Function: Biometrics{Environment.NewLine}" + + $"Compliance: 0{Environment.NewLine}" + + $" Number Of: 0{Environment.NewLine}" + + Environment.NewLine; Assert.That(actual.ToString(), Is.EqualTo(expectedText).NoClip); } From c66aa09abaf5173c048db25a0c055ac0b53e607d Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Mon, 25 Nov 2024 08:14:53 -0500 Subject: [PATCH 05/11] Better handle errors when parsing OSDPCAP files --- src/Console/Program.cs | 13 ++++- .../IntegrationTests/PeripheryDeviceTest.cs | 4 +- src/OSDP.Net/Messages/IncomingMessage.cs | 2 + src/OSDP.Net/Messages/Message.cs | 2 +- src/OSDP.Net/Messages/OutgoingMessage.cs | 3 +- src/OSDP.Net/Tracing/MessageSpy.cs | 57 ++++++++++--------- 6 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/Console/Program.cs b/src/Console/Program.cs index 92ad53c5..dc1161c0 100644 --- a/src/Console/Program.cs +++ b/src/Console/Program.cs @@ -509,8 +509,17 @@ void ParseButtonClicked() return; } - var builder = BuildTextFromEntries(PacketDecoding.OSDPCapParser(json, key).Where(entry => - FilterAddress(entry, address) && IgnorePollsAndAcks(entry))); + StringBuilder builder; + try + { + builder = BuildTextFromEntries(PacketDecoding.OSDPCapParser(json, key).Where(entry => + FilterAddress(entry, address) && IgnorePollsAndAcks(entry))); + } + catch (Exception exception) + { + MessageBox.ErrorQuery(40, 10, "Error", $"Unable to parse. {exception.Message}", "OK"); + return; + } var saveDialog = new SaveDialog("Save Parsed File", "Successfully completed parsing of file, select location to save file.", new List { ".txt" }); diff --git a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs index b6d687d5..9c9bb1b9 100644 --- a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs +++ b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs @@ -52,11 +52,11 @@ public class PeripheryDeviceTest : IntegrationTestFixtureBase //// PD doesn't require Security; ACU doesn't use secure channel; two sides use different keys ==> OK new (IntegrationConsts.NonDefaultSCBK, IntegrationConsts.DefaultSCBK, false, false, true), - //// PD doesn't requires Security; ACU opens secure channel; two sides use different keys ==> NO + //// PD doesn't require Security; ACU opens secure channel; two sides use different keys ==> NO new (IntegrationConsts.NonDefaultSCBK, IntegrationConsts.DefaultSCBK, false, true, false), new (IntegrationConsts.DefaultSCBK, IntegrationConsts.NonDefaultSCBK, false, true, false), - //// PD doesn't requires Security; ACU opens secure channel; both sides use same key ==> OK + //// PD doesn't require Security; ACU opens secure channel; both sides use same key ==> OK new (IntegrationConsts.NonDefaultSCBK, IntegrationConsts.NonDefaultSCBK, false, true, true), new (IntegrationConsts.DefaultSCBK, IntegrationConsts.DefaultSCBK, false, true, true), ]; diff --git a/src/OSDP.Net/Messages/IncomingMessage.cs b/src/OSDP.Net/Messages/IncomingMessage.cs index cc3eb4ad..f6d4beed 100644 --- a/src/OSDP.Net/Messages/IncomingMessage.cs +++ b/src/OSDP.Net/Messages/IncomingMessage.cs @@ -54,6 +54,8 @@ internal IncomingMessage(ReadOnlySpan data, IMessageSecureChannel channel) while (lastByteIdx > 0 && paddedPayload[--lastByteIdx] != FirstPaddingByte) { } + + if (lastByteIdx == 0) throw new Exception("The encrypted payload is missing a padding byte"); Payload = paddedPayload.AsSpan().Slice(0, lastByteIdx).ToArray(); } diff --git a/src/OSDP.Net/Messages/Message.cs b/src/OSDP.Net/Messages/Message.cs index f32bc2dd..958bb910 100644 --- a/src/OSDP.Net/Messages/Message.cs +++ b/src/OSDP.Net/Messages/Message.cs @@ -208,7 +208,7 @@ internal static ushort CalculateMaximumMessageSize(ushort dataSize, bool isEncry internal static byte[] PadTheData(ReadOnlySpan data, byte cryptoLength, byte paddingStart) { - int paddingLength = data.Length + 16 - data.Length % 16; + int paddingLength = data.Length + cryptoLength - data.Length % cryptoLength; Span buffer = stackalloc byte[paddingLength]; buffer.Clear(); diff --git a/src/OSDP.Net/Messages/OutgoingMessage.cs b/src/OSDP.Net/Messages/OutgoingMessage.cs index ea1c5366..6c83d1a2 100644 --- a/src/OSDP.Net/Messages/OutgoingMessage.cs +++ b/src/OSDP.Net/Messages/OutgoingMessage.cs @@ -24,7 +24,8 @@ internal OutgoingMessage(byte address, Control controlBlock, PayloadData data) internal byte[] BuildMessage(IMessageSecureChannel secureChannel) { var payload = PayloadData.BuildData(); - var securityEstablished = secureChannel != null && secureChannel.IsSecurityEstablished; + + var securityEstablished = secureChannel is { IsSecurityEstablished: true }; if (securityEstablished && payload.Length > 0) { diff --git a/src/OSDP.Net/Tracing/MessageSpy.cs b/src/OSDP.Net/Tracing/MessageSpy.cs index e07e0ffd..80116142 100644 --- a/src/OSDP.Net/Tracing/MessageSpy.cs +++ b/src/OSDP.Net/Tracing/MessageSpy.cs @@ -6,16 +6,16 @@ namespace OSDP.Net.Tracing; internal class MessageSpy { - private readonly SecurityContext _context; - private readonly MessageSecureChannel _commandSpyChannel; - private readonly MessageSecureChannel _replySpyChannel; + private readonly SecurityContext _context; + private readonly MessageSecureChannel _commandSpyChannel; + private readonly MessageSecureChannel _replySpyChannel; - public MessageSpy(byte[] securityKey = null) - { - _context = new SecurityContext(securityKey); - _commandSpyChannel = new PdMessageSecureChannelBase(_context); - _replySpyChannel = new ACUMessageSecureChannel(_context); - } + public MessageSpy(byte[] securityKey = null) + { + _context = new SecurityContext(securityKey); + _commandSpyChannel = new PdMessageSecureChannelBase(_context); + _replySpyChannel = new ACUMessageSecureChannel(_context); + } public byte PeekAddressByte(ReadOnlySpan data) { @@ -45,25 +45,28 @@ public IncomingMessage ParseReply(byte[] data) }; } - private IncomingMessage HandleSessionChallenge(IncomingMessage command) - { - byte[] rndA = command.Payload; - var crypto = _context.CreateCypher(true); - _context.Enc = SecurityContext.GenerateKey(crypto, new byte[] { 0x01, 0x82, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); - _context.SMac1 = SecurityContext.GenerateKey(crypto, new byte[] { 0x01, 0x01, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); - _context.SMac2 = SecurityContext.GenerateKey(crypto, new byte[] { 0x01, 0x02, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); - return command; - } + private IncomingMessage HandleSessionChallenge(IncomingMessage command) + { + byte[] rndA = command.Payload; + var crypto = _context.CreateCypher(true); + _context.Enc = SecurityContext.GenerateKey(crypto, + new byte[] { 0x01, 0x82, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); + _context.SMac1 = SecurityContext.GenerateKey(crypto, + new byte[] { 0x01, 0x01, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); + _context.SMac2 = SecurityContext.GenerateKey(crypto, + new byte[] { 0x01, 0x02, rndA[0], rndA[1], rndA[2], rndA[3], rndA[4], rndA[5] }); + return command; + } - private IncomingMessage HandleSCrypt(IncomingMessage command) - { - var serverCryptogram = command.Payload; - using var crypto = _context.CreateCypher(true, _context.SMac1); - var intermediate = SecurityContext.GenerateKey(crypto, serverCryptogram); - crypto.Key = _context.SMac2; - _context.RMac = SecurityContext.GenerateKey(crypto, intermediate); - return command; - } + private IncomingMessage HandleSCrypt(IncomingMessage command) + { + var serverCryptogram = command.Payload; + using var crypto = _context.CreateCypher(true, _context.SMac1); + var intermediate = SecurityContext.GenerateKey(crypto, serverCryptogram); + crypto.Key = _context.SMac2; + _context.RMac = SecurityContext.GenerateKey(crypto, intermediate); + return command; + } private IncomingMessage HandleInitialRMac(IncomingMessage reply) { From b0a749c5a989b4c95350557f484b7e3778da824f Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Mon, 25 Nov 2024 08:19:00 -0500 Subject: [PATCH 06/11] Add obsolete notice to osdp_FMT reply --- src/OSDP.Net/ControlPanel.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OSDP.Net/ControlPanel.cs b/src/OSDP.Net/ControlPanel.cs index fe50f8a9..f98e1bb7 100644 --- a/src/OSDP.Net/ControlPanel.cs +++ b/src/OSDP.Net/ControlPanel.cs @@ -1471,6 +1471,7 @@ private void OnReplyReceived(ReplyTracker reply) /// /// Occurs when formatted card data reply is received. /// + [Obsolete("Use Raw Card Data for incoming card reads.")] public event EventHandler FormattedCardDataReplyReceived; /// From cc60ed40c90dd87fcf4ce21db59ab27a9684c33e Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Thu, 9 Jan 2025 21:41:25 -0500 Subject: [PATCH 07/11] #188 Prevent concurrent enumeration issues with _configurationDevices --- src/OSDP.Net/Bus.cs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/OSDP.Net/Bus.cs b/src/OSDP.Net/Bus.cs index ee69258a..a0dd5383 100644 --- a/src/OSDP.Net/Bus.cs +++ b/src/OSDP.Net/Bus.cs @@ -71,7 +71,7 @@ public Bus(IOsdpConnection connection, BlockingCollection replies, /// public IOsdpConnection Connection { get; private set; } - public IEnumerable ConfigureDeviceAddresses => _configuredDevices.Select(device => device.Address); + public IEnumerable ConfigureDeviceAddresses => _configuredDevices.ToArray().Select(device => device.Address); public void Dispose() { @@ -108,7 +108,7 @@ public async Task Close() /// The data for the command public void SendCommand(byte address, CommandData command) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); foundDevice.SendCommand(command); _commandAvailableEvent.Set(); } @@ -124,7 +124,7 @@ public void AddDevice(byte address, bool useCrc, bool useSecureChannel, byte[] s { lock (_configuredDevicesLock) { - var foundDevice = _configuredDevices.FirstOrDefault(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().FirstOrDefault(device => device.Address == address); if (foundDevice != null) { @@ -145,7 +145,7 @@ public void RemoveDevice(byte address) { lock (_configuredDevicesLock) { - var foundDevice = _configuredDevices.FirstOrDefault(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().FirstOrDefault(device => device.Address == address); if (foundDevice == null) return; _configuredDevices.Remove(foundDevice); @@ -159,7 +159,7 @@ public void RemoveDevice(byte address) /// True if the device is online public bool IsOnline(byte address) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); return foundDevice.IsConnected; } @@ -225,17 +225,19 @@ private async Task PollingLoop(CancellationToken cancellationToken) if (IsPolling) { + var configuredDevices = _configuredDevices.ToArray(); + // Allow for immediate processing of commands in queue or incoming multipart messages while (_pollInterval - (DateTime.UtcNow - lastMessageSentTime) > TimeSpan.Zero && - !_configuredDevices.Any(device1 => device1.HasQueuedCommand) && - !_configuredDevices.Any(device2 => device2.IsReceivingMultipartMessage)) + !configuredDevices.Any(device1 => device1.HasQueuedCommand) && + !configuredDevices.Any(device2 => device2.IsReceivingMultipartMessage)) { delayTime.WaitOne(TimeSpan.FromMilliseconds(10)); } lastMessageSentTime = DateTime.UtcNow; - if (!_configuredDevices.Any()) + if (!configuredDevices.Any()) { continue; } @@ -453,28 +455,28 @@ errorCode is ErrorCode.DoesNotSupportSecurityBlock or ErrorCode.CommunicationSec public void ResetDevice(int address) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); ResetDevice(foundDevice); } public void SetSendingMultipartMessage(byte address, bool isSendingMultipartMessage) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); foundDevice.IsSendingMultipartMessage = isSendingMultipartMessage; } public void SetReceivingMultipartMessage(byte address, bool isReceivingMultipartMessage) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); foundDevice.IsReceivingMultipartMessage = isReceivingMultipartMessage; } public void SetSendingMultiMessageNoSecureChannel(byte address, bool isSendingMultiMessageNoSecureChannel) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); foundDevice.IsSendingMultiMessageNoSecureChannel = isSendingMultiMessageNoSecureChannel; foundDevice.MessageControl.IsSendingMultiMessageNoSecureChannel = isSendingMultiMessageNoSecureChannel; @@ -486,7 +488,7 @@ public void SetSendingMultiMessageNoSecureChannel(byte address, bool isSendingMu public void SetRequestDelay(byte address, DateTime requestDelay) { - var foundDevice = _configuredDevices.First(device => device.Address == address); + var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); foundDevice.RequestDelay = requestDelay; } From 3b2889a38c53bf51ada137a24017031a625d33b3 Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Fri, 10 Jan 2025 07:53:08 -0500 Subject: [PATCH 08/11] #188 Switch to ImmutableSortedSet to track devices. This will provide performant usage of the set if updated by a separate thread. --- src/OSDP.Net/Bus.cs | 72 ++++++++++++++++++------------------ src/OSDP.Net/OSDP.Net.csproj | 1 + 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/OSDP.Net/Bus.cs b/src/OSDP.Net/Bus.cs index a0dd5383..8df0da4d 100644 --- a/src/OSDP.Net/Bus.cs +++ b/src/OSDP.Net/Bus.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Linq; using System.Threading; @@ -30,8 +31,6 @@ internal class Bus : IDisposable public static readonly TimeSpan DefaultPollInterval = TimeSpan.FromMilliseconds(200); - private readonly SortedSet _configuredDevices = new (); - private readonly object _configuredDevicesLock = new (); private readonly Dictionary _lastOnlineConnectionStatus = new (); private readonly Dictionary _lastSecureConnectionStatus = new (); @@ -44,6 +43,7 @@ internal class Bus : IDisposable private readonly IDeviceProxyFactory _deviceProxyFactory; private CancellationTokenSource _cancellationTokenSource; + private ImmutableSortedSet _configuredDevices = ImmutableSortedSet.Empty; public Bus(IOsdpConnection connection, BlockingCollection replies, TimeSpan pollInterval, Action tracer, IDeviceProxyFactory deviceProxyFactory, @@ -71,7 +71,7 @@ public Bus(IOsdpConnection connection, BlockingCollection replies, /// public IOsdpConnection Connection { get; private set; } - public IEnumerable ConfigureDeviceAddresses => _configuredDevices.ToArray().Select(device => device.Address); + public IEnumerable ConfigureDeviceAddresses => _configuredDevices.Select(device => device.Address); public void Dispose() { @@ -108,7 +108,7 @@ public async Task Close() /// The data for the command public void SendCommand(byte address, CommandData command) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); foundDevice.SendCommand(command); _commandAvailableEvent.Set(); } @@ -122,19 +122,20 @@ public void SendCommand(byte address, CommandData command) /// Set the secure channel key, default is used if not specified public void AddDevice(byte address, bool useCrc, bool useSecureChannel, byte[] secureChannelKey = null) { - lock (_configuredDevicesLock) + var configuredDevices = _configuredDevices.ToImmutableSortedSet(); + + var foundDevice = configuredDevices.FirstOrDefault(device => device.Address == address); + + if (foundDevice != null) { - var foundDevice = _configuredDevices.ToArray().FirstOrDefault(device => device.Address == address); - - if (foundDevice != null) - { - _configuredDevices.Remove(foundDevice); - } + configuredDevices = configuredDevices.Remove(foundDevice); + } - var addedDevice = _deviceProxyFactory.Create(address, useCrc, useSecureChannel, secureChannelKey); + var addedDevice = _deviceProxyFactory.Create(address, useCrc, useSecureChannel, secureChannelKey); - _configuredDevices.Add(addedDevice); - } + configuredDevices = configuredDevices.Add(addedDevice); + + _configuredDevices = configuredDevices; } /// @@ -143,13 +144,14 @@ public void AddDevice(byte address, bool useCrc, bool useSecureChannel, byte[] s /// Address of the device public void RemoveDevice(byte address) { - lock (_configuredDevicesLock) - { - var foundDevice = _configuredDevices.ToArray().FirstOrDefault(device => device.Address == address); - if (foundDevice == null) return; - - _configuredDevices.Remove(foundDevice); - } + var configuredDevices = _configuredDevices.ToImmutableSortedSet(); + + var foundDevice = configuredDevices.FirstOrDefault(device => device.Address == address); + if (foundDevice == null) return; + + configuredDevices = configuredDevices.Remove(foundDevice); + + _configuredDevices = configuredDevices; } /// @@ -159,7 +161,7 @@ public void RemoveDevice(byte address) /// True if the device is online public bool IsOnline(byte address) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); return foundDevice.IsConnected; } @@ -192,7 +194,7 @@ public void StartPolling() } /// - /// Poll the the devices on the bus + /// Poll the devices on the bus /// /// private async Task PollingLoop(CancellationToken cancellationToken) @@ -211,7 +213,7 @@ private async Task PollingLoop(CancellationToken cancellationToken) catch (Exception exception) { _logger?.LogError(exception, $"[{Connection}] Error while opening connection"); - foreach (var device in _configuredDevices.ToArray()) + foreach (var device in _configuredDevices) { ResetDevice(device); UpdateConnectionStatus(device); @@ -225,19 +227,17 @@ private async Task PollingLoop(CancellationToken cancellationToken) if (IsPolling) { - var configuredDevices = _configuredDevices.ToArray(); - // Allow for immediate processing of commands in queue or incoming multipart messages while (_pollInterval - (DateTime.UtcNow - lastMessageSentTime) > TimeSpan.Zero && - !configuredDevices.Any(device1 => device1.HasQueuedCommand) && - !configuredDevices.Any(device2 => device2.IsReceivingMultipartMessage)) + !_configuredDevices.Any(device1 => device1.HasQueuedCommand) && + !_configuredDevices.Any(device2 => device2.IsReceivingMultipartMessage)) { delayTime.WaitOne(TimeSpan.FromMilliseconds(10)); } lastMessageSentTime = DateTime.UtcNow; - if (!configuredDevices.Any()) + if (!_configuredDevices.Any()) { continue; } @@ -248,7 +248,7 @@ private async Task PollingLoop(CancellationToken cancellationToken) _commandAvailableEvent.WaitOne(TimeSpan.FromMilliseconds(10)); } - foreach (var device in _configuredDevices.ToArray()) + foreach (var device in _configuredDevices) { // Right now it always sends sequence 0 if (!IsPolling) @@ -293,7 +293,7 @@ private async Task PollingLoop(CancellationToken cancellationToken) // Prevent plain text message replies when secure channel has been established // The busy and Nak reply types are a special case which is allowed to be sent as insecure message on a secure channel - // Workaround for KeySet command sending back an clear text Ack + // Workaround for KeySet command sending back a clear text Ack if (reply.ReplyMessage.Type != (byte)ReplyType.Busy && reply.ReplyMessage.Type != (byte)ReplyType.Nak && device.UseSecureChannel && device.IsSecurityEstablished && !reply.ReplyMessage.IsSecureMessage && commandMessage.Code != (byte)CommandType.KeySet) { @@ -455,28 +455,28 @@ errorCode is ErrorCode.DoesNotSupportSecurityBlock or ErrorCode.CommunicationSec public void ResetDevice(int address) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); ResetDevice(foundDevice); } public void SetSendingMultipartMessage(byte address, bool isSendingMultipartMessage) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); foundDevice.IsSendingMultipartMessage = isSendingMultipartMessage; } public void SetReceivingMultipartMessage(byte address, bool isReceivingMultipartMessage) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); foundDevice.IsReceivingMultipartMessage = isReceivingMultipartMessage; } public void SetSendingMultiMessageNoSecureChannel(byte address, bool isSendingMultiMessageNoSecureChannel) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); foundDevice.IsSendingMultiMessageNoSecureChannel = isSendingMultiMessageNoSecureChannel; foundDevice.MessageControl.IsSendingMultiMessageNoSecureChannel = isSendingMultiMessageNoSecureChannel; @@ -488,7 +488,7 @@ public void SetSendingMultiMessageNoSecureChannel(byte address, bool isSendingMu public void SetRequestDelay(byte address, DateTime requestDelay) { - var foundDevice = _configuredDevices.ToArray().First(device => device.Address == address); + var foundDevice = _configuredDevices.First(device => device.Address == address); foundDevice.RequestDelay = requestDelay; } diff --git a/src/OSDP.Net/OSDP.Net.csproj b/src/OSDP.Net/OSDP.Net.csproj index b25726f2..f13bf8d8 100644 --- a/src/OSDP.Net/OSDP.Net.csproj +++ b/src/OSDP.Net/OSDP.Net.csproj @@ -29,6 +29,7 @@ + From cc7b4b055d5e773b70f327462257e11120945471 Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Fri, 10 Jan 2025 19:13:57 -0500 Subject: [PATCH 09/11] #189 Ensure the correct reply is processed when sending commands --- .../IntegrationTests/PeripheryDeviceTest.cs | 6 ++--- src/OSDP.Net/ControlPanel.cs | 2 +- src/OSDP.Net/Messages/ReplyTracker.cs | 27 ++++++++++++++----- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs index 9c9bb1b9..a708846c 100644 --- a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs +++ b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs @@ -12,7 +12,7 @@ namespace OSDP.Net.Tests.IntegrationTests; // -// NOTE: Majority of naming/structure in this file is very much a work-in-progress +// NOTE: The Majority of naming/structure in this file is very much a work-in-progress // and will be updated if we continue to build out a set of integration tests // // Presently this is a POC experiment to see how far we can take a test harness that @@ -24,7 +24,7 @@ namespace OSDP.Net.Tests.IntegrationTests; // need to be added to make it easy/clear to write assertions which wait for certain // events to occur (e.g. device came online). // -// NOTE: Integration tests by nature are SLOWER than unit tests. Hence why they are +// NOTE: Integration tests by nature are SLOWER than unit tests. Hence, why they are // tagged with "Integration" category as we might want to exclude them at some point if // the default PR test checks become too slow. There's only 5 tests here and they already // take 25 sec to run. Then again, this might also highlight improvement opportunity @@ -212,7 +212,7 @@ protected override PayloadData HandleIdReport() protected override PayloadData HandleDeviceCapabilities() { - var deviceCapabilities = new DeviceCapabilities(new[] + var deviceCapabilities = new DeviceCapabilities(ew[] { new DeviceCapability(CapabilityFunction.CardDataFormat, 1, 0), new DeviceCapability(CapabilityFunction.ReaderLEDControl, 1, 0), diff --git a/src/OSDP.Net/ControlPanel.cs b/src/OSDP.Net/ControlPanel.cs index f98e1bb7..9c2c680f 100644 --- a/src/OSDP.Net/ControlPanel.cs +++ b/src/OSDP.Net/ControlPanel.cs @@ -1029,7 +1029,7 @@ private async Task SendCommand(Guid connectionId, byte address, void EventHandler(object sender, ReplyEventArgs replyEventArgs) { var reply = replyEventArgs.Reply; - if (!reply.MatchIssuingCommand(command.Code)) return; + if (!reply.MatchIssuingCommand(address, command.Code)) return; if (throwOnNak && replyEventArgs.Reply.ReplyMessage.Type == (byte)ReplyType.Nak) { diff --git a/src/OSDP.Net/Messages/ReplyTracker.cs b/src/OSDP.Net/Messages/ReplyTracker.cs index 7e1d8e10..a5285c48 100644 --- a/src/OSDP.Net/Messages/ReplyTracker.cs +++ b/src/OSDP.Net/Messages/ReplyTracker.cs @@ -7,7 +7,8 @@ internal class ReplyTracker private readonly OutgoingMessage _issuingCommand; private readonly DeviceProxy _device; - public ReplyTracker(Guid connectionId, IncomingMessage replyMessage, OutgoingMessage issuingCommand, DeviceProxy device) + public ReplyTracker(Guid connectionId, IncomingMessage replyMessage, OutgoingMessage issuingCommand, + DeviceProxy device) { ConnectionId = connectionId; ReplyMessage = replyMessage; @@ -16,13 +17,27 @@ public ReplyTracker(Guid connectionId, IncomingMessage replyMessage, OutgoingMes } public IncomingMessage ReplyMessage { get; } - + public Guid ConnectionId { get; } - + + /// + /// Indicates whether the reply received is valid based on the originating command's address + /// and the correctness of the reply's data. + /// + /// + /// True if the reply is valid; otherwise, false. + /// public bool IsValidReply => ReplyMessage.Address == _issuingCommand.Address && ReplyMessage.IsDataCorrect; - - public bool MatchIssuingCommand(byte commandCode) => commandCode.Equals(_issuingCommand.Code); - + + /// + /// Determines if the given address and command code match the issuing command. + /// + /// The device address to check against the issuing command's address. + /// The command code to check against the issuing command's command code. + /// True if the provided address and command code match the issuing command; otherwise false. + public bool MatchIssuingCommand(byte address, byte commandCode) => + address.Equals(_issuingCommand.Address) && commandCode.Equals(_issuingCommand.Code); + internal bool ValidateSecureChannelEstablishment() { if (!ReplyMessage.SecureCryptogramHasBeenAccepted()) From 83329d33f2c45d6a675b9f58225d079ccf1745dc Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Tue, 27 May 2025 19:59:20 -0400 Subject: [PATCH 10/11] #194 Fix code assignment for Output status report --- src/OSDP.Net/Model/ReplyData/OutputStatus.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OSDP.Net/Model/ReplyData/OutputStatus.cs b/src/OSDP.Net/Model/ReplyData/OutputStatus.cs index c433222a..87ba8bb5 100644 --- a/src/OSDP.Net/Model/ReplyData/OutputStatus.cs +++ b/src/OSDP.Net/Model/ReplyData/OutputStatus.cs @@ -20,7 +20,7 @@ public OutputStatus(bool[] statuses) } /// - public override byte Code => (byte)ReplyType.InputStatusReport; + public override byte Code => (byte)ReplyType.OutputStatusReport; /// /// Gets the all the PDs output statuses as an array ordered by output number. From 8c5a71cdf25befdea004f7f60a72e96cfb6586d8 Mon Sep 17 00:00:00 2001 From: Jonathan Horvath Date: Tue, 27 May 2025 21:45:33 -0400 Subject: [PATCH 11/11] Fix build error --- src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs index a708846c..7cb68d63 100644 --- a/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs +++ b/src/OSDP.Net.Tests/IntegrationTests/PeripheryDeviceTest.cs @@ -212,8 +212,7 @@ protected override PayloadData HandleIdReport() protected override PayloadData HandleDeviceCapabilities() { - var deviceCapabilities = new DeviceCapabilities(ew[] - { + var deviceCapabilities = new DeviceCapabilities([ new DeviceCapability(CapabilityFunction.CardDataFormat, 1, 0), new DeviceCapability(CapabilityFunction.ReaderLEDControl, 1, 0), new DeviceCapability(CapabilityFunction.ReaderTextOutput, 0, 0), @@ -221,7 +220,7 @@ protected override PayloadData HandleDeviceCapabilities() new DeviceCapability(CapabilityFunction.CommunicationSecurity, 1, 1), new DeviceCapability(CapabilityFunction.ReceiveBufferSize, 0, 1), new DeviceCapability(CapabilityFunction.OSDPVersion, 2, 0) - }); + ]); return deviceCapabilities; }