[knx] add initial support for KNX data secure [WIP], openhab#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 <mail@holger-friedrich.de>
holgerfriedrich · Mar 21, 2021 · 7c06668 · 7c06668
1 parent ebdb916
commit 7c06668
Show file tree

Hide file tree

Showing 13 changed files with 260 additions and 11 deletions.
diff --git a/.github/workflows/build-knx-addon.yml 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
@@ -22,7 +22,7 @@
     <dependency>
       <groupId>com.github.calimero</groupId>
       <artifactId>calimero-core</artifactId>
-      <version>2.4</version>
+      <version>2.5-M1</version>
       <scope>compile</scope>
       <exclusions>
         <exclusion>
@@ -34,7 +34,7 @@
     <dependency>
       <groupId>com.github.calimero</groupId>
       <artifactId>calimero-device</artifactId>
-      <version>2.4</version>
+      <version>2.5-M1</version>
       <scope>compile</scope>
       <exclusions>
         <exclusion>
@@ -46,7 +46,7 @@
     <dependency>
       <groupId>com.github.calimero</groupId>
       <artifactId>calimero-rxtx</artifactId>
-      <version>2.4</version>
+      <version>2.5-M1</version>
       <scope>compile</scope>
       <exclusions>
         <exclusion>

diff --git a/...enhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/KNXBindingConstants.java b/...enhab.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 <a
     // href="http://www.iana.org/assignments/multicast-addresses/multicast-addresses.xml">iana</a> EIBnet/IP

diff --git a/....binding.knx/src/main/java/org/openhab/binding/knx/internal/client/AbstractKNXClient.java b/....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/...inding.knx/src/main/java/org/openhab/binding/knx/internal/config/BridgeConfiguration.java b/...inding.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/...inding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java b/...inding.knx/src/main/java/org/openhab/binding/knx/internal/handler/DeviceThingHandler.java
@@ -72,8 +72,8 @@ public class DeviceThingHandler extends AbstractKNXThingHandler {
     private final Set<GroupAddress> groupAddresses = new HashSet<>();
     private final Set<GroupAddress> groupAddressesWriteBlockedOnce = new HashSet<>();
     private final Set<OutboundSpec> groupAddressesRespondingSpec = new HashSet<>();
-    private final Map<GroupAddress, ScheduledFuture<?>> readFutures = new HashMap<>();
-    private final Map<ChannelUID, ScheduledFuture<?>> channelFutures = new HashMap<>();
+    private final Map<GroupAddress, @Nullable ScheduledFuture<?>> readFutures = new HashMap<>();
+    private final Map<ChannelUID, @Nullable ScheduledFuture<?>> channelFutures = new HashMap<>();
     private int readInterval;
 
     public DeviceThingHandler(Thing thing) {

diff --git a/...ding.knx/src/main/java/org/openhab/binding/knx/internal/handler/IPBridgeThingHandler.java b/...ding.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/...knx/src/main/java/org/openhab/binding/knx/internal/handler/KNXBridgeBaseThingHandler.java b/...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,13 +48,47 @@ public abstract class KNXBridgeBaseThingHandler extends BaseBridgeHandler implem
     protected ConcurrentHashMap<IndividualAddress, Destination> 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);
     }
 
     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/....knx/src/main/java/org/openhab/binding/knx/internal/handler/SerialBridgeThingHandler.java b/....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
@@ -66,6 +66,16 @@
 				<description>Seconds between connection retries when KNX link has been lost, 0 means never retry, minimum 30s</description>
 				<default>60</default>
 			</parameter>
+			<parameter name="keyringFile" type="text">
+				<label>Keyring file</label>
+				<description>Keyring file exported from ETS and placed in openhab config/misc folder, e.g. knx.knxkeys</description>
+				<default></default>
+			</parameter>
+			<parameter name="keyringPassword" type="text">
+				<label>Keyring password</label>
+				<description>Keyring file password (set during exported from ETS)</description>
+				<default></default>
+			</parameter>
 		</config-description>
 	</bridge-type>
 

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
@@ -39,6 +39,16 @@
 				<required>true</required>
 				<default>0</default>
 			</parameter>
+			<parameter name="keyringFile" type="text">
+				<label>Keyring file</label>
+				<description>Keyring file exported from ETS and placed in openhab config/misc folder, e.g. knx.knxkeys</description>
+				<default></default>
+			</parameter>
+			<parameter name="keyringPassword" type="text">
+				<label>Keyring password</label>
+				<description>Keyring file password (set during exported from ETS)</description>
+				<default></default>
+			</parameter>
 		</config-description>
 	</bridge-type>