Skip to content

Commit

Permalink
fix(nm): 802.1x TLS Private Key encryption (#5099)
Browse files Browse the repository at this point in the history
* chore: refactor code to take encryption step into account

* draft: something almost working, need more work

* feat: first working version

* feat: use SHA-256 as a password

* feat: Base64 encoded SHA-256 because... why not?

* refactor: extract PEM conversion step

* fix: do not use deprecated method setPasssword, use setPassword instead

lolwut?

* refactor: remove unnecessary settings (private-key-password-flags)

* refactor: nicer exception handling

* chore: remove unnecessary comments

* refactor: move method away from higher level stuff

* style: fix broken comment

* test(nm): fix test private key generation (still WIP)

* test(nm): add thenResultingBuildAllMapContainsEncrypted for key comparison

* refactor(test.nm): fix remaining tests

* refactor: rename method

* refactor: extract convertToDer method

* refactor: more logic parameters placement

* refactor: make it more readable
  • Loading branch information
mattdibi authored Jan 16, 2024
1 parent 41cbbd0 commit 83b8591
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 36 deletions.
12 changes: 9 additions & 3 deletions kura/org.eclipse.kura.nm/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,21 @@ Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Import-Package: javax.xml.bind;version="2.3.3",
org.apache.commons.io;version="2.4.0",
org.apache.commons.lang3.tuple;version="3.12.0",
org.bouncycastle.asn1;version="1.77.0",
org.bouncycastle.crypto.prng;version="1.77.0",
org.bouncycastle.openssl;version="1.77.0",
org.bouncycastle.openssl.jcajce;version="1.77.0",
org.bouncycastle.operator;version="1.77.0",
org.bouncycastle.util.io.pem;version="1.77.0",
org.eclipse.kura;version="[1.0,2.0)",
org.eclipse.kura.configuration;version="[1.1,2.0)",
org.eclipse.kura.core.configuration;version="[2.0,3.0)",
org.eclipse.kura.core.keystore.util;version="[1.0,2.0)",
org.eclipse.kura.core.linux.util;version="[1.0,2.0)",
org.eclipse.kura.core.net;version="[1.0,2.0)",
org.eclipse.kura.core.net.modem;version="[1.0,2.0)",
org.eclipse.kura.core.net.util;version="[1.0,2.0)",
org.eclipse.kura.core.util;version="[1.0,2.0)",
org.eclipse.kura.core.keystore.util;version="[1.0,2.0)",
org.eclipse.kura.crypto;version="[1.1,2.0)",
org.eclipse.kura.executor;version="[1.0,2.0)",
org.eclipse.kura.internal.linux.net.dns;version="[1.0,2.0)",
Expand All @@ -33,11 +39,11 @@ Import-Package: javax.xml.bind;version="2.3.3",
org.eclipse.kura.net.status.ethernet;version="[1.0,2.0)",
org.eclipse.kura.net.status.loopback;version="[1.0,2.0)",
org.eclipse.kura.net.status.modem;version="[1.0,2.0)",
org.eclipse.kura.net.status.wifi;version="[1.0,2.0)",
org.eclipse.kura.net.status.vlan;version="[1.0,2.0)",
org.eclipse.kura.net.status.wifi;version="[1.0,2.0)",
org.eclipse.kura.net.wifi;version="[2.4,3.0]",
org.eclipse.kura.usb;version="[1.0,2.0)",
org.eclipse.kura.security.keystore;version="[1.0,2.0)",
org.eclipse.kura.usb;version="[1.0,2.0)",
org.osgi.framework;version="1.5.0",
org.osgi.service.component;version="1.2.0",
org.osgi.service.event;version="1.3.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,28 @@
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Optional;

import javax.xml.bind.DatatypeConverter;

import org.bouncycastle.openssl.PKCS8Generator;
import org.bouncycastle.openssl.jcajce.JcaPKCS8Generator;
import org.bouncycastle.openssl.jcajce.JceOpenSSLPKCS8EncryptorBuilder;
import org.bouncycastle.operator.OperatorCreationException;
import org.bouncycastle.operator.OutputEncryptor;
import org.bouncycastle.util.io.pem.PemGenerationException;
import org.eclipse.kura.configuration.Password;
import org.eclipse.kura.nm.Kura8021xEAP;
import org.eclipse.kura.nm.Kura8021xInnerAuth;
Expand Down Expand Up @@ -72,8 +82,6 @@ public class NMSettingsConverter {
private static final String KURA_PROPS_KEY_WIFI_SECURITY_TYPE = "net.interface.%s.config.wifi.%s.securityType";
private static final String KURA_PROPS_IPV4_MTU = "net.interface.%s.config.ip4.mtu";

private static final UInt32 NM_SECRET_FLAGS_NOT_REQUIRED = new UInt32(4);

private NMSettingsConverter() {
throw new IllegalStateException("Utility class");
}
Expand Down Expand Up @@ -189,22 +197,29 @@ private static void create8021xTls(NetworkProperties props, String deviceId, Map
String identity = props.get(String.class, "net.interface.%s.config.802-1x.identity", deviceId);
settings.put("identity", new Variant<>(identity));

Certificate clientCert = props.get(Certificate.class, "net.interface.%s.config.802-1x.client-cert-name",
deviceId);
try {
Certificate clientCert = props.get(Certificate.class, "net.interface.%s.config.802-1x.client-cert-name",
deviceId);
settings.put("client-cert", new Variant<>(clientCert.getEncoded()));
} catch (CertificateEncodingException e) {
logger.error("Unable to decode Client Certificate for interface \"{}\"", deviceId);
}

PrivateKey privateKey = props.get(PrivateKey.class, "net.interface.%s.config.802-1x.private-key-name",
deviceId);
if (privateKey.getEncoded() != null) {
settings.put("private-key", new Variant<>(convertToPem(privateKey.getEncoded())));
} else {
logger.error("Unable to decode Private Key for interface \"{}\"", deviceId);
try {
// The private key is encrypted using the SHA-256 of the private key itself as password
byte[] privateKeyPasswordBytes = MessageDigest.getInstance("SHA-256").digest(privateKey.getEncoded());
String privateKeyPassword = Base64.getEncoder().encodeToString(privateKeyPasswordBytes);
settings.put("private-key-password", new Variant<>(privateKeyPassword));

byte[] encryptedPrivateKey = convertToPem(encryptPrivateKey(privateKey, privateKeyPassword));
settings.put("private-key", new Variant<>(encryptedPrivateKey));
} catch (NoSuchAlgorithmException e) {
logger.error("Something went wrong while computing SHA-256 of private key, bailing out. Caused by: ", e);
} catch (OperatorCreationException | PemGenerationException e) {
logger.error("Something went wrong during private key encryption, bailing out. Caused by: ", e);
}
settings.put("private-key-password-flags", new Variant<>(NM_SECRET_FLAGS_NOT_REQUIRED));
}

private static void create8021xOptionalCaCertAndAnonIdentity(NetworkProperties props, String deviceId,
Expand Down Expand Up @@ -303,8 +318,7 @@ public static Map<String, Variant<?>> buildIpv6Settings(NetworkProperties props,
SemanticVersion nmVersion) {

// buildIpv6Settings doesn't support Unmanaged status. Therefore if ip6.status
// property is not set, it assumes
// it is disabled.
// property is not set, it assumes it is disabled.

Optional<KuraIpStatus> ip6OptStatus = KuraIpStatus
.fromString(props.getOpt(String.class, "net.interface.%s.config.ip6.status", deviceId));
Expand Down Expand Up @@ -777,10 +791,26 @@ private static String connectionTypeConvert(NMDeviceType deviceType) {
}
}

private static byte[] encryptPrivateKey(PrivateKey privateKey, String privateKeyPassword)
throws OperatorCreationException, PemGenerationException {
// Assumption: the private key is encoded in PKCS#8 DER format
if (privateKey.getEncoded() == null) {
throw new NoSuchElementException("Unable to decode Private Key");
}

JceOpenSSLPKCS8EncryptorBuilder encryptorBuilder = new JceOpenSSLPKCS8EncryptorBuilder(
PKCS8Generator.PBE_SHA1_3DES);
encryptorBuilder.setPassword(privateKeyPassword.toCharArray());
OutputEncryptor oe = encryptorBuilder.build();
JcaPKCS8Generator gen = new JcaPKCS8Generator(privateKey, oe);

return gen.generate().getContent();
}

private static byte[] convertToPem(byte[] derKey) {
String pem = "-----BEGIN PRIVATE KEY-----\n"
String pem = "-----BEGIN ENCRYPTED PRIVATE KEY-----\n"
+ DatatypeConverter.printBase64Binary(derKey).replaceAll("(.{64})", "$1\n")
+ "\n-----END PRIVATE KEY-----\n";
+ "\n-----END ENCRYPTED PRIVATE KEY-----\n";
return pem.getBytes(StandardCharsets.UTF_8);
}
}
5 changes: 4 additions & 1 deletion kura/test/org.eclipse.kura.nm.test/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ Bundle-Version: 5.5.0.qualifier
Bundle-Vendor: Eclipse Kura
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Bundle-ActivationPolicy: lazy
Import-Package: org.eclipse.kura.configuration.metatype;version="[1.1,2.0)",
Import-Package: org.bouncycastle.jce.provider;version="1.77.0",
org.bouncycastle.pkcs;version="1.77.0",
org.bouncycastle.pkcs.jcajce;version="1.77.0",
org.eclipse.kura.configuration.metatype;version="[1.1,2.0)",
org.eclipse.kura.core.linux.executor;version="1.0.0",
org.eclipse.kura.core.testutil,
org.eclipse.kura.executor;version="[1.0,2.0)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,20 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.KeyFactory;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -36,9 +43,14 @@
import java.util.Objects;
import java.util.Optional;

import javax.crypto.EncryptedPrivateKeyInfo;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;

import org.eclipse.kura.configuration.Password;
import org.eclipse.kura.nm.SemanticVersion;
import org.eclipse.kura.nm.NetworkProperties;
import org.eclipse.kura.nm.SemanticVersion;
import org.eclipse.kura.nm.enums.NMDeviceType;
import org.freedesktop.dbus.types.UInt32;
import org.freedesktop.dbus.types.Variant;
Expand All @@ -48,6 +60,8 @@

public class NMSettingsConverterTest {

private final static String PEM_PRIVATE_KEY = "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDFWXYxR1zfnzpeO1771SosgCRhzyANqqxH600iLajJww+o1QeKR5n08INKBBNRRW6bCJpPNA5XNLl9ucnu/Bl2CIZ/NeAyFHtau+8kYrkT5wp/g2FCKIPqNAOUik2N7rEPB6FPm0FTWjlBUz2qRIQ7Szdqbw6ZXgK2Zn15MPb+CLjum2biqv1YPxaFnrPhHO2APVSu+xYEB90byFgGWEfL8qY+BAycVmNxPzq4C3LRdJwvCUvsMhcnNhpN0ZHg0ujAFEeQLZXm3SXZGvAQat5IAZLHxIQbUSeJjt1H2yWwkxrNoSMpwOGyAiUTiPAKwpfT2ab1cJZXILWF1+QmNC3tAgMBAAECggEABCT7CHMqDiU9y9KANl1HIfwc1PWk6OSQykndKtLmOvdUx+kcVTdoJoLpfT6l7dawLl/Xj3ILePLXP3ST6jjRVYpl+l9opPjO09kV5feCQ7kNP+ovknzYzkC/EhSsoEbAWqGbjET2Gll+MAIsdhbVAi5mhA3Nb4caNgHIyxsTMXHidl/BwaxkLyv4RWOiPxQPA1XFCTGX9b3KcIDte8hRvEuK7mD6V6VKMm0ArxJgJXtOQ/dhH4Jhra/RH3Y3NjszgP2OW18z71/Yeud18ykNNgzrX2EkXAYXulfa9O4Yfi/k3TttP3QxItbRD+VetZCvQj3jHaG1Ly3dJRGhC2xaOQKBgQD+R7g8FHxjihpJt3bSZNClftRl3bY/4jRV7MBXNC7XA9zj3zNp5R6JCV9PAI5CyY1lhOq+pHfDggufcQZlSC6h4n6b0rj+b2vxbNBy8efQUDtgQw7QfunPGYs+OHPpNK7JGYUezbIw0PV7ahxiD6ncG6DibnNInEeyBC5AJ803LwKBgQDGryr1wOE15Q/lH+XPPf+cclDC/vKpp3Fm4btzSWrOyQWh+yxKGd5dmeRk0h2cDp92jOAHNjLVA0ejvcQUIwew+DYRnXJe/YDvOijFWW+LDRdm/oPcqtjzrfFd1ROQRSeEB0R/BF4m1EDcLligs3N3pWiBEWs/HYdJuIRhK2PlowKBgDh4AOgGvKD2WGQqhA6xKMy3379Hf2OsfmbejtBO3GAPkYxhUu+fXCqelDXdL7qRO/9hhygTKi2WwbIEzaDMaN62h9te7opCgDw7KAd+xTYzuxvjiHSw2oeNaqjErKkLdA1gx3lRwNKqdPmVVPxJ8jTZRd9DHALyAdH8r7C7pg0tAoGBAJIc1/cK9ZRw9BOINbUG3yfqWcJNQ5/IZ/lFIFlUMJwJ8X6B/Lwx8fnb5r7OVsAhcNv6Ffa3wQIt+01LjRtR96IJp5mktCtvOpazqrAXaZRU+FTh748khZAO52YeANkkQj8yKQlP6P2dMmW6H6tuzQe8OPJSIRC1YnywmYnsIvcJAoGAHLoik8+9ej4zcFnO2xTve6cvEym/sISnlE5GLC2sYomG6cxDnMq4DWL3tVBbBnOCkany+p0oWuhSGsbnEWVDHvA3Wo3uuY7NdL3iTPhIHbZ+0AkgjHT99LYZHr4lhTJP8XU6UKT15aDJyljvRFuWrcHKrQ18VSiIwOQoQzbAVAc=";

private Map<String, Variant<?>> resultMap;

private Map<String, Map<String, Variant<?>>> internalComparatorAllSettingsMap = new HashMap<>();
Expand Down Expand Up @@ -650,8 +664,7 @@ public void build8021xSettingsShouldWorkWithTls() {
buildMockedCertificateWithCert("binary ca cert"));
givenMapWith("net.interface.wlan0.config.802-1x.client-cert-name",
buildMockedCertificateWithCert("binary client cert"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name",
buildMockedPrivateKeyWithKey("binary private key"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name", buildMockPrivateKeyWith(PEM_PRIVATE_KEY));
givenNetworkPropsCreatedWithTheMap(this.internetNetworkPropertiesInstanciationMap);

whenBuild8021xSettingsIsRunWith(this.networkProperties, "wlan0");
Expand All @@ -663,10 +676,10 @@ public void build8021xSettingsShouldWorkWithTls() {
thenResultingMapContains("identity", "[email protected]");
thenResultingMapContainsBytes("ca-cert", "binary ca cert");
thenResultingMapContainsBytes("client-cert", "binary client cert");
thenResultingMapContainsBytes("private-key",
"-----BEGIN PRIVATE KEY-----\nYmluYXJ5IHByaXZhdGUga2V5\n-----END PRIVATE KEY-----\n");
thenResultingMapContains("private-key-password", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=");
thenResultingMapContainsEncryptedPrivateKey("private-key", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=",
PEM_PRIVATE_KEY);

thenResultingMapNotContains("private-key-password");
thenResultingMapNotContains("ca-cert-password");
thenResultingMapNotContains("client-cert-password");
}
Expand Down Expand Up @@ -713,8 +726,7 @@ public void build8021xSettingsShouldWorkWithTlsWithNullCACert() {
givenMapWith("net.interface.wlan0.config.802-1x.ca-cert-name", null);
givenMapWith("net.interface.wlan0.config.802-1x.client-cert-name",
buildMockedCertificateWithCert("binary client cert"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name",
buildMockedPrivateKeyWithKey("binary private key"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name", buildMockPrivateKeyWith(PEM_PRIVATE_KEY));
givenNetworkPropsCreatedWithTheMap(this.internetNetworkPropertiesInstanciationMap);

whenBuild8021xSettingsIsRunWith(this.networkProperties, "wlan0");
Expand All @@ -724,12 +736,12 @@ public void build8021xSettingsShouldWorkWithTlsWithNullCACert() {
thenResultingMapContainsArray("eap", new Variant<>(new String[] { "tls" }).getValue());
thenResultingMapContains("identity", "[email protected]");
thenResultingMapContainsBytes("client-cert", "binary client cert");
thenResultingMapContainsBytes("private-key",
"-----BEGIN PRIVATE KEY-----\nYmluYXJ5IHByaXZhdGUga2V5\n-----END PRIVATE KEY-----\n");
thenResultingMapContains("private-key-password", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=");
thenResultingMapContainsEncryptedPrivateKey("private-key", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=",
PEM_PRIVATE_KEY);

thenResultingMapNotContains("phase2-auth");
thenResultingMapNotContains("ca-cert");
thenResultingMapNotContains("private-key-password");
thenResultingMapNotContains("ca-cert-password");
thenResultingMapNotContains("client-cert-password");
}
Expand All @@ -743,8 +755,7 @@ public void build8021xSettingsShouldWorkWithTlsWithWrongTypeCACert() {
new Password("When I grow up I want to be a certificate"));
givenMapWith("net.interface.wlan0.config.802-1x.client-cert-name",
buildMockedCertificateWithCert("binary client cert"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name",
buildMockedPrivateKeyWithKey("binary private key"));
givenMapWith("net.interface.wlan0.config.802-1x.private-key-name", buildMockPrivateKeyWith(PEM_PRIVATE_KEY));
givenNetworkPropsCreatedWithTheMap(this.internetNetworkPropertiesInstanciationMap);

whenBuild8021xSettingsIsRunWith(this.networkProperties, "wlan0");
Expand All @@ -754,12 +765,12 @@ public void build8021xSettingsShouldWorkWithTlsWithWrongTypeCACert() {
thenResultingMapContainsArray("eap", new Variant<>(new String[] { "tls" }).getValue());
thenResultingMapContains("identity", "[email protected]");
thenResultingMapContainsBytes("client-cert", "binary client cert");
thenResultingMapContainsBytes("private-key",
"-----BEGIN PRIVATE KEY-----\nYmluYXJ5IHByaXZhdGUga2V5\n-----END PRIVATE KEY-----\n");
thenResultingMapContains("private-key-password", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=");
thenResultingMapContainsEncryptedPrivateKey("private-key", "sOPM6ph9zBENU0rrOiZhIAk8wn26W8qj0r+DBVu6Zbk=",
PEM_PRIVATE_KEY);

thenResultingMapNotContains("phase2-auth");
thenResultingMapNotContains("ca-cert");
thenResultingMapNotContains("private-key-password");
thenResultingMapNotContains("ca-cert-password");
thenResultingMapNotContains("client-cert-password");
}
Expand Down Expand Up @@ -3155,7 +3166,7 @@ private <E extends Exception> void thenExceptionOccurred(Class<E> expectedExcept
assertEquals(expectedException.getName(), this.occurredException.getClass().getName());
}

public void thenNoExceptionOccurred() {
private void thenNoExceptionOccurred() {
String errorMessage = "Empty message";
if (Objects.nonNull(this.occurredException)) {
StringWriter sw = new StringWriter();
Expand All @@ -3168,6 +3179,35 @@ public void thenNoExceptionOccurred() {
assertNull(errorMessage, this.occurredException);
}

private void thenResultingMapContainsEncryptedPrivateKey(String key, String expectedPrivateKeyPassword,
String expectedPemPrivateKeyContent) {
byte[] encryptedKey = (byte[]) this.resultMap.get(key).getValue();
byte[] decryptedKey = decryptKey(convertToDer(encryptedKey), expectedPrivateKeyPassword);
assertEquals(expectedPemPrivateKeyContent, Base64.getEncoder().encodeToString(decryptedKey));
}

private byte[] decryptKey(byte[] encryptedKey, String expectedPrivateKeyPassword) {
PBEKeySpec pbeSpec = new PBEKeySpec(expectedPrivateKeyPassword.toCharArray());
try {
EncryptedPrivateKeyInfo privateKeyInfo = new EncryptedPrivateKeyInfo(encryptedKey);
SecretKeyFactory secretKeyFact = SecretKeyFactory.getInstance(privateKeyInfo.getAlgName());
SecretKey secret = secretKeyFact.generateSecret(pbeSpec);
PKCS8EncodedKeySpec keySpec = privateKeyInfo.getKeySpec(secret);
KeyFactory keyFact = KeyFactory.getInstance("RSA");
return keyFact.generatePrivate(keySpec).getEncoded();
} catch (IOException | NoSuchAlgorithmException | InvalidKeyException | InvalidKeySpecException e) {
throw new RuntimeException(e);
}
}

private byte[] convertToDer(byte[] privateKeyPem) {
String privateKeyString = new String(privateKeyPem, StandardCharsets.UTF_8);
String privateKeyStringContent = privateKeyString.replace("\n", "")
.replace("-----BEGIN ENCRYPTED PRIVATE KEY-----", "")
.replace("-----END ENCRYPTED PRIVATE KEY-----", "");
return Base64.getDecoder().decode(privateKeyStringContent.getBytes());
}

/*
* Helper Methods
*/
Expand Down Expand Up @@ -3196,9 +3236,11 @@ public Certificate buildMockedCertificateWithCert(String certBytes) {
return cert;
}

public PrivateKey buildMockedPrivateKeyWithKey(String keyBytes) {
public PrivateKey buildMockPrivateKeyWith(String privateKeyPEM) {
byte[] decoded = Base64.getDecoder().decode(privateKeyPEM);

PrivateKey key = mock(PrivateKey.class);
when(key.getEncoded()).thenReturn(keyBytes.getBytes());
when(key.getEncoded()).thenReturn(decoded);

return key;
}
Expand Down

0 comments on commit 83b8591

Please sign in to comment.