From 4aae0a89c92241989550c4a3aabc93dd6ccce830 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 * build for OH 302 and 31x Signed-off-by: Holger Friedrich --- .github/workflows/build-knx-addon.yml | 44 +++++++++++ .../knx/internal/KNXBindingConstants.java | 2 + .../internal/client/AbstractKNXClient.java | 20 ++++- .../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 +++++ 12 files changed, 255 insertions(+), 4 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..edbb894c0ff9e --- /dev/null +++ b/.github/workflows/build-knx-addon.yml @@ -0,0 +1,44 @@ +# 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: + build302: + + 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 -Dohc.version=3.0.2 + - uses: actions/upload-artifact@v2 + with: + name: org.openhab.binding.knx.302 + path: bundles/org.openhab.binding.knx/target/org.openhab.binding.knx-*.jar + + build31x: + + 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.31x + path: bundles/org.openhab.binding.knx/target/org.openhab.binding.knx-*.jar + 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 4f243cb62c723..58be5ef26a5ab 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 695edb277edfe..5399260b1d962 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.time.Duration; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; @@ -26,6 +29,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; @@ -189,6 +193,19 @@ private synchronized boolean connect() { deviceInfoClient = new DeviceInfoClientImpl(managementClient); + // 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("file:///" + 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 = ""; + ProcessCommunicator processCommunicator = new ProcessCommunicatorImpl(link); processCommunicator.responseTimeout(Duration.ofSeconds(responseTimeout)); processCommunicator.addProcessListener(processListener); @@ -217,9 +234,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); } 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 1d12d324f16c4..6e7c95297784a 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 2e5ff3c8fd525..7b91d746b3e16 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 e40f126f6c40e..be828abab30b7 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 { + if (initializeSecurity(config.getKeyringFile(), config.getKeyringPassword())) + logger.info("KNX security available for {} group addresses, {} devices", + Security.defaultInstallation().groupKeys().size(), + Security.defaultInstallation().deviceToolKeys().size()); + } catch (KnxSecureException e) { + logger.warn("{}", e.toString()); + } + 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 e9c087b4acf5a..0e92be8c9f856 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 boolean initializeSecurity(String keyringFile, String password) { + keyring = null; + keyringPassword = null; + + if (keyringFile == null || keyringFile.trim().isEmpty()) + return false; + 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; + } + return true; + } + @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 5e6738646a463..2b0bdeb197a62 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 { + if (initializeSecurity(config.getKeyringFile(), config.getKeyringPassword())) + logger.debug("KNX security available for {} group addresses, {} devices", + Security.defaultInstallation().groupKeys().size(), + Security.defaultInstallation().deviceToolKeys().size()); + } catch (KnxSecureException e) { + logger.warn("{}", e.toString()); + } 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 89e2c6472f22f..a6811a9b02ebb 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 @@ -64,6 +64,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 bb7705928889d..fb6ebf5328f6f 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 @@ -34,6 +34,16 @@ Seconds between connect retries when KNX link has been lost, 0 means never retry 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 @@ + + + + + + + + + + + + + + + + +