Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fixed NetworkConfigurationService returning passwords as strings #4532

Merged
merged 2 commits into from
Apr 5, 2023
Merged

fix: Fixed NetworkConfigurationService returning passwords as strings #4532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c36f65
fix: Fixed NetworkConfigurationService returning passwords as strings
nicolatimeus Apr 5, 2023
648ff9b
Added test for string wrapping as password
nicolatimeus Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions ...a.net.admin/src/main/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -385,11 +384,10 @@ private void decryptPasswordProperties(Map<String, Object> 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
Expand Down
2 changes: 2 additions & 0 deletions kura/org.eclipse.kura.net.configuration/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
31 changes: 31 additions & 0 deletions ...n/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -112,6 +118,31 @@ public static Tocd getDefinition(Map<String, Object> properties, Optional<List<U
return tocd;
}

public static ComponentConfiguration getConfiguration(final String pid, final Map<String, Object> properties,
final Optional<List<UsbNetDevice>> usbNetDevices) throws KuraException {
final Tocd definition = getDefinition(properties, usbNetDevices);

return new ComponentConfigurationImpl(pid, definition, convertStringsToPasswords(definition, properties));
}

private static Map<String, Object> convertStringsToPasswords(final Tocd ocd, final Map<String, Object> properties) {
Optional<Map<String, Object>> 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),
Expand Down
11 changes: 5 additions & 6 deletions ...e.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -156,7 +155,7 @@ public void activate(ComponentContext componentContext, Map<String, Object> prop
this.networkProperties = new NetworkProperties(discardModifiedNetworkInterfaces(new HashMap<>(properties)));
update(this.networkProperties.getProperties());
}

logger.info("Activate NetworkConfigurationService... Done.");
}

Expand Down Expand Up @@ -271,12 +270,12 @@ private void decryptAndConvertPasswordProperties(Map<String, Object> 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<String, Object> source, final Map<String, Object> dest) {
Expand Down
70 changes: 70 additions & 0 deletions ...c/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -33,6 +35,7 @@ public class NetworkConfigurationServiceCommonTest {
private final Map<String, Object> properties = new HashMap<>();
private Tocd tocd;
private List<AD> ads;
private Map<String, Object> returnedProperties = new HashMap<>();

@Test
public void componentDefinitionShouldHaveBasicPropertiesTest() throws KuraException {
Expand Down Expand Up @@ -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", "");
Expand Down Expand Up @@ -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());
Expand All @@ -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) {
Expand Down