diff --git a/aaa-cert/src/main/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtils.java b/aaa-cert/src/main/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtils.java index 58f04682d..a6d610b9b 100644 --- a/aaa-cert/src/main/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtils.java +++ b/aaa-cert/src/main/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtils.java @@ -7,6 +7,9 @@ */ package org.opendaylight.aaa.cert.impl; +import java.nio.charset.Charset; +import java.security.GeneralSecurityException; +import java.util.Base64; import java.util.List; import org.opendaylight.aaa.encrypt.AAAEncryptionService; import org.opendaylight.mdsal.binding.api.DataBroker; @@ -29,7 +32,6 @@ * KeyStoresDataUtils manage the SslData operations add, delete and update. * * @author mserngawy - * */ public class KeyStoresDataUtils { @@ -62,9 +64,15 @@ public static OdlKeystore updateOdlKeystore(final OdlKeystore baseOdlKeyStore, f public SslData addSslData(final DataBroker dataBroker, final String bundleName, final OdlKeystore odlKeystore, final TrustKeystore trustKeystore, final List cipherSuites, final String tlsProtocols) { final SslDataKey sslDataKey = new SslDataKey(bundleName); - final SslData sslData = new SslDataBuilder().withKey(sslDataKey).setOdlKeystore(encryptOdlKeyStore(odlKeystore)) - .setTrustKeystore(encryptTrustKeystore(trustKeystore)).setCipherSuites(cipherSuites) - .setTlsProtocols(tlsProtocols).build(); + final SslData sslData; + try { + sslData = new SslDataBuilder().withKey(sslDataKey).setOdlKeystore(encryptOdlKeyStore(odlKeystore)) + .setTrustKeystore(encryptTrustKeystore(trustKeystore)).setCipherSuites(cipherSuites) + .setTlsProtocols(tlsProtocols).build(); + } catch (GeneralSecurityException e) { + LOG.error("Encryption of TrustKeystore for SslData failed.", e); + return null; + } if (MdsalUtils.put(dataBroker, LogicalDatastoreType.CONFIGURATION, getSslDataIid(bundleName), sslData)) { return new SslDataBuilder().withKey(sslDataKey).setOdlKeystore(odlKeystore).setTrustKeystore(trustKeystore) @@ -99,13 +107,13 @@ public OdlKeystore createOdlKeystore(final String name, final String alias, fina LOG.debug("Odl keystore string {} ", keyStoreBytes); return new OdlKeystoreBuilder().setKeystoreFile(keyStoreBytes) - .setAlias(alias).setDname(dname).setKeyAlg(keyAlg) - .setKeysize(keySize) - .setName(name) - .setSignAlg(sigAlg) - .setStorePassword(password) - .setValidity(validity) - .build(); + .setAlias(alias).setDname(dname).setKeyAlg(keyAlg) + .setKeysize(keySize) + .setName(name) + .setSignAlg(sigAlg) + .setStorePassword(password) + .setValidity(validity) + .build(); } public TrustKeystore createTrustKeystore(final String name, final String password, final byte[] keyStoreBytes) { @@ -118,66 +126,76 @@ public TrustKeystore createTrustKeystore(final String name, final String passwor password); LOG.debug("trust keystore string {} ", keyStoreBytes); return new TrustKeystoreBuilder() - .setKeystoreFile(keyStoreBytes) - .setName(name) - .setStorePassword(password) - .build(); + .setKeystoreFile(keyStoreBytes) + .setName(name) + .setStorePassword(password) + .build(); } - private OdlKeystore decryptOdlKeyStore(final OdlKeystore odlKeystore) { - if (odlKeystore == null) { - return null; - } - final OdlKeystoreBuilder odlKeystoreBuilder = new OdlKeystoreBuilder(odlKeystore); - odlKeystoreBuilder.setKeystoreFile(encryService.decrypt(odlKeystore.getKeystoreFile())); - odlKeystoreBuilder.setStorePassword(encryService.decrypt(odlKeystore.getStorePassword())); - return odlKeystoreBuilder.build(); + private OdlKeystore decryptOdlKeyStore(final OdlKeystore odlKeystore) throws GeneralSecurityException { + return odlKeystore == null ? null : new OdlKeystoreBuilder(odlKeystore) + .setKeystoreFile(decryptNullable(odlKeystore.getKeystoreFile())) + .setStorePassword(decryptStringFromBase64(odlKeystore.getStorePassword())) + .build(); } - private SslData decryptSslData(final SslData sslData) { - if (sslData == null) { - return null; - } - final SslDataBuilder sslDataBuilder = new SslDataBuilder(sslData) - .setOdlKeystore(decryptOdlKeyStore(sslData.getOdlKeystore())) - .setTrustKeystore(decryptTrustKeystore(sslData.getTrustKeystore())); - return sslDataBuilder.build(); + private SslData decryptSslData(final SslData sslData) throws GeneralSecurityException { + return sslData == null ? null : new SslDataBuilder(sslData) + .setOdlKeystore(decryptOdlKeyStore(sslData.getOdlKeystore())) + .setTrustKeystore(decryptTrustKeystore(sslData.getTrustKeystore())) + .build(); } - private TrustKeystore decryptTrustKeystore(final TrustKeystore trustKeyStore) { - if (trustKeyStore == null) { - return null; - } - final TrustKeystoreBuilder trustKeyStoreBuilder = new TrustKeystoreBuilder(trustKeyStore); - trustKeyStoreBuilder.setKeystoreFile(encryService.decrypt(trustKeyStore.getKeystoreFile())); - trustKeyStoreBuilder.setStorePassword(encryService.decrypt(trustKeyStore.getStorePassword())); - return trustKeyStoreBuilder.build(); + private TrustKeystore decryptTrustKeystore(final TrustKeystore trustKeyStore) throws GeneralSecurityException { + return trustKeyStore == null ? null : new TrustKeystoreBuilder(trustKeyStore) + .setKeystoreFile(decryptNullable(trustKeyStore.getKeystoreFile())) + .setStorePassword(decryptStringFromBase64(trustKeyStore.getStorePassword())) + .build(); + } + + private byte[] decryptNullable(final byte[] bytes) throws GeneralSecurityException { + return bytes == null ? null : encryService.decrypt(bytes); + } + + private String decryptStringFromBase64(final String base64) throws GeneralSecurityException { + return base64 == null ? null + : new String(encryService.decrypt(Base64.getDecoder().decode(base64)), Charset.defaultCharset()); } - private OdlKeystore encryptOdlKeyStore(final OdlKeystore odlKeystore) { - final OdlKeystoreBuilder odlKeystoreBuilder = new OdlKeystoreBuilder(odlKeystore); - odlKeystoreBuilder.setKeystoreFile(encryService.encrypt(odlKeystore.getKeystoreFile())); - odlKeystoreBuilder.setStorePassword(encryService.encrypt(odlKeystore.getStorePassword())); - return odlKeystoreBuilder.build(); + private String encryptStringToBase64(final String str) throws GeneralSecurityException { + return str == null ? null + : Base64.getEncoder().encodeToString(encryService.encrypt(str.getBytes(Charset.defaultCharset()))); } - private SslData encryptSslData(final SslData sslData) { - final SslDataBuilder sslDataBuilder = new SslDataBuilder(sslData) - .setOdlKeystore(encryptOdlKeyStore(sslData.getOdlKeystore())) - .setTrustKeystore(encryptTrustKeystore(sslData.getTrustKeystore())); - return sslDataBuilder.build(); + private OdlKeystore encryptOdlKeyStore(final OdlKeystore odlKeystore) throws GeneralSecurityException { + return new OdlKeystoreBuilder(odlKeystore) + .setKeystoreFile(encryService.encrypt(odlKeystore.getKeystoreFile())) + .setStorePassword(encryptStringToBase64(odlKeystore.getStorePassword())) + .build(); } - private TrustKeystore encryptTrustKeystore(final TrustKeystore trustKeyStore) { - final TrustKeystoreBuilder trustKeyStoreBuilder = new TrustKeystoreBuilder(trustKeyStore); - trustKeyStoreBuilder.setKeystoreFile(encryService.encrypt(trustKeyStore.getKeystoreFile())); - trustKeyStoreBuilder.setStorePassword(encryService.encrypt(trustKeyStore.getStorePassword())); - return trustKeyStoreBuilder.build(); + private SslData encryptSslData(final SslData sslData) throws GeneralSecurityException { + return new SslDataBuilder(sslData) + .setOdlKeystore(encryptOdlKeyStore(sslData.getOdlKeystore())) + .setTrustKeystore(encryptTrustKeystore(sslData.getTrustKeystore())) + .build(); + } + + private TrustKeystore encryptTrustKeystore(final TrustKeystore trustKeyStore) throws GeneralSecurityException { + return new TrustKeystoreBuilder(trustKeyStore) + .setKeystoreFile(encryService.encrypt(trustKeyStore.getKeystoreFile())) + .setStorePassword(encryptStringToBase64(trustKeyStore.getStorePassword())) + .build(); } public SslData getSslData(final DataBroker dataBroker, final String bundleName) { final InstanceIdentifier sslDataIid = getSslDataIid(bundleName); - return decryptSslData(MdsalUtils.read(dataBroker, LogicalDatastoreType.CONFIGURATION, sslDataIid)); + try { + return decryptSslData(MdsalUtils.read(dataBroker, LogicalDatastoreType.CONFIGURATION, sslDataIid)); + } catch (GeneralSecurityException e) { + LOG.error("Decryption of KeyStore for SslData failed.", e); + return null; + } } public boolean removeSslData(final DataBroker dataBroker, final String bundleName) { @@ -187,25 +205,29 @@ public boolean removeSslData(final DataBroker dataBroker, final String bundleNam public boolean updateSslData(final DataBroker dataBroker, final SslData sslData) { final InstanceIdentifier sslDataIid = getSslDataIid(sslData.getBundleName()); - return MdsalUtils.merge(dataBroker, LogicalDatastoreType.CONFIGURATION, sslDataIid, encryptSslData(sslData)); + final SslData encryptedSslData; + try { + encryptedSslData = encryptSslData(sslData); + } catch (GeneralSecurityException e) { + LOG.error("Encryption of KeyStore for SslData failed.", e); + return false; + } + return MdsalUtils.merge(dataBroker, LogicalDatastoreType.CONFIGURATION, sslDataIid, encryptedSslData); } public boolean updateSslDataCipherSuites(final DataBroker dataBroker, final SslData baseSslData, final List cipherSuites) { - final SslDataBuilder sslDataBuilder = new SslDataBuilder(baseSslData).setCipherSuites(cipherSuites); - return updateSslData(dataBroker, sslDataBuilder.build()); + return updateSslData(dataBroker, new SslDataBuilder(baseSslData).setCipherSuites(cipherSuites).build()); } public boolean updateSslDataOdlKeystore(final DataBroker dataBroker, final SslData baseSslData, final OdlKeystore odlKeyStore) { - final SslDataBuilder sslDataBuilder = new SslDataBuilder(baseSslData).setOdlKeystore(odlKeyStore); - return updateSslData(dataBroker, sslDataBuilder.build()); + return updateSslData(dataBroker, new SslDataBuilder(baseSslData).setOdlKeystore(odlKeyStore).build()); } public boolean updateSslDataTrustKeystore(final DataBroker dataBroker, final SslData baseSslData, final TrustKeystore trustKeyStore) { - final SslDataBuilder sslDataBuilder = new SslDataBuilder(baseSslData).setTrustKeystore(trustKeyStore); - return updateSslData(dataBroker, sslDataBuilder.build()); + return updateSslData(dataBroker, new SslDataBuilder(baseSslData).setTrustKeystore(trustKeyStore).build()); } public TrustKeystore updateTrustKeystore(final TrustKeystore baseTrustKeyStore, final byte[] keyStoreBytes) { diff --git a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertMdsalProviderTest.java b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertMdsalProviderTest.java index d65b7044a..94469c69d 100644 --- a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertMdsalProviderTest.java +++ b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertMdsalProviderTest.java @@ -9,22 +9,20 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.opendaylight.aaa.cert.impl.TestUtils.mockDataBroker; import java.io.File; +import java.nio.charset.Charset; import java.security.KeyStore; import java.security.Security; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.junit.BeforeClass; import org.junit.Test; import org.opendaylight.aaa.encrypt.AAAEncryptionService; -import org.opendaylight.yang.gen.v1.urn.opendaylight.yang.aaa.cert.mdsal.rev160321.cipher.suite.CipherSuites; import org.opendaylight.yang.gen.v1.urn.opendaylight.yang.aaa.cert.mdsal.rev160321.cipher.suite.CipherSuitesBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.yang.aaa.cert.mdsal.rev160321.key.stores.SslData; import org.opendaylight.yang.gen.v1.urn.opendaylight.yang.aaa.cert.mdsal.rev160321.key.stores.SslDataBuilder; @@ -68,11 +66,9 @@ public static void setUpBeforeClass() throws Exception { final TrustKeystore unsignedTrustKeyStore = keyStoresDataUtils.createTrustKeystore(TRUST_NAME, PASSWORD, odlKeyTool); - final CipherSuites cipherSuite = new CipherSuitesBuilder().setSuiteName(CIPHER_SUITE_NAME).build(); - - final List cipherSuites = new ArrayList<>(Arrays.asList(cipherSuite)); - - signedSslData = new SslDataBuilder().setCipherSuites(cipherSuites).setOdlKeystore(signedOdlKeystore) + signedSslData = new SslDataBuilder() + .setCipherSuites(List.of(new CipherSuitesBuilder().setSuiteName(CIPHER_SUITE_NAME).build())) + .setOdlKeystore(signedOdlKeystore) .setTrustKeystore(signedTrustKeyStore).setTlsProtocols(PROTOCOL).setBundleName(BUNDLE_NAME).build(); final OdlKeystore unsignedOdlKeystore = new OdlKeystoreBuilder().setAlias(ALIAS).setDname(D_NAME) @@ -84,11 +80,14 @@ public static void setUpBeforeClass() throws Exception { unsignedSslData = new SslDataBuilder().setOdlKeystore(unsignedOdlKeystore) .setTrustKeystore(unsignedTrustKeyStore).setBundleName(BUNDLE_NAME).build(); + when(aaaEncryptionServiceInit.encrypt(PASSWORD.getBytes())) + .thenReturn(PASSWORD.getBytes(Charset.defaultCharset())); when(aaaEncryptionServiceInit.decrypt(unsignedTrustKeyStore.getKeystoreFile())) .thenReturn(unsignedTrustKeyStore.getKeystoreFile()); when(aaaEncryptionServiceInit.decrypt(signedOdlKeystore.getKeystoreFile())) .thenReturn(signedOdlKeystore.getKeystoreFile()); - when(aaaEncryptionServiceInit.decrypt(isA(String.class))).thenReturn(PASSWORD); + when(aaaEncryptionServiceInit.decrypt(new byte[] { -91, -85, 44, 90, -118, -35 })) + .thenReturn(PASSWORD.getBytes(Charset.defaultCharset())); aaaEncryptionService = aaaEncryptionServiceInit; // Create class diff --git a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertRpcServiceImplTest.java b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertRpcServiceImplTest.java index 044b3d2c0..17754011e 100644 --- a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertRpcServiceImplTest.java +++ b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/AaaCertRpcServiceImplTest.java @@ -9,14 +9,15 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.opendaylight.aaa.cert.impl.TestUtils.mockDataBroker; import com.google.common.util.concurrent.Futures; import java.io.File; +import java.nio.charset.Charset; import java.security.Security; +import java.util.Base64; import java.util.List; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.junit.BeforeClass; @@ -99,7 +100,10 @@ public static void setUpBeforeClass() throws Exception { .thenReturn(unsignedTrustKeyStore.getKeystoreFile()); when(aaaEncryptionServiceInit.decrypt(signedOdlKeystore.getKeystoreFile())) .thenReturn(signedOdlKeystore.getKeystoreFile()); - when(aaaEncryptionServiceInit.decrypt(any(String.class))).thenReturn(PASSWORD); + when(aaaEncryptionServiceInit.decrypt(Base64.getDecoder().decode(PASSWORD))) + .thenReturn(PASSWORD.getBytes(Charset.defaultCharset())); + when(aaaEncryptionServiceInit.encrypt(PASSWORD.getBytes(Charset.defaultCharset()))) + .thenReturn(PASSWORD.getBytes(Charset.defaultCharset())); aaaEncryptionService = aaaEncryptionServiceInit; // Create class diff --git a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtilsTest.java b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtilsTest.java index aceabf958..99cc95c22 100644 --- a/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtilsTest.java +++ b/aaa-cert/src/test/java/org/opendaylight/aaa/cert/impl/KeyStoresDataUtilsTest.java @@ -10,11 +10,10 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; import java.io.File; +import java.nio.charset.Charset; import java.security.Security; import java.util.ArrayList; import java.util.Arrays; @@ -23,6 +22,7 @@ import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.opendaylight.aaa.encrypt.AAAEncryptionService; import org.opendaylight.mdsal.binding.api.DataBroker; @@ -48,7 +48,6 @@ public class KeyStoresDataUtilsTest { Security.addProvider(new BouncyCastleProvider()); } - private static final AAAEncryptionService AAA_ENCRYPTION_SERVICE = mock(AAAEncryptionService.class); private static final byte[] ENCRYPTED_BYTE = new byte[] { 1, 2, 3 }; private static final String ALIAS = "fooTest"; private static final String BUNDLE_NAME = "opendaylight"; @@ -61,10 +60,17 @@ public class KeyStoresDataUtilsTest { private static final String TRUST_NAME = "trustTest.jks"; private static final String TEST_PATH = "target" + File.separator + "test" + File.separator; - private final DataBroker dataBroker = mock(DataBroker.class); + @Mock + private AAAEncryptionService encryptionService; + @Mock + private DataBroker dataBroker; + @Mock + private WriteTransaction wtx; + @Mock + private ReadTransaction rtx; @Test - public void keyStoresDataUtilsTest() { + public void keyStoresDataUtilsTest() throws Exception { // Test vars setup final OdlKeystore odlKeystore = new OdlKeystoreBuilder().setAlias(ALIAS).setDname(D_NAME).setName(ODL_NAME) .setStorePassword(PASSWORD).setValidity(KeyStoreConstant.DEFAULT_VALIDITY) @@ -81,22 +87,21 @@ public void keyStoresDataUtilsTest() { .setBundleName(BUNDLE_NAME).build(); final ODLKeyTool odlKeyTool = new ODLKeyTool(TEST_PATH); - final KeyStoresDataUtils keyStoresDataUtils = new KeyStoresDataUtils(AAA_ENCRYPTION_SERVICE); + final KeyStoresDataUtils keyStoresDataUtils = new KeyStoresDataUtils(encryptionService); // Mock setup - final WriteTransaction wtx = mock(WriteTransaction.class); doReturn(CommitInfo.emptyFluentFuture()).when(wtx).commit(); doReturn(wtx).when(dataBroker).newWriteOnlyTransaction(); - final ReadTransaction rtx = mock(ReadTransaction.class); doReturn(FluentFutures.immediateFluentFuture(Optional.of(sslData))).when(rtx).read( any(LogicalDatastoreType.class), any(InstanceIdentifier.class)); doReturn(rtx).when(dataBroker).newReadOnlyTransaction(); - doReturn(ENCRYPTED_STRING).when(AAA_ENCRYPTION_SERVICE).encrypt(isA(String.class)); + doReturn(ENCRYPTED_STRING.getBytes(Charset.defaultCharset())).when(encryptionService).encrypt(any()); + doReturn(PASSWORD.getBytes(Charset.defaultCharset())).when(encryptionService).decrypt(any()); // getKeystoresIid - InstanceIdentifier instanceIdentifierResult = KeyStoresDataUtils.getKeystoresIid(); + InstanceIdentifier instanceIdentifierResult = KeyStoresDataUtils.getKeystoresIid(); assertNotNull(instanceIdentifierResult); // getSslIid() diff --git a/aaa-encrypt-service/api/src/main/java/org/opendaylight/aaa/encrypt/AAAEncryptionService.java b/aaa-encrypt-service/api/src/main/java/org/opendaylight/aaa/encrypt/AAAEncryptionService.java index 82903574d..55f0a55eb 100644 --- a/aaa-encrypt-service/api/src/main/java/org/opendaylight/aaa/encrypt/AAAEncryptionService.java +++ b/aaa-encrypt-service/api/src/main/java/org/opendaylight/aaa/encrypt/AAAEncryptionService.java @@ -7,42 +7,31 @@ */ package org.opendaylight.aaa.encrypt; +import java.security.GeneralSecurityException; + /** * A generic encryption/decryption service for encrypting various data in ODL. * * @author - Sharon Aicler (saichler@gmail.com) */ public interface AAAEncryptionService { - - /** - * Encrypt data using a 2-way encryption mechanism. - * - * @param data plaintext data - * @return an encrypted representation of data - */ - String encrypt(String data); - /** - * Encrypt data using a 2-way encryption mechanism. + * Encrypt {@code data} using a 2-way encryption mechanism. * * @param data plaintext data - * @return an encrypted representation of data - */ - byte[] encrypt(byte[] data); - - /** - * Decrypt data using a 2-way decryption mechanism. - * - * @param encryptedData encrypted data - * @return plaintext data + * @return an encrypted representation of {@code data} + * @throws NullPointerException when {@code data} is {@code null} + * @throws GeneralSecurityException when encryption fails */ - String decrypt(String encryptedData); + byte[] encrypt(byte[] data) throws GeneralSecurityException; /** - * Decrypt data using a 2-way decryption mechanism. + * Decrypt {@code encryptedData} using a 2-way decryption mechanism. * * @param encryptedData encrypted data - * @return plaintext data + * @return plaintext bytes + * @throws NullPointerException when {@code encryptedData} is {@code null} + * @throws GeneralSecurityException when decryption fails */ - byte[] decrypt(byte[] encryptedData); + byte[] decrypt(byte[] encryptedData) throws GeneralSecurityException; } diff --git a/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptionServiceImpl.java b/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptionServiceImpl.java index 695c204f4..34c681e16 100644 --- a/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptionServiceImpl.java +++ b/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptionServiceImpl.java @@ -10,18 +10,11 @@ import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; -import java.nio.charset.Charset; -import java.security.InvalidAlgorithmParameterException; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; -import java.security.spec.InvalidKeySpecException; -import java.security.spec.KeySpec; -import java.util.Base64; +import java.security.GeneralSecurityException; import java.util.Map; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.IvParameterSpec; @@ -54,37 +47,30 @@ public final class AAAEncryptionServiceImpl implements AAAEncryptionService { public AAAEncryptionServiceImpl(final EncryptServiceConfig configuration) { final byte[] encryptionKeySalt = configuration.requireEncryptSalt(); - IvParameterSpec tempIvSpec = null; - SecretKey tempKey = null; + final IvParameterSpec ivSpec; try { - final SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(configuration.getEncryptMethod()); - final KeySpec spec = new PBEKeySpec(configuration.requireEncryptKey().toCharArray(), encryptionKeySalt, + final var keyFactory = SecretKeyFactory.getInstance(configuration.getEncryptMethod()); + final var spec = new PBEKeySpec(configuration.requireEncryptKey().toCharArray(), encryptionKeySalt, configuration.getEncryptIterationCount(), configuration.getEncryptKeyLength()); - tempKey = new SecretKeySpec(keyFactory.generateSecret(spec).getEncoded(), configuration.getEncryptType()); - tempIvSpec = new IvParameterSpec(encryptionKeySalt); - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { - LOG.error("Failed to initialize secret key", e); + key = new SecretKeySpec(keyFactory.generateSecret(spec).getEncoded(), configuration.getEncryptType()); + ivSpec = new IvParameterSpec(encryptionKeySalt); + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Failed to initialize secret key", e); } - key = tempKey; - final var ivSpec = tempIvSpec; - Cipher cipher = null; try { - cipher = Cipher.getInstance(configuration.getCipherTransforms()); + final var cipher = Cipher.getInstance(configuration.getCipherTransforms()); cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec); - } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException - | InvalidKeyException e) { - LOG.error("Failed to create encrypt cipher.", e); + encryptCipher = cipher; + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Failed to create encrypt cipher.", e); } - encryptCipher = cipher; - cipher = null; try { - cipher = Cipher.getInstance(configuration.getCipherTransforms()); + final var cipher = Cipher.getInstance(configuration.getCipherTransforms()); cipher.init(Cipher.DECRYPT_MODE, key, ivSpec); - } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException - | InvalidKeyException e) { - LOG.error("Failed to create decrypt cipher.", e); + decryptCipher = cipher; + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Failed to create decrypt cipher.", e); } - decryptCipher = cipher; LOG.info("AAAEncryptionService activated"); } @@ -104,73 +90,14 @@ void deactivate() { } @Override - public String encrypt(final String data) { - // We could not instantiate the encryption key, hence no encryption or - // decryption will be done. - if (key == null) { - LOG.warn("Encryption Key is NULL, will not encrypt data."); - return data; - } - - final byte[] cryptobytes; - try { - synchronized (encryptCipher) { - cryptobytes = encryptCipher.doFinal(data.getBytes(Charset.defaultCharset())); - } - } catch (IllegalBlockSizeException | BadPaddingException e) { - LOG.error("Failed to encrypt data.", e); - return data; - } - return Base64.getEncoder().encodeToString(cryptobytes); - } - - @Override - public byte[] encrypt(final byte[] data) { - // We could not instantiate the encryption key, hence no encryption or - // decryption will be done. - if (key == null) { - LOG.warn("Encryption Key is NULL, will not encrypt data."); - return data; - } - try { - synchronized (encryptCipher) { - return encryptCipher.doFinal(data); - } - } catch (IllegalBlockSizeException | BadPaddingException e) { - LOG.error("Failed to encrypt data.", e); - return data; + public byte[] encrypt(final byte[] data) throws BadPaddingException, IllegalBlockSizeException { + synchronized (encryptCipher) { + return encryptCipher.doFinal(requireNonNull(data)); } } @Override - public String decrypt(final String encryptedData) { - if (key == null || encryptedData == null || encryptedData.length() == 0) { - LOG.warn("String {} was not decrypted.", encryptedData); - return encryptedData; - } - - final byte[] cryptobytes = Base64.getDecoder().decode(encryptedData); - final byte[] clearbytes; - try { - clearbytes = decryptCipher.doFinal(cryptobytes); - } catch (IllegalBlockSizeException | BadPaddingException e) { - LOG.error("Failed to decrypt encoded data", e); - return encryptedData; - } - return new String(clearbytes, Charset.defaultCharset()); - } - - @Override - public byte[] decrypt(final byte[] encryptedData) { - if (encryptedData == null) { - LOG.warn("encryptedData is null."); - return encryptedData; - } - try { - return decryptCipher.doFinal(encryptedData); - } catch (IllegalBlockSizeException | BadPaddingException e) { - LOG.error("Failed to decrypt encoded data", e); - } - return encryptedData; + public byte[] decrypt(final byte[] encryptedData) throws BadPaddingException, IllegalBlockSizeException { + return decryptCipher.doFinal(requireNonNull(encryptedData)); } } diff --git a/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/OSGiEncryptionServiceConfigurator.java b/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/OSGiEncryptionServiceConfigurator.java index be485d03c..eca84bc4d 100644 --- a/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/OSGiEncryptionServiceConfigurator.java +++ b/aaa-encrypt-service/impl/src/main/java/org/opendaylight/aaa/encrypt/impl/OSGiEncryptionServiceConfigurator.java @@ -33,6 +33,7 @@ import org.opendaylight.yangtools.concepts.Registration; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.osgi.framework.FrameworkUtil; +import org.osgi.service.component.ComponentException; import org.osgi.service.component.ComponentFactory; import org.osgi.service.component.ComponentInstance; import org.osgi.service.component.annotations.Activate; @@ -196,8 +197,14 @@ private synchronized void updateInstance(final AaaEncryptServiceConfig newConfig } disableInstance(); - instance = factory.newInstance(FrameworkUtil.asDictionary( - AAAEncryptionServiceImpl.props(new EncryptServiceConfigImpl(newConfig)))); + try { + instance = factory.newInstance(FrameworkUtil.asDictionary( + AAAEncryptionServiceImpl.props(new EncryptServiceConfigImpl(newConfig)))); + } catch (ComponentException e) { + LOG.error("Failed to start Encryption Service", e); + return; + } + current = newConfig; LOG.info("Encryption Service enabled"); } diff --git a/aaa-encrypt-service/impl/src/test/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptServiceImplTest.java b/aaa-encrypt-service/impl/src/test/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptServiceImplTest.java index 5eac2f18e..d0bbdf41e 100644 --- a/aaa-encrypt-service/impl/src/test/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptServiceImplTest.java +++ b/aaa-encrypt-service/impl/src/test/java/org/opendaylight/aaa/encrypt/impl/AAAEncryptServiceImplTest.java @@ -7,24 +7,28 @@ */ package org.opendaylight.aaa.encrypt.impl; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import java.nio.charset.StandardCharsets; -import java.util.Base64; -import org.junit.Before; -import org.junit.Test; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Arrays; +import javax.crypto.BadPaddingException; +import javax.crypto.IllegalBlockSizeException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.opendaylight.yang.gen.v1.config.aaa.authn.encrypt.service.config.rev160915.AaaEncryptServiceConfigBuilder; /* * @author - Sharon Aicler (saichler@gmail.com) */ @Deprecated -public class AAAEncryptServiceImplTest { +class AAAEncryptServiceImplTest { private AAAEncryptionServiceImpl impl; - @Before - public void setup() { + @BeforeEach + void setup() { impl = new AAAEncryptionServiceImpl(new EncryptServiceConfigImpl( OSGiEncryptionServiceConfigurator.generateConfig(new AaaEncryptServiceConfigBuilder() .setCipherTransforms("AES/CBC/PKCS5Padding") @@ -38,99 +42,100 @@ public void setup() { .build()))); } + private void changePadding() { + impl = new AAAEncryptionServiceImpl(new EncryptServiceConfigImpl( + OSGiEncryptionServiceConfigurator.generateConfig(new AaaEncryptServiceConfigBuilder() + .setCipherTransforms("AES/CBC/NoPadding") + .setEncryptIterationCount(32768) + .setEncryptKey("") + .setEncryptKeyLength(128) + .setEncryptMethod("PBKDF2WithHmacSHA1") + .setEncryptSalt("") + .setEncryptType("AES") + .setPasswordLength(12) + .build()))); + } + @Test - public void testShortString() { - String before = "shortone"; - String encrypt = impl.encrypt(before); - assertNotEquals(before, encrypt); - String after = impl.decrypt(encrypt); - assertEquals(before, after); + void testShortString() throws Exception { + final var before = "shortone".getBytes(); + final var encrypt = impl.encrypt(before); + assertFalse(Arrays.equals(before, encrypt)); + assertArrayEquals(before, impl.decrypt(encrypt)); } @Test - public void testLongString() { - String before = "This is a very long string to encrypt for testing 1...2...3"; - String encrypt = impl.encrypt(before); - assertNotEquals(before, encrypt); - String after = impl.decrypt(encrypt); - assertEquals(before, after); + void testLongString() throws Exception { + final var before = "This is a very long string to encrypt for testing 1...2...3".getBytes(); + final var encrypt = impl.encrypt(before); + assertFalse(Arrays.equals(before, encrypt)); + assertArrayEquals(before, impl.decrypt(encrypt)); } @Test - public void testNetconfEncodedPasswordWithoutPadding() { + void testNetconfEncodedPasswordWithoutPadding() { changePadding(); - String password = "bmV0Y29uZgo="; - String unencrypted = impl.decrypt(password); - assertEquals(password, unencrypted); + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("netconf\n".getBytes())); + assertEquals("Input length not multiple of 16 bytes", ex.getMessage()); } @Test - public void testNetconfEncodedPasswordWithPadding() { - String password = "bmV0Y29uZgo="; - String unencrypted = impl.decrypt(password); - assertEquals(password, unencrypted); + void testNetconfEncodedPasswordWithPadding() { + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("netconf\n".getBytes())); + assertEquals("Input length must be multiple of 16 when decrypting with padded cipher", ex.getMessage()); } @Test - public void testNetconfPasswordWithoutPadding() { + void testNetconfPasswordWithoutPadding() { changePadding(); - String password = "netconf"; - String encodedPassword = Base64.getEncoder().encodeToString(password.getBytes(StandardCharsets.UTF_8)); - String unencrypted = impl.decrypt(encodedPassword); - assertEquals(encodedPassword, unencrypted); + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("netconf".getBytes())); + assertEquals("Input length not multiple of 16 bytes", ex.getMessage()); } @Test - public void testNetconfPasswordWithPadding() { - String password = "netconf"; - String encodedPassword = Base64.getEncoder().encodeToString(password.getBytes(StandardCharsets.UTF_8)); - String unencrypted = impl.decrypt(encodedPassword); - assertEquals(encodedPassword, unencrypted); + void testNetconfPasswordWithPadding() { + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("netconf".getBytes())); + assertEquals("Input length must be multiple of 16 when decrypting with padded cipher", ex.getMessage()); } @Test - public void testAdminEncodedPasswordWithoutPadding() { + void testAdminEncodedPasswordWithoutPadding() { changePadding(); - String password = "YWRtaW4K"; - String unencrypted = impl.decrypt(password); - assertEquals(password, unencrypted); + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("admin\n".getBytes())); + assertEquals("Input length not multiple of 16 bytes", ex.getMessage()); } @Test - public void testAdminEncodedPasswordWithPadding() { - String password = "YWRtaW4K"; - String unencrypted = impl.decrypt(password); - assertEquals(password, unencrypted); + void testAdminEncodedPasswordWithPadding() { + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("admin\n".getBytes())); + assertEquals("Input length must be multiple of 16 when decrypting with padded cipher", ex.getMessage()); } @Test - public void testAdminPasswordWithoutPadding() { + void testAdminPasswordWithoutPadding() { changePadding(); - String password = "admin"; - String encodedPassword = Base64.getEncoder().encodeToString(password.getBytes(StandardCharsets.UTF_8)); - String unencrypted = impl.decrypt(encodedPassword); - assertEquals(encodedPassword, unencrypted); + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("admin".getBytes())); + assertEquals("Input length not multiple of 16 bytes", ex.getMessage()); } @Test - public void testAdminPasswordWithPadding() { - String password = "admin"; - String encodedPassword = Base64.getEncoder().encodeToString(password.getBytes(StandardCharsets.UTF_8)); - String unencrypted = impl.decrypt(encodedPassword); - assertEquals(encodedPassword, unencrypted); + void testAdminPasswordWithPadding() { + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("admin".getBytes())); + assertEquals("Input length must be multiple of 16 when decrypting with padded cipher", ex.getMessage()); } - private void changePadding() { - impl = new AAAEncryptionServiceImpl(new EncryptServiceConfigImpl( - OSGiEncryptionServiceConfigurator.generateConfig(new AaaEncryptServiceConfigBuilder() - .setCipherTransforms("AES/CBC/NoPadding") - .setEncryptIterationCount(32768) - .setEncryptKey("") - .setEncryptKeyLength(128) - .setEncryptMethod("PBKDF2WithHmacSHA1") - .setEncryptSalt("") - .setEncryptType("AES") - .setPasswordLength(12) - .build()))); + @Test + void testDecryptWithIllegalBlockSizeException() { + final var ex = assertThrows(IllegalBlockSizeException.class, () -> impl.decrypt("adminadmin".getBytes())); + assertEquals("Input length must be multiple of 16 when decrypting with padded cipher", ex.getMessage()); + } + + @Test + void testDecryptWithBadPaddingException() { + final var bytes = new byte[] { 85, -87, 98, 116, -23, -84, 123, -82, 4, -99, -54, 29, 121, -48, -38, -75 }; + final var ex = assertThrows(BadPaddingException.class, () -> impl.decrypt(bytes)); + assertEquals( + "Given final block not properly padded. Such issues can arise if a bad key is used during decryption.", + ex.getMessage()); } }