Skip to content

Commit

Permalink
[knx] add initial support for KNX data secure [WIP], openhab#8872
Browse files Browse the repository at this point in the history
* 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 <mail@holger-friedrich.de>
  • Loading branch information
holgerfriedrich committed Jan 26, 2022
1 parent 56b2f47 commit 4aae0a8
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 4 deletions.
44 changes: 44 additions & 0 deletions .github/workflows/build-knx-addon.yml
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,4 +49,12 @@ public BigDecimal getResponseTimeout() {
public void setAutoReconnectPeriod(int period) {
autoReconnectPeriod = period;
}

public String getKeyringFile() {
return keyringFile;
}

public String getKeyringPassword() {
return keyringPassword;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@
<description>Seconds between connect retries when KNX link has been lost, 0 means never retry</description>
<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>

Expand Down
Original file line number Diff line number Diff line change
@@ -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<GroupAddress, byte[]> groupKeys = Security.defaultInstallation().groupKeys();
assertEquals(3, groupKeys.size());
groupKeys.remove(ga);
assertEquals(2, groupKeys.size());
Security.defaultInstallation().useKeyring(keys, password);
Map<GroupAddress, byte[]> 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());
}
}
Loading

0 comments on commit 4aae0a8

Please sign in to comment.