From 4a19d46c3ba54c1accf035dce99ac501389da730 Mon Sep 17 00:00:00 2001 From: Holger Friedrich Date: Fri, 8 Nov 2024 22:19:57 +0100 Subject: [PATCH] [knx] Improve handling of unknown encrypted frames * Show encrypted frames without a matching key using console command "openhab:knx list-unknown-ga"; sort output numerically by GA. * Add trace logging to show decryption error due to missing key. Previously, those frames were silently dropped unless logging for Calimero was explicitly enabled as well. Signed-off-by: Holger Friedrich --- .../internal/client/AbstractKNXClient.java | 49 ++++++++++++++++--- .../internal/console/KNXCommandExtension.java | 2 +- .../handler/KNXBridgeBaseThingHandler.java | 3 +- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java index fc2a4c1775d9f..3ac7fd7286d29 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java @@ -39,12 +39,16 @@ import org.slf4j.LoggerFactory; import tuwien.auto.calimero.CloseEvent; +import tuwien.auto.calimero.DataUnitBuilder; import tuwien.auto.calimero.DetachEvent; import tuwien.auto.calimero.FrameEvent; import tuwien.auto.calimero.GroupAddress; import tuwien.auto.calimero.IndividualAddress; +import tuwien.auto.calimero.KNXAddress; import tuwien.auto.calimero.KNXException; import tuwien.auto.calimero.KNXIllegalArgumentException; +import tuwien.auto.calimero.cemi.CEMILData; +import tuwien.auto.calimero.cemi.CemiTData; import tuwien.auto.calimero.datapoint.CommandDP; import tuwien.auto.calimero.datapoint.Datapoint; import tuwien.auto.calimero.device.ProcessCommunicationResponder; @@ -60,6 +64,7 @@ import tuwien.auto.calimero.process.ProcessEvent; import tuwien.auto.calimero.process.ProcessListener; import tuwien.auto.calimero.secure.KnxSecureException; +import tuwien.auto.calimero.secure.SecureApplicationLayer; import tuwien.auto.calimero.secure.Security; /** @@ -348,12 +353,13 @@ private void processEvent(String task, ProcessEvent event, ListenerNotification if (!isHandled) { logger.trace("Address '{}' is not configured in openHAB", destination); final String type = switch (event.getServiceCode()) { - case 0x80 -> " GROUP_WRITE("; - case 0x40 -> " GROUP_RESPONSE("; - case 0x00 -> " GROUP_READ("; - default -> " ?("; + case 0x80 -> "GROUP_WRITE"; + case 0x40 -> "GROUP_RESPONSE"; + case 0x00 -> "GROUP_READ"; + default -> "?"; }; - final String key = destination.toString() + type + event.getASDU().length + ")"; + final String key = String.format("%2d/%1d/%3d %s(%02d)", destination.getMainGroup(), + destination.getMiddleGroup(), destination.getSubGroup8(), type, event.getASDU().length); commandExtensionData.unknownGA().compute(key, (k, v) -> v == null ? 1 : v + 1); } } @@ -429,7 +435,38 @@ public void linkClosed(@Nullable CloseEvent closeEvent) { @Override public void indication(@Nullable FrameEvent e) { - // no-op + // NetworkLinkListener indication. This implementation is triggered whenever a frame is received. + // It is not necessary for OH, as we process incoming group writes via different triggers. + // However, this indication also covers encrypted data secure frames, which would typically + // be dropped silently by the Calimero library (a log message is only visible when log level for Calimero + // is set manually). + + // Implementation searches for incoming data secure frames which cannot be decoded due to missing key + if (e != null) { + final var cemi = e.getFrame(); + if (!(cemi instanceof CemiTData)) { + final CEMILData f = (CEMILData) cemi; + final int ctrl = f.getPayload()[0] & 0xfc; + if (ctrl == 0) { + final KNXAddress dst = f.getDestination(); + if (dst instanceof GroupAddress ga) { + if (dst.getRawAddress() != 0) { + final byte[] payload = f.getPayload(); + final int service = DataUnitBuilder.getAPDUService(payload); + if (service == SecureApplicationLayer.SecureService) { + if (!openhabSecurity.groupKeys().containsKey(dst)) { + logger.trace("Address '{}' cannot be decrypted, group key missing", dst); + final String key = String.format( + "%2d/%1d/%3d secure: missing group key, cannot decrypt", ga.getMainGroup(), + ga.getMiddleGroup(), ga.getSubGroup8()); + commandExtensionData.unknownGA().compute(key, (k, v) -> v == null ? 1 : v + 1); + } + } + } + } + } + } + } } @Override diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/console/KNXCommandExtension.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/console/KNXCommandExtension.java index fe1b6d2590cbd..4a0faed330c7f 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/console/KNXCommandExtension.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/console/KNXCommandExtension.java @@ -56,7 +56,7 @@ public void execute(String[] args, Console console) { console.println("KNX bridge \"" + bridgeHandler.getThing().getLabel() + "\": group address, type, number of bytes, and number of occurrence since last reload of binding:"); for (Entry entry : bridgeHandler.getCommandExtensionData().unknownGA().entrySet()) { - console.println(entry.getKey() + " " + entry.getValue()); + console.println(entry.getKey() + " " + entry.getValue()); } } return; diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/KNXBridgeBaseThingHandler.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/KNXBridgeBaseThingHandler.java index a2ab8573d4e5f..9c5c1a70c3d8a 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/KNXBridgeBaseThingHandler.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/KNXBridgeBaseThingHandler.java @@ -21,6 +21,7 @@ import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; +import java.util.SortedMap; import java.util.TreeMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -85,7 +86,7 @@ public SecureRoutingConfig() { * Helper class to carry information which can be used by the * command line extension (openHAB console). */ - public record CommandExtensionData(Map unknownGA) { + public record CommandExtensionData(SortedMap unknownGA) { } private final ScheduledExecutorService knxScheduler = ThreadPoolManager.getScheduledPool("knx");