From 9c36f65c0ab4635bbef58da1a85ec00da5eba7ab Mon Sep 17 00:00:00 2001 From: Nicola Timeus Date: Wed, 5 Apr 2023 10:57:33 +0200 Subject: [PATCH 1/2] fix: Fixed NetworkConfigurationService returning passwords as strings Signed-off-by: Nicola Timeus --- .../NetworkConfigurationServiceImpl.java | 6 ++-- .../META-INF/MANIFEST.MF | 2 ++ .../NetworkConfigurationServiceCommon.java | 31 +++++++++++++++++++ .../NMConfigurationServiceImpl.java | 11 +++---- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/kura/org.eclipse.kura.net.admin/src/main/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImpl.java b/kura/org.eclipse.kura.net.admin/src/main/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImpl.java index f4309921ceb..2bf80524aa1 100644 --- a/kura/org.eclipse.kura.net.admin/src/main/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImpl.java +++ b/kura/org.eclipse.kura.net.admin/src/main/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImpl.java @@ -33,7 +33,6 @@ import org.eclipse.kura.configuration.ConfigurationService; import org.eclipse.kura.configuration.Password; import org.eclipse.kura.configuration.SelfConfiguringComponent; -import org.eclipse.kura.core.configuration.ComponentConfigurationImpl; import org.eclipse.kura.core.net.EthernetInterfaceConfigImpl; import org.eclipse.kura.core.net.IpConfigurationInterpreter; import org.eclipse.kura.core.net.LoopbackInterfaceConfigImpl; @@ -385,11 +384,10 @@ private void decryptPasswordProperties(Map modifiedProps) throws } @Override + @SuppressWarnings("restriction") public synchronized ComponentConfiguration getConfiguration() throws KuraException { - return new ComponentConfigurationImpl(PID, - NetworkConfigurationServiceCommon.getDefinition(this.properties, Optional.of(this.usbNetDevices)), - this.properties); + return NetworkConfigurationServiceCommon.getConfiguration(PID, properties, Optional.of(this.usbNetDevices)); } @Override diff --git a/kura/org.eclipse.kura.net.configuration/META-INF/MANIFEST.MF b/kura/org.eclipse.kura.net.configuration/META-INF/MANIFEST.MF index 8a6dfef4a8b..60196f88d9e 100644 --- a/kura/org.eclipse.kura.net.configuration/META-INF/MANIFEST.MF +++ b/kura/org.eclipse.kura.net.configuration/META-INF/MANIFEST.MF @@ -7,6 +7,8 @@ Export-Package: org.eclipse.kura.net.configuration;version="1.0.0";x-internal:=t Bundle-Vendor: Eclipse Kura Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))" Import-Package: org.eclipse.kura;version="[1.0,2.0)", + org.eclipse.kura.configuration;version="[1.2,2.0)", + org.eclipse.kura.configuration.metatype;version="[1.1,2.0)", org.eclipse.kura.core.configuration;version="[2.0,3.0)", org.eclipse.kura.core.configuration.metatype;version="[1.0,2.0)", org.eclipse.kura.net;version="[2.4,2.5)", diff --git a/kura/org.eclipse.kura.net.configuration/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java b/kura/org.eclipse.kura.net.configuration/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java index b1caaae1a2f..ab8d7671fb5 100644 --- a/kura/org.eclipse.kura.net.configuration/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java +++ b/kura/org.eclipse.kura.net.configuration/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java @@ -12,6 +12,7 @@ *******************************************************************************/ package org.eclipse.kura.net.configuration; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -22,6 +23,11 @@ import org.eclipse.kura.KuraErrorCode; import org.eclipse.kura.KuraException; +import org.eclipse.kura.configuration.ComponentConfiguration; +import org.eclipse.kura.configuration.Password; +import org.eclipse.kura.configuration.metatype.AD; +import org.eclipse.kura.configuration.metatype.Scalar; +import org.eclipse.kura.core.configuration.ComponentConfigurationImpl; import org.eclipse.kura.core.configuration.metatype.ObjectFactory; import org.eclipse.kura.core.configuration.metatype.Tad; import org.eclipse.kura.core.configuration.metatype.Tocd; @@ -112,6 +118,31 @@ public static Tocd getDefinition(Map properties, Optional properties, + final Optional> usbNetDevices) throws KuraException { + final Tocd definition = getDefinition(properties, usbNetDevices); + + return new ComponentConfigurationImpl(pid, definition, convertStringsToPasswords(definition, properties)); + } + + private static Map convertStringsToPasswords(final Tocd ocd, final Map properties) { + Optional> result = Optional.empty(); + + for (final AD ad : ocd.getAD()) { + final String key = ad.getId(); + final Object value = result.orElse(properties).get(ad.getId()); + + if (ad.getType() == Scalar.PASSWORD && value instanceof String) { + if (!result.isPresent()) { + result = Optional.of(new HashMap<>(properties)); + } + result.get().put(key, new Password(((String) value).toCharArray())); + } + } + + return result.orElse(properties); + } + private static void getModemDefinition(Tocd tocd, String ifaceName) { tocd.addAD(buildAttributeDefinition( String.format(PREFIX + "%s.config.enabled", ifaceName), diff --git a/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java b/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java index e12d6c089a8..3298becb851 100644 --- a/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java +++ b/kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java @@ -27,7 +27,6 @@ import org.eclipse.kura.configuration.ComponentConfiguration; import org.eclipse.kura.configuration.Password; import org.eclipse.kura.configuration.SelfConfiguringComponent; -import org.eclipse.kura.core.configuration.ComponentConfigurationImpl; import org.eclipse.kura.crypto.CryptoService; import org.eclipse.kura.executor.CommandExecutorService; import org.eclipse.kura.linux.net.util.LinuxNetworkUtil; @@ -156,7 +155,7 @@ public void activate(ComponentContext componentContext, Map prop this.networkProperties = new NetworkProperties(discardModifiedNetworkInterfaces(new HashMap<>(properties))); update(this.networkProperties.getProperties()); } - + logger.info("Activate NetworkConfigurationService... Done."); } @@ -271,12 +270,12 @@ private void decryptAndConvertPasswordProperties(Map modifiedPro } @Override + @SuppressWarnings("restriction") public synchronized ComponentConfiguration getConfiguration() throws KuraException { - return new ComponentConfigurationImpl( - NetworkConfigurationServiceCommon.PID, NetworkConfigurationServiceCommon - .getDefinition(this.networkProperties.getProperties(), Optional.empty()), - this.networkProperties.getProperties()); + return NetworkConfigurationServiceCommon.getConfiguration(NetworkConfigurationServiceCommon.PID, + this.networkProperties.getProperties(), Optional.empty()); + } private void mergeNetworkConfigurationProperties(final Map source, final Map dest) { From 648ff9bbcced8a99f660c246d2d8ae66f2acb78f Mon Sep 17 00:00:00 2001 From: Nicola Timeus Date: Wed, 5 Apr 2023 15:14:49 +0200 Subject: [PATCH 2/2] Added test for string wrapping as password Signed-off-by: Nicola Timeus --- ...NetworkConfigurationServiceCommonTest.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/kura/test/org.eclipse.kura.net.configuration.test/src/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java b/kura/test/org.eclipse.kura.net.configuration.test/src/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java index a3b09478bf8..5776df0f589 100644 --- a/kura/test/org.eclipse.kura.net.configuration.test/src/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java +++ b/kura/test/org.eclipse.kura.net.configuration.test/src/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java @@ -12,6 +12,7 @@ ******************************************************************************/ package org.eclipse.kura.net.configuration; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -23,6 +24,7 @@ import java.util.Optional; import org.eclipse.kura.KuraException; +import org.eclipse.kura.configuration.Password; import org.eclipse.kura.configuration.metatype.AD; import org.eclipse.kura.configuration.metatype.Scalar; import org.eclipse.kura.core.configuration.metatype.Tocd; @@ -33,6 +35,7 @@ public class NetworkConfigurationServiceCommonTest { private final Map properties = new HashMap<>(); private Tocd tocd; private List ads; + private Map returnedProperties = new HashMap<>(); @Test public void componentDefinitionShouldHaveBasicPropertiesTest() throws KuraException { @@ -76,6 +79,54 @@ public void pppNumberShouldBeAnInteger() throws KuraException { thenPppNumIsInteger(); } + @Test + public void shouldWrapStringPasswords() throws KuraException { + givenFullProperties(); + givenConfigurationProperty("net.interface.1-4.config.password", "foo"); + givenConfigurationProperty("net.interface.wlp1s0.config.wifi.master.passphrase", "bar"); + givenConfigurationProperty("net.interface.wlp1s0.config.wifi.infra.passphrase", "baz"); + + whenConfigurationPropertiesAreRetrieved(); + + thenReturnedPropertyEqualsPassword("net.interface.1-4.config.password", + new Password(new char[] { 'f', 'o', 'o' })); + thenReturnedPropertyEqualsPassword("net.interface.wlp1s0.config.wifi.master.passphrase", + new Password(new char[] { 'b', 'a', 'r' })); + thenReturnedPropertyEqualsPassword("net.interface.wlp1s0.config.wifi.infra.passphrase", + new Password(new char[] { 'b', 'a', 'z' })); + + } + + @Test + public void shouldNotChangeWrappedPasswords() throws KuraException { + givenFullProperties(); + givenConfigurationProperty("net.interface.1-4.config.password", new Password(new char[] { 'f', 'o', 'o' })); + givenConfigurationProperty("net.interface.wlp1s0.config.wifi.master.passphrase", + new Password(new char[] { 'b', 'a', 'r' })); + givenConfigurationProperty("net.interface.wlp1s0.config.wifi.infra.passphrase", + new Password(new char[] { 'b', 'a', 'z' })); + + whenConfigurationPropertiesAreRetrieved(); + + thenReturnedPropertyEqualsPassword("net.interface.1-4.config.password", + new Password(new char[] { 'f', 'o', 'o' })); + thenReturnedPropertyEqualsPassword("net.interface.wlp1s0.config.wifi.master.passphrase", + new Password(new char[] { 'b', 'a', 'r' })); + thenReturnedPropertyEqualsPassword("net.interface.wlp1s0.config.wifi.infra.passphrase", + new Password(new char[] { 'b', 'a', 'z' })); + + } + + @Test + public void shouldNotChangeOtherProperties() throws KuraException { + givenFullProperties(); + whenConfigurationPropertiesAreRetrieved(); + + thenReturnedPropertyEquals("net.interface.lo.config.ip4.status", "netIPv4StatusEnabledLAN"); + thenReturnedPropertyEquals("net.interface.1-4.config.resetTimeout", 5); + thenReturnedPropertyEquals("net.interface.1-4.config.ip4.prefix", -1); + } + private void givenPropertiesWithoutInterfaces() { this.properties.clear(); this.properties.put("net.interfaces", ""); @@ -189,12 +240,21 @@ private void givenFullProperties() { this.properties.put("net.interface.1-4.config.pdpType", "IP"); } + private void givenConfigurationProperty(final String key, final Object value) { + this.properties.put(key, value); + } + private void whenTocdIsRetrieved() throws KuraException { this.ads = null; this.tocd = NetworkConfigurationServiceCommon.getDefinition(this.properties, Optional.empty()); this.ads = this.tocd.getAD(); } + private void whenConfigurationPropertiesAreRetrieved() throws KuraException { + this.returnedProperties = NetworkConfigurationServiceCommon + .getConfiguration("foo", this.properties, Optional.empty()).getConfigurationProperties(); + } + private void thenComponentDefinitionHasBasicProperties() { assertEquals(NetworkConfigurationServiceCommon.PID, this.tocd.getId()); assertEquals("NetworkConfigurationService", this.tocd.getName()); @@ -208,6 +268,16 @@ private void thenComponentDefinitionHasCorrectNumberOfResources() { assertEquals(110, this.ads.size()); } + private void thenReturnedPropertyEquals(final String key, final Object value) { + assertEquals(value, this.returnedProperties.get(key)); + } + + private void thenReturnedPropertyEqualsPassword(final String key, final Password value) { + assertEquals(Password.class, + Optional.ofNullable(this.returnedProperties.get(key)).map(Object::getClass).orElse(null)); + assertArrayEquals(value.getPassword(), ((Password) this.returnedProperties.get(key)).getPassword()); + } + private void thenComponentDefinitionHasCorrectProperties() { int adsConfigured = 0; for (AD ad : this.ads) {