From 7c0666881f7e382e5d09304411675a6ed01e6198 Mon Sep 17 00:00:00 2001 From: Holger Friedrich Date: Tue, 8 Dec 2020 01:50:09 +0100 Subject: [PATCH] [knx] add initial support for KNX data secure [WIP], #8872 * use Calimero library in latest version 2.5-M1 (to be replaced once release 2.5 is available) * add config options for keyring file(s) and password(s) * add tests for security functions * TODO replace ProcessCommunicationResponder Signed-off-by: Holger Friedrich --- .github/workflows/build-knx-addon.yml | 27 +++++++ bundles/org.openhab.binding.knx/pom.xml | 6 +- .../knx/internal/KNXBindingConstants.java | 2 + .../internal/client/AbstractKNXClient.java | 43 +++++++++-- .../internal/config/BridgeConfiguration.java | 10 +++ .../internal/handler/DeviceThingHandler.java | 4 +- .../handler/IPBridgeThingHandler.java | 12 +++ .../handler/KNXBridgeBaseThingHandler.java | 39 ++++++++++ .../handler/SerialBridgeThingHandler.java | 15 ++++ .../src/main/resources/OH-INF/thing/ip.xml | 10 +++ .../main/resources/OH-INF/thing/serial.xml | 10 +++ .../internal/security/KNXSecurityTest.java | 76 +++++++++++++++++++ .../src/test/resources/test.knxkeys | 17 +++++ 13 files changed, 260 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/build-knx-addon.yml create mode 100644 bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/security/KNXSecurityTest.java create mode 100644 bundles/org.openhab.binding.knx/src/test/resources/test.knxkeys diff --git a/.github/workflows/build-knx-addon.yml b/.github/workflows/build-knx-addon.yml new file mode 100644 index 0000000000000..1d3622e63aef0 --- /dev/null +++ b/.github/workflows/build-knx-addon.yml @@ -0,0 +1,27 @@ +# This workflow will build a Java project with Maven +# For more information see: https://help.github.com/actions/language-and-framework-guides/building-and-testing-java-with-maven + +name: Build knx addon with Maven + +on: + push: + branches: [ pr-knx-data-secure ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Set up Java 11 + uses: actions/setup-java@v1 + with: + java-version: 11 + - name: Build with Maven + run: mvn -B package --file pom.xml -pl :org.openhab.binding.knx + - uses: actions/upload-artifact@v2 + with: + name: org.openhab.binding.knx.tgz + path: openhab-addons/bundles/org.openhab.binding.knx.tgz + diff --git a/bundles/org.openhab.binding.knx/pom.xml b/bundles/org.openhab.binding.knx/pom.xml index be2da7a25863b..f0d3e484b8123 100644 --- a/bundles/org.openhab.binding.knx/pom.xml +++ b/bundles/org.openhab.binding.knx/pom.xml @@ -22,7 +22,7 @@ com.github.calimero calimero-core - 2.4 + 2.5-M1 compile @@ -34,7 +34,7 @@ com.github.calimero calimero-device - 2.4 + 2.5-M1 compile @@ -46,7 +46,7 @@ com.github.calimero calimero-rxtx - 2.4 + 2.5-M1 compile diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXBindingConstants.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXBindingConstants.java index 10e075d7654e1..f129efbe59bfd 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXBindingConstants.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXBindingConstants.java @@ -51,6 +51,8 @@ public class KNXBindingConstants { public static final String LOCAL_SOURCE_ADDRESS = "localSourceAddr"; public static final String PORT_NUMBER = "portNumber"; public static final String SERIAL_PORT = "serialPort"; + public static final String KEYRING_FILE = "keyringFile"; + public static final String KEYRING_PASSWORD = "keyringPassword"; // The default multicast ip address (see iana EIBnet/IP 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 de860c6bed274..7d628ce3717b5 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 @@ -12,6 +12,9 @@ */ package org.openhab.binding.knx.internal.client; +import java.io.File; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.LinkedBlockingQueue; @@ -25,6 +28,7 @@ import org.openhab.binding.knx.internal.KNXTypeMapper; import org.openhab.binding.knx.internal.dpt.KNXCoreTypeMapper; import org.openhab.binding.knx.internal.handler.GroupAddressListener; +import org.openhab.core.OpenHAB; import org.openhab.core.thing.ThingStatus; import org.openhab.core.thing.ThingStatusDetail; import org.openhab.core.thing.ThingUID; @@ -34,12 +38,14 @@ import tuwien.auto.calimero.CloseEvent; import tuwien.auto.calimero.DetachEvent; +import tuwien.auto.calimero.DeviceDescriptor; import tuwien.auto.calimero.FrameEvent; import tuwien.auto.calimero.GroupAddress; import tuwien.auto.calimero.IndividualAddress; import tuwien.auto.calimero.KNXException; import tuwien.auto.calimero.datapoint.CommandDP; import tuwien.auto.calimero.datapoint.Datapoint; +import tuwien.auto.calimero.device.BaseKnxDevice; import tuwien.auto.calimero.device.ProcessCommunicationResponder; import tuwien.auto.calimero.link.KNXNetworkLink; import tuwien.auto.calimero.link.NetworkLinkListener; @@ -48,7 +54,7 @@ import tuwien.auto.calimero.mgmt.ManagementClientImpl; import tuwien.auto.calimero.mgmt.ManagementProcedures; import tuwien.auto.calimero.mgmt.ManagementProceduresImpl; -import tuwien.auto.calimero.process.ProcessCommunicationBase; +import tuwien.auto.calimero.process.ProcessCommunication; import tuwien.auto.calimero.process.ProcessCommunicator; import tuwien.auto.calimero.process.ProcessCommunicatorImpl; import tuwien.auto.calimero.process.ProcessEvent; @@ -186,12 +192,38 @@ private synchronized boolean connect() { deviceInfoClient = new DeviceInfoClientImpl(managementClient); - ProcessCommunicator processCommunicator = new ProcessCommunicatorImpl(link); + // KNX secure "device" needs to store properties, e.g. sequence numbers. + // Store in userdata folder for now. + // TODO: ensure this is written from time to time to safeguard against sudden crashes + final String folder = OpenHAB.getUserDataFolder(); + URI ios = null; + try { + ios = new URI(folder + File.separator + "knx_secure_device.xml"); + } catch (URISyntaxException e) { + logger.warn("failure specifying setting file, {}", e.toString()); + } + // no encryption of property file for now + final String iosPassword = ""; + + // public BaseKnxDevice(final String name, final DeviceDescriptor dd, final ProcessCommunicationService + // process, + // final ManagementService mgmt, final URI iosResource, final char[] iosPassword) throws + // KnxPropertyException + BaseKnxDevice dev = new BaseKnxDevice("openHAB", DeviceDescriptor.DD0.TYPE_07B0, null, null, ios, + iosPassword.toCharArray()); + // use existing link, derive address from link setting + dev.setDeviceLink(link); + // manually setting the address is only allowed if we inherit BaseKnxDevice + // dev.setAddress(new IndividualAddress("1.0.128")); + + ProcessCommunicator processCommunicator = new ProcessCommunicatorImpl(link, dev.secureApplicationLayer()); processCommunicator.setResponseTimeout(responseTimeout); processCommunicator.addProcessListener(processListener); this.processCommunicator = processCommunicator; - ProcessCommunicationResponder responseCommunicator = new ProcessCommunicationResponder(link); + ProcessCommunicationResponder responseCommunicator = new ProcessCommunicationResponder(link, + dev.secureApplicationLayer()); // new + // tuwien.auto.calimero.device.DeviceSecureApplicationLayer(dev)); this.responseCommunicator = responseCommunicator; link.addLinkListener(this); @@ -213,9 +245,8 @@ private synchronized boolean connect() { private void disconnect(@Nullable Exception e) { releaseConnection(); if (e != null) { - String message = e.getLocalizedMessage(); statusUpdateCallback.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, - message != null ? message : ""); + "" + e.getLocalizedMessage()); } else { statusUpdateCallback.updateStatus(ThingStatus.OFFLINE); } @@ -439,7 +470,7 @@ public void respondToKNX(OutboundSpec responseSpec) throws KNXException { } } - private void sendToKNX(ProcessCommunicationBase communicator, KNXNetworkLink link, GroupAddress groupAddress, + private void sendToKNX(ProcessCommunication communicator, KNXNetworkLink link, GroupAddress groupAddress, String dpt, Type type) throws KNXException { if (!connectIfNotAutomatic()) { return; diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java index a41ed96567ea5..075d2bb56fdb3 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java @@ -27,6 +27,8 @@ public class BridgeConfiguration { private BigDecimal readingPause; private BigDecimal readRetriesLimit; private BigDecimal responseTimeout; + private String keyringFile; + private String keyringPassword; public int getAutoReconnectPeriod() { return autoReconnectPeriod; @@ -47,4 +49,12 @@ public BigDecimal getResponseTimeout() { public void setAutoReconnectPeriod(int period) { autoReconnectPeriod = period; } + + public String getKeyringFile() { + return keyringFile; + } + + public String getKeyringPassword() { + return keyringPassword; + } } diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java index 83c0576040c6c..0b00159f25af1 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java @@ -72,8 +72,8 @@ public class DeviceThingHandler extends AbstractKNXThingHandler { private final Set groupAddresses = new HashSet<>(); private final Set groupAddressesWriteBlockedOnce = new HashSet<>(); private final Set groupAddressesRespondingSpec = new HashSet<>(); - private final Map> readFutures = new HashMap<>(); - private final Map> channelFutures = new HashMap<>(); + private final Map> readFutures = new HashMap<>(); + private final Map> channelFutures = new HashMap<>(); private int readInterval; public DeviceThingHandler(Thing thing) { diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java index ec162526d3726..ea659e472575d 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java @@ -30,6 +30,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tuwien.auto.calimero.secure.KnxSecureException; +import tuwien.auto.calimero.secure.Security; + /** * The {@link IPBridgeThingHandler} is responsible for handling commands, which are * sent to one of the channels. It implements a KNX/IP Gateway, that either acts a a @@ -57,6 +60,15 @@ public IPBridgeThingHandler(Bridge bridge, NetworkAddressService networkAddressS @Override public void initialize() { IPBridgeConfiguration config = getConfigAs(IPBridgeConfiguration.class); + try { + inializeSecurity(config.getKeyringFile(), config.getKeyringPassword()); + } catch (KnxSecureException e) { + logger.warn("{}", e.toString()); + } + logger.debug("Security enabled for {} group addresses, {} devices", + Security.defaultInstallation().groupKeys().size(), + Security.defaultInstallation().deviceToolKeys().size()); + logger.debug("Security: {}", Security.defaultInstallation().groupSenders().toString()); int autoReconnectPeriod = config.getAutoReconnectPeriod(); if (autoReconnectPeriod != 0 && autoReconnectPeriod < 30) { logger.info("autoReconnectPeriod for {} set to {}s, allowed range is 0 (never) or >30", thing.getUID(), 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 9e4ca0b5b68fd..f57ff12dc14d9 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 @@ -12,6 +12,7 @@ */ package org.openhab.binding.knx.internal.handler; +import java.io.File; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -20,6 +21,7 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.knx.internal.client.KNXClient; import org.openhab.binding.knx.internal.client.StatusUpdateCallback; +import org.openhab.core.OpenHAB; import org.openhab.core.common.ThreadPoolManager; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.ChannelUID; @@ -30,6 +32,9 @@ import tuwien.auto.calimero.IndividualAddress; import tuwien.auto.calimero.mgmt.Destination; +import tuwien.auto.calimero.secure.Keyring; +import tuwien.auto.calimero.secure.KnxSecureException; +import tuwien.auto.calimero.secure.Security; /** * The {@link KNXBridgeBaseThingHandler} is responsible for handling commands, which are @@ -43,6 +48,8 @@ public abstract class KNXBridgeBaseThingHandler extends BaseBridgeHandler implem protected ConcurrentHashMap destinations = new ConcurrentHashMap<>(); private final ScheduledExecutorService knxScheduler = ThreadPoolManager.getScheduledPool("knx"); private final ScheduledExecutorService backgroundScheduler = Executors.newSingleThreadScheduledExecutor(); + private @Nullable Keyring keyring; + private @Nullable String keyringPassword; public KNXBridgeBaseThingHandler(Bridge bridge) { super(bridge); @@ -50,6 +57,38 @@ public KNXBridgeBaseThingHandler(Bridge bridge) { protected abstract KNXClient getClient(); + protected void inializeSecurity(String keyringFile, String password) { + keyring = null; + keyringPassword = null; + + if (keyringFile != null && !keyringFile.trim().isEmpty()) { + try { + // load keyring file from config dir, folder misc + String keyringUri = OpenHAB.getConfigFolder() + File.separator + "misc" + File.separator + + keyringFile.trim(); + keyring = Keyring.load(keyringUri); + if (keyring == null) + throw new KnxSecureException("keyring file configured, but loading failed: " + keyringUri); + + // loading was successful, check signatures + if (!keyring.verifySignature(password.toCharArray())) + throw new KnxSecureException( + "signature verification failed, please check keyring file: " + keyringUri); + keyringPassword = password; + + // Add to global static key(ring) storage of Calimero library. + // More than one can be added ONLY IF addresses are different, + // as Calimero adds all information to this static object. + // -> to be discussed with owner of Calimero lib. + Security.defaultInstallation().useKeyring(keyring, keyringPassword.toCharArray()); + } catch (KnxSecureException e) { + keyring = null; + keyringPassword = null; + throw e; + } + } + } + @Override public void handleCommand(ChannelUID channelUID, Command command) { // Nothing to do here diff --git a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java index 590d4f0825ffd..42bd299db7a67 100644 --- a/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java +++ b/bundles/org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java @@ -18,6 +18,11 @@ import org.openhab.binding.knx.internal.config.SerialBridgeConfiguration; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.ThingStatus; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import tuwien.auto.calimero.secure.KnxSecureException; +import tuwien.auto.calimero.secure.Security; /** * The {@link IPBridgeThingHandler} is responsible for handling commands, which are @@ -31,6 +36,7 @@ @NonNullByDefault public class SerialBridgeThingHandler extends KNXBridgeBaseThingHandler { + private final Logger logger = LoggerFactory.getLogger(SerialBridgeThingHandler.class); private final SerialClient client; public SerialBridgeThingHandler(Bridge bridge) { @@ -44,6 +50,15 @@ public SerialBridgeThingHandler(Bridge bridge) { @Override public void initialize() { updateStatus(ThingStatus.UNKNOWN); + SerialBridgeConfiguration config = getConfigAs(SerialBridgeConfiguration.class); + try { + inializeSecurity(config.getKeyringFile(), config.getKeyringPassword()); + } catch (KnxSecureException e) { + logger.warn("{}", e.toString()); + } + logger.debug("Security enabled for {} group addresses, {} devices", + Security.defaultInstallation().groupKeys().size(), + Security.defaultInstallation().deviceToolKeys().size()); client.initialize(); } diff --git a/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/ip.xml b/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/ip.xml index 36fb253ed5669..6dbdc8273af42 100644 --- a/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/ip.xml +++ b/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/ip.xml @@ -66,6 +66,16 @@ Seconds between connection retries when KNX link has been lost, 0 means never retry, minimum 30s 60 + + + Keyring file exported from ETS and placed in openhab config/misc folder, e.g. knx.knxkeys + + + + + Keyring file password (set during exported from ETS) + + diff --git a/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/serial.xml b/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/serial.xml index 1a5f39510c82e..fac812089ad49 100644 --- a/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/serial.xml +++ b/bundles/org.openhab.binding.knx/src/main/resources/OH-INF/thing/serial.xml @@ -39,6 +39,16 @@ true 0 + + + Keyring file exported from ETS and placed in openhab config/misc folder, e.g. knx.knxkeys + + + + + Keyring file password (set during exported from ETS) + + diff --git a/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/security/KNXSecurityTest.java b/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/security/KNXSecurityTest.java new file mode 100644 index 0000000000000..8d5c4595c370c --- /dev/null +++ b/bundles/org.openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/security/KNXSecurityTest.java @@ -0,0 +1,76 @@ +/** + * Copyright (c) 2010-2021 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.binding.knx.internal.security; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import tuwien.auto.calimero.GroupAddress; +import tuwien.auto.calimero.IndividualAddress; +import tuwien.auto.calimero.secure.Keyring; +import tuwien.auto.calimero.secure.Security; + +/** + * + * @author Simon Kaufmann - initial contribution and API + * + */ +public class KNXSecurityTest { + + @Test + public void testCalimero_keyring() { + final String testFile = getClass().getClassLoader().getResource("test.knxkeys").toString(); + final char[] password = "habopen".toCharArray(); + + assertNotEquals("", testFile); + Keyring keys = Keyring.load(testFile); + assertTrue(keys.verifySignature(password)); + + System.out.println(keys.devices().toString()); + System.out.println(keys.groups().toString()); + System.out.println(keys.interfaces().toString()); + + GroupAddress ga = new GroupAddress(8, 0, 0); + byte[] key800enc = keys.groups().get(ga); + assertNotEquals(0, key800enc.length); + byte[] key800dec = keys.decryptKey(key800enc, password); + assertEquals(16, key800dec.length); + + IndividualAddress pa = new IndividualAddress(1, 2, 72); + Keyring.Device dev = keys.devices().get(pa); + // cannot check this for dummy test file, needs real device to be included + // assertNotEquals(0, dev.sequenceNumber()); + + // currently Calimero uses _one_ static map to store all keys + // -> check if this is still the case + Security.defaultInstallation().useKeyring(keys, password); + Map groupKeys = Security.defaultInstallation().groupKeys(); + assertEquals(3, groupKeys.size()); + groupKeys.remove(ga); + assertEquals(2, groupKeys.size()); + Security.defaultInstallation().useKeyring(keys, password); + Map groupKeys2 = Security.defaultInstallation().groupKeys(); + assertEquals(3, groupKeys2.size()); + assertEquals(3, groupKeys.size()); + ga = new GroupAddress(1, 0, 0); + groupKeys.put(ga, new byte[1]); + assertEquals(4, groupKeys2.size()); + assertEquals(4, groupKeys.size()); + Security.defaultInstallation().useKeyring(keys, password); + assertEquals(4, groupKeys2.size()); + assertEquals(4, groupKeys.size()); + } +} diff --git a/bundles/org.openhab.binding.knx/src/test/resources/test.knxkeys b/bundles/org.openhab.binding.knx/src/test/resources/test.knxkeys new file mode 100644 index 0000000000000..0da9d56dc7412 --- /dev/null +++ b/bundles/org.openhab.binding.knx/src/test/resources/test.knxkeys @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + +