public/openhab-addons
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[mqtt] Fix most SAT findings (#12492)

Browse Source 
Signed-off-by: Wouter Born <github@maindrain.net>
This commit is contained in:
Wouter Born
2022-03-19 09:27:41 +01:00
committed by GitHub
parent af8202e668
commit a6f5b48dd5
60 changed files with 490 additions and 481 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/handler/BrokerHandler.java
View File

@@ -154,13 +154,13 @@ public class BrokerHandler extends AbstractBrokerHandler implements PinnedCallba
try {
Pin pin;
if (config.certificate.isBlank()) {
pin = Pin.LearningPin(PinType.CERTIFICATE_TYPE);
pin = Pin.learningPin(PinType.CERTIFICATE_TYPE);
} else {
String[] split = config.certificate.split(":");
if (split.length != 2) {
throw new NoSuchAlgorithmException("Algorithm is missing");
}
pin = Pin.CheckingPin(PinType.CERTIFICATE_TYPE, new PinMessageDigest(split[0]),
pin = Pin.checkingPin(PinType.CERTIFICATE_TYPE, new PinMessageDigest(split[0]),
HexUtils.hexToBytes(split[1]));
}
trustManager.addPinning(pin);
@@ -172,13 +172,13 @@ public class BrokerHandler extends AbstractBrokerHandler implements PinnedCallba
try {
Pin pin;
if (config.publickey.isBlank()) {
pin = Pin.LearningPin(PinType.PUBLIC_KEY_TYPE);
pin = Pin.learningPin(PinType.PUBLIC_KEY_TYPE);
} else {
String[] split = config.publickey.split(":");
if (split.length != 2) {
throw new NoSuchAlgorithmException("Algorithm is missing");
}
pin = Pin.CheckingPin(PinType.PUBLIC_KEY_TYPE, new PinMessageDigest(split[0]),
pin = Pin.checkingPin(PinType.PUBLIC_KEY_TYPE, new PinMessageDigest(split[0]),
HexUtils.hexToBytes(split[1]));
}
trustManager.addPinning(pin);

4
bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/internal/MqttBrokerHandlerFactory.java
View File

@@ -34,8 +34,6 @@ import org.openhab.core.thing.binding.BaseThingHandlerFactory;
import org.openhab.core.thing.binding.ThingHandler;
import org.openhab.core.thing.binding.ThingHandlerFactory;
import org.osgi.service.component.annotations.Component;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* The {@link MqttBrokerHandlerFactory} is responsible for creating things and thing
@@ -52,8 +50,6 @@ public class MqttBrokerHandlerFactory extends BaseThingHandlerFactory implements
private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Stream
.of(MqttBindingConstants.BRIDGE_TYPE_BROKER).collect(Collectors.toSet());
private final Logger logger = LoggerFactory.getLogger(MqttBrokerHandlerFactory.class);
/**
* This Map provides a lookup between a Topic string (key) and a Set of MQTTTopicDiscoveryParticipants (value),
* where the Set itself is a list of participants which are subscribed to the respective Topic.

2
bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/internal/MqttThingID.java
View File

@@ -12,6 +12,7 @@
*/
package org.openhab.binding.mqtt.internal;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.mqtt.MqttBindingConstants;
import org.openhab.core.thing.ThingUID;
@@ -20,6 +21,7 @@ import org.openhab.core.thing.ThingUID;
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public class MqttThingID {
/**
* Convert the url (tcp://122.123.111.123:1883) to a version without colons, dots or slashes

4
bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/internal/ssl/Pin.java
View File

@@ -72,11 +72,11 @@ public class Pin {
this.pinData = data;
}
public static Pin LearningPin(PinType pinType) {
public static Pin learningPin(PinType pinType) {
return new Pin(pinType, null, true, null);
}
public static Pin CheckingPin(PinType pinType, PinMessageDigest method, byte[] pinData) {
public static Pin checkingPin(PinType pinType, PinMessageDigest method, byte[] pinData) {
return new Pin(pinType, method, false, pinData);
}

3
bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/internal/ssl/PinType.java
View File

@@ -12,11 +12,14 @@
*/
package org.openhab.binding.mqtt.internal.ssl;
import org.eclipse.jdt.annotation.NonNullByDefault;
/**
* A {@link Pin} is either a Public Key or Certificate Pin.
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public enum PinType {
PUBLIC_KEY_TYPE,
CERTIFICATE_TYPE

5
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/BrokerHandlerEx.java
View File

@@ -14,7 +14,7 @@ package org.openhab.binding.mqtt.handler;
import static org.mockito.Mockito.verify;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.mockito.Mockito;
import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
import org.openhab.core.thing.Bridge;
@@ -25,6 +25,7 @@ import org.openhab.core.thing.Bridge;
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public class BrokerHandlerEx extends BrokerHandler {
final MqttBrokerConnectionEx e;
@@ -34,7 +35,7 @@ public class BrokerHandlerEx extends BrokerHandler {
}
@Override
protected @NonNull MqttBrokerConnection createBrokerConnection() throws IllegalArgumentException {
protected MqttBrokerConnection createBrokerConnection() throws IllegalArgumentException {
return e;
}

25
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/BrokerHandlerTest.java
View File

@@ -21,6 +21,7 @@ import static org.mockito.Mockito.*;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -48,15 +49,15 @@ import org.osgi.service.cm.ConfigurationException;
*/
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.WARN)
@NonNullByDefault
public class BrokerHandlerTest extends JavaTest {
private ScheduledExecutorService scheduler;
private @Mock ThingHandlerCallback callback;
private @Mock Bridge thing;
private @Mock @NonNullByDefault({}) ThingHandlerCallback callbackMock;
private @Mock @NonNullByDefault({}) Bridge thingMock;
private MqttBrokerConnectionEx connection;
private BrokerHandler handler;
private @NonNullByDefault({}) MqttBrokerConnectionEx connection;
private @NonNullByDefault({}) BrokerHandler handler;
private @NonNullByDefault({}) ScheduledExecutorService scheduler;
@BeforeEach
public void setUp() {
@@ -66,10 +67,10 @@ public class BrokerHandlerTest extends JavaTest {
connection.setConnectionCallback(connection);
Configuration config = new Configuration();
when(thing.getConfiguration()).thenReturn(config);
when(thingMock.getConfiguration()).thenReturn(config);
handler = spy(new BrokerHandlerEx(thing, connection));
handler.setCallback(callback);
handler = spy(new BrokerHandlerEx(thingMock, connection));
handler.setCallback(callbackMock);
}
@AfterEach
@@ -80,7 +81,7 @@ public class BrokerHandlerTest extends JavaTest {
@Test
public void handlerInitWithoutUrl() throws IllegalArgumentException {
// Assume it is a real handler and not a mock as defined above
handler = new BrokerHandler(thing);
handler = new BrokerHandler(thingMock);
assertThrows(IllegalArgumentException.class, this::initializeHandlerWaitForTimeout);
}
@@ -89,7 +90,7 @@ public class BrokerHandlerTest extends JavaTest {
Configuration config = new Configuration();
config.put("host", "10.10.0.10");
config.put("port", 80);
when(thing.getConfiguration()).thenReturn(config);
when(thingMock.getConfiguration()).thenReturn(config);
handler.initialize();
verify(handler).createBrokerConnection();
}
@@ -99,7 +100,7 @@ public class BrokerHandlerTest extends JavaTest {
assertThat(initializeHandlerWaitForTimeout(), is(true));
ArgumentCaptor<ThingStatusInfo> statusInfoCaptor = ArgumentCaptor.forClass(ThingStatusInfo.class);
verify(callback, atLeast(3)).statusUpdated(eq(thing), statusInfoCaptor.capture());
verify(callbackMock, atLeast(3)).statusUpdated(eq(thingMock), statusInfoCaptor.capture());
assertThat(statusInfoCaptor.getValue().getStatus(), is(ThingStatus.ONLINE));
}

3
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/MqttBrokerConnectionEx.java
View File

@@ -16,7 +16,6 @@ import static org.mockito.Mockito.spy;
import java.util.Map;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
@@ -65,7 +64,7 @@ public class MqttBrokerConnectionEx extends MqttBrokerConnection {
}
@Override
public @NonNull MqttConnectionState connectionState() {
public MqttConnectionState connectionState() {
return connectionStateOverwrite;
}
}

5
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/MqttConnectionObserverEx.java
View File

@@ -14,7 +14,7 @@ package org.openhab.binding.mqtt.handler;
import java.util.concurrent.Semaphore;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.io.transport.mqtt.MqttConnectionObserver;
import org.openhab.core.io.transport.mqtt.MqttConnectionState;
@@ -24,6 +24,7 @@ import org.openhab.core.io.transport.mqtt.MqttConnectionState;
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public class MqttConnectionObserverEx implements MqttConnectionObserver {
public int counter = 0;
public Semaphore semaphore = new Semaphore(1);
@@ -33,7 +34,7 @@ public class MqttConnectionObserverEx implements MqttConnectionObserver {
}
@Override
public void connectionStateChanged(@NonNull MqttConnectionState state, @Nullable Throwable error) {
public void connectionStateChanged(MqttConnectionState state, @Nullable Throwable error) {
// First we expect a CONNECTING state and then a DISCONNECTED state change
if (counter == 0 && state == MqttConnectionState.CONNECTING) {
counter = 1;

55
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/internal/MQTTTopicDiscoveryServiceTest.java
View File

@@ -20,6 +20,7 @@ import static org.mockito.Mockito.*;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -44,29 +45,23 @@ import org.openhab.core.thing.binding.ThingHandlerCallback;
*/
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.WARN)
@NonNullByDefault
public class MQTTTopicDiscoveryServiceTest {
private ScheduledExecutorService scheduler;
private MqttBrokerHandlerFactory subject;
private @Mock @NonNullByDefault({}) Bridge thingMock;
private @Mock @NonNullByDefault({}) ThingHandlerCallback callbackMock;
private @Mock @NonNullByDefault({}) MQTTTopicDiscoveryParticipant listenerMock;
@Mock
private Bridge thing;
@Mock
private ThingHandlerCallback callback;
@Mock
MQTTTopicDiscoveryParticipant listener;
private MqttBrokerConnectionEx connection;
private BrokerHandler handler;
private @NonNullByDefault({}) MqttBrokerConnectionEx connection;
private @NonNullByDefault({}) BrokerHandler handler;
private @NonNullByDefault({}) ScheduledExecutorService scheduler;
private @NonNullByDefault({}) MqttBrokerHandlerFactory subject;
@BeforeEach
public void setUp() {
scheduler = new ScheduledThreadPoolExecutor(1);
when(thing.getUID()).thenReturn(MqttThingID.getThingUID("10.10.0.10", 80));
when(thingMock.getUID()).thenReturn(MqttThingID.getThingUID("10.10.0.10", 80));
connection = spy(new MqttBrokerConnectionEx("10.10.0.10", 80, false, "BrokerHandlerTest"));
connection.setTimeoutExecutor(scheduler, 10);
connection.setConnectionCallback(connection);
@@ -74,10 +69,10 @@ public class MQTTTopicDiscoveryServiceTest {
Configuration config = new Configuration();
config.put("host", "10.10.0.10");
config.put("port", 80);
when(thing.getConfiguration()).thenReturn(config);
when(thingMock.getConfiguration()).thenReturn(config);
handler = spy(new BrokerHandlerEx(thing, connection));
handler.setCallback(callback);
handler = spy(new BrokerHandlerEx(thingMock, connection));
handler.setCallback(callbackMock);
subject = new MqttBrokerHandlerFactory();
}
@@ -92,13 +87,13 @@ public class MQTTTopicDiscoveryServiceTest {
handler.initialize();
BrokerHandlerEx.verifyCreateBrokerConnection(handler, 1);
subject.subscribe(listener, "topic");
subject.subscribe(listenerMock, "topic");
subject.createdHandler(handler);
assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
assertThat(subject.discoveryTopics.get("topic"), hasItem(listenerMock));
// Simulate receiving
final byte[] bytes = "TEST".getBytes();
connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
verify(listenerMock).receivedMessage(eq(thingMock.getUID()), eq(connection), eq("topic"), eq(bytes));
}
@Test
@@ -107,20 +102,20 @@ public class MQTTTopicDiscoveryServiceTest {
BrokerHandlerEx.verifyCreateBrokerConnection(handler, 1);
subject.createdHandler(handler);
subject.subscribe(listener, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
subject.subscribe(listenerMock, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listenerMock));
// Simulate receiving
final byte[] bytes = "TEST".getBytes();
connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
verify(listenerMock).receivedMessage(eq(thingMock.getUID()), eq(connection), eq("topic"), eq(bytes));
}
@Test
public void handlerInitializeAfterSubscribe() {
subject.createdHandler(handler);
subject.subscribe(listener, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
subject.subscribe(listenerMock, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listenerMock));
// Init handler -> create connection
handler.initialize();
@@ -129,7 +124,7 @@ public class MQTTTopicDiscoveryServiceTest {
// Simulate receiving
final byte[] bytes = "TEST".getBytes();
connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
verify(listenerMock).receivedMessage(eq(thingMock.getUID()), eq(connection), eq("topic"), eq(bytes));
}
@Test
@@ -138,12 +133,12 @@ public class MQTTTopicDiscoveryServiceTest {
BrokerHandlerEx.verifyCreateBrokerConnection(handler, 1);
subject.createdHandler(handler);
subject.subscribe(listener, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
subject.subscribe(listenerMock, "topic");
assertThat(subject.discoveryTopics.get("topic"), hasItem(listenerMock));
// Simulate receiving
final byte[] bytes = "".getBytes();
connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
verify(listener).topicVanished(eq(thing.getUID()), eq(connection), eq("topic"));
verify(listenerMock).topicVanished(eq(thingMock.getUID()), eq(connection), eq("topic"));
}
}

14
bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/internal/ssl/PinningSSLContextProviderTest.java
View File

@@ -25,7 +25,7 @@ import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.openhab.core.util.HexUtils;
@@ -34,6 +34,7 @@ import org.openhab.core.util.HexUtils;
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public class PinningSSLContextProviderTest {
@Test
@@ -61,7 +62,7 @@ public class PinningSSLContextProviderTest {
@Test
public void certPinCallsX509CertificateGetEncoded() throws NoSuchAlgorithmException, CertificateException {
PinTrustManager pinTrustManager = new PinTrustManager();
pinTrustManager.addPinning(Pin.LearningPin(PinType.CERTIFICATE_TYPE));
pinTrustManager.addPinning(Pin.learningPin(PinType.CERTIFICATE_TYPE));
// Mock a certificate
X509Certificate certificate = mock(X509Certificate.class);
@@ -77,7 +78,7 @@ public class PinningSSLContextProviderTest {
@Test
public void pubKeyPinCallsX509CertificateGetPublicKey() throws NoSuchAlgorithmException, CertificateException {
PinTrustManager pinTrustManager = new PinTrustManager();
pinTrustManager.addPinning(Pin.LearningPin(PinType.PUBLIC_KEY_TYPE));
pinTrustManager.addPinning(Pin.learningPin(PinType.PUBLIC_KEY_TYPE));
// Mock a certificate
PublicKey publicKey = mock(PublicKey.class);
@@ -102,8 +103,7 @@ public class PinningSSLContextProviderTest {
}
@Override
@NonNull
PinMessageDigest getMessageDigestForSigAlg(@NonNull String sigAlg) throws CertificateException {
PinMessageDigest getMessageDigestForSigAlg(String sigAlg) throws CertificateException {
return pinMessageDigest;
}
}
@@ -116,7 +116,7 @@ public class PinningSSLContextProviderTest {
byte[] digestOfTestCert = pinMessageDigest.digest(testCert);
// Add a certificate pin in learning mode to a trust manager
Pin pin = Pin.LearningPin(PinType.CERTIFICATE_TYPE);
Pin pin = Pin.learningPin(PinType.CERTIFICATE_TYPE);
pinTrustManager.addPinning(pin);
assertThat(pinTrustManager.pins.size(), is(1));
@@ -150,7 +150,7 @@ public class PinningSSLContextProviderTest {
byte[] digestOfTestCert = pinMessageDigest.digest(testCert);
// Add a certificate pin in checking mode to a trust manager
Pin pin = Pin.CheckingPin(PinType.CERTIFICATE_TYPE, pinMessageDigest, digestOfTestCert);
Pin pin = Pin.checkingPin(PinType.CERTIFICATE_TYPE, pinMessageDigest, digestOfTestCert);
pinTrustManager.addPinning(pin);
assertThat(pinTrustManager.pins.size(), is(1));