Skip to content

Commit

Permalink
Add parts of re-encryption feature (MID-4942)
Browse files Browse the repository at this point in the history
This commit adds CryptoUtil.reencryptValues(protector, object) method.
To complete the MID-4942 a wrapper would be needed that would take an
object, call this method, and if there are any changes detected,
replace the object in the repository.
  • Loading branch information
mederly committed Oct 16, 2018
1 parent 8ffa8a1 commit 93b1f3b
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 12 deletions.
Expand Up @@ -37,6 +37,7 @@
import com.evolveum.midpoint.prism.delta.ObjectDelta;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.result.OperationResultStatus;
import com.evolveum.midpoint.util.Holder;
import com.evolveum.midpoint.util.exception.TunnelException;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
Expand Down Expand Up @@ -89,6 +90,7 @@ private static Visitor createEncryptingVisitor(Protector protector) {
};
}

// todo refactor - align with the other 2 variants of this method
private static void encryptValue(Protector protector, PrismPropertyValue<?> pval) throws EncryptionException{
Itemable item = pval.getParent();
if (item == null) {
Expand All @@ -99,6 +101,8 @@ private static void encryptValue(Protector protector, PrismPropertyValue<?> pval
return;
}

// todo what about ProtectedByteArrayType?
// todo shouldn't we check the actual value instead of declared one?
if (itemDef.getTypeName().equals(ProtectedStringType.COMPLEX_TYPE)) {
QName propName = item.getElementName();
@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -185,6 +189,8 @@ private static void checkEncrypted(PrismPropertyValue<?> pval) {
if (itemDef == null) {
return;
}
// todo what about ProtectedByteArrayType?
// todo shouldn't we check the actual value instead of declared one?
if (itemDef.getTypeName().equals(ProtectedStringType.COMPLEX_TYPE)) {
QName propName = item.getElementName();
@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -232,7 +238,6 @@ public static void checkEncrypted(Collection<? extends ItemDelta> modifications)
} catch (IllegalStateException e) {
throw new IllegalStateException(e.getMessage() + " in modification " + delta, e);
}

}
}

Expand Down Expand Up @@ -279,6 +284,105 @@ public static void securitySelfTest(OperationResult parentTestResult) {
result.computeStatus();
}

/**
* Re-encrypts all encryptable values in the object.
*/
public static <T extends ObjectType> int reencryptValues(Protector protector, PrismObject<T> object) throws EncryptionException {
try {
Holder<Integer> modCountHolder = new Holder<>(0);
object.accept(createReencryptingVisitor(protector, modCountHolder));
return modCountHolder.getValue();
} catch (TunnelException e) {
throw (EncryptionException) e.getCause();
}
}

@NotNull
private static Visitor createReencryptingVisitor(Protector protector, Holder<Integer> modCountHolder) {
return visitable -> {
if (!(visitable instanceof PrismPropertyValue)) {
return;
}
PrismPropertyValue<?> pval = (PrismPropertyValue<?>)visitable;
try {
reencryptValue(protector, pval, modCountHolder);
} catch (EncryptionException e) {
throw new TunnelException(e);
}
};
}

private static void reencryptValue(Protector protector, PrismPropertyValue<?> pval, Holder<Integer> modCountHolder)
throws EncryptionException {
Itemable item = pval.getParent();
if (item == null) {
return;
}
ItemDefinition itemDef = item.getDefinition();
if (itemDef == null) {
return;
}

// todo what about ProtectedByteArrayType?
// todo shouldn't we check the actual value instead of declared one?
if (itemDef.getTypeName().equals(ProtectedStringType.COMPLEX_TYPE)) {
QName propName = item.getElementName();
@SuppressWarnings("unchecked")
PrismPropertyValue<ProtectedStringType> psPval = (PrismPropertyValue<ProtectedStringType>) pval;
ProtectedStringType ps = psPval.getValue();
reencryptProtectedStringType(protector, ps, propName.getLocalPart(), modCountHolder);
if (pval.getParent() == null) {
pval.setParent(item); // todo ??? if the parent is null we wouldn't get here
}
} else if (itemDef.getTypeName().equals(MailConfigurationType.COMPLEX_TYPE)) {
// todo fix this hack (it's because MailConfigurationType is not a container)
@SuppressWarnings("unchecked")
MailConfigurationType mailCfg = ((PrismPropertyValue<MailConfigurationType>) pval).getValue();
if (mailCfg != null) {
for (MailServerConfigurationType serverCfg : mailCfg.getServer()) {
reencryptProtectedStringType(protector, serverCfg.getPassword(), "mail server password", modCountHolder);
}
}
} else if (itemDef.getTypeName().equals(SmsConfigurationType.COMPLEX_TYPE)) {
// todo fix this hack (it's because SmsConfigurationType is not a container)
@SuppressWarnings("unchecked")
SmsConfigurationType smsCfg = ((PrismPropertyValue<SmsConfigurationType>) pval).getValue();
if (smsCfg != null) {
for (SmsGatewayConfigurationType gwCfg : smsCfg.getGateway()) {
reencryptProtectedStringType(protector, gwCfg.getPassword(), "sms gateway password", modCountHolder);
}
}
}
}

private static void reencryptProtectedStringType(Protector protector, ProtectedStringType ps, String propName,
Holder<Integer> modCountHolder) throws EncryptionException {
if (ps == null) {
// nothing to do here
} else if (ps.isHashed()) {
// nothing to do here
} else if (ps.getClearValue() != null) {
try {
protector.encrypt(ps);
increment(modCountHolder);
} catch (EncryptionException e) {
throw new EncryptionException("Failed to encrypt value for field " + propName + ": " + e.getMessage(), e);
}
} else if (ps.getEncryptedDataType() != null) {
if (!protector.isEncryptedByCurrentKey(ps.getEncryptedDataType())) {
ProtectedStringType reencrypted = protector.encryptString(protector.decryptString(ps));
ps.setEncryptedData(reencrypted.getEncryptedDataType());
increment(modCountHolder);
}
} else {
// no clear nor encrypted value
}
}

private static void increment(Holder<Integer> countHolder) {
countHolder.setValue(countHolder.getValue() + 1);
}

private static void securitySelfTestAlgorithm(String algorithmName, String transformationName,
Integer keySize, boolean critical, OperationResult parentResult) {
OperationResult subresult = parentResult.createSubresult(CryptoUtil.class.getName()+".securitySelfTest.algorithm."+algorithmName);
Expand Down
Expand Up @@ -26,9 +26,8 @@
import com.evolveum.midpoint.test.util.TestUtil;
import com.evolveum.midpoint.util.PrettyPrinter;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.SystemConfigurationType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import com.evolveum.prism.xml.ns._public.types_3.ProtectedStringType;
import org.apache.xml.security.encryption.XMLCipher;
import org.testng.annotations.BeforeSuite;
import org.testng.annotations.Listeners;
Expand All @@ -37,10 +36,14 @@

import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.Set;

import static com.evolveum.midpoint.test.util.MidPointTestConstants.TEST_RESOURCES_DIR;
import static com.evolveum.midpoint.test.util.MidPointTestConstants.TEST_RESOURCES_PATH;
import static org.testng.Assert.fail;
import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertFalse;

/**
* @author mederly
Expand Down Expand Up @@ -126,6 +129,63 @@ public void test210EncryptSystemConfiguration() throws Exception {
CryptoUtil.checkEncrypted(config);
}

// MID-4942
@Test
public void test300Reencryption() throws Exception {
final String TEST_NAME = "test300Reencryption";
TestUtil.displayTestTitle(TEST_NAME);

// GIVEN
PrismContext prismContext = PrismTestUtil.getPrismContext();
PrismObject<UserType> jack = prismContext.parserFor(FILE_USER_JACK).xml().parse();
PrismObject<SystemConfigurationType> config = prismContext.parserFor(FILE_SYSTEM_CONFIGURATION).xml().parse();
Protector compromisedProtector = createCompromisedProtector();

// WHEN
CryptoUtil.encryptValues(compromisedProtector, jack);
CryptoUtil.encryptValues(compromisedProtector, config);
System.out.println("jack compromised:\n" + prismContext.xmlSerializer().serialize(jack));
System.out.println("sysconfig compromised:\n" + prismContext.xmlSerializer().serialize(config));
CryptoUtil.checkEncrypted(jack);
CryptoUtil.checkEncrypted(config);
MailConfigurationType mail = config.asObjectable().getNotificationConfiguration().getMail();
SmsConfigurationType sms1 = config.asObjectable().getNotificationConfiguration().getSms().get(0);
SmsConfigurationType sms2 = config.asObjectable().getNotificationConfiguration().getSms().get(1);
String compromisedKeyName = getKeyName(
jack.asObjectable().getCredentials().getPassword().getValue(),
mail.getServer().get(0).getPassword(),
sms1.getGateway().get(0).getPassword(),
sms2.getGateway().get(0).getPassword());
System.out.println("Compromised key name: " + compromisedKeyName);

// THEN
int reencryptJackOld = CryptoUtil.reencryptValues(compromisedProtector, jack);
int reencryptJackNew = CryptoUtil.reencryptValues(protector, jack);
int reencryptConfigNew = CryptoUtil.reencryptValues(protector, config);

assertEquals("Wrong # of reencrypted passwords (jack old)", 0, reencryptJackOld);
assertEquals("Wrong # of reencrypted passwords (jack new)", 1, reencryptJackNew);
assertEquals("Wrong # of reencrypted passwords (sysconfig new)", 3, reencryptConfigNew);
System.out.println("jack reencrypted:\n" + prismContext.xmlSerializer().serialize(jack));
System.out.println("sysconfig reencrypted:\n" + prismContext.xmlSerializer().serialize(config));
String newKeyName = getKeyName(
jack.asObjectable().getCredentials().getPassword().getValue(),
mail.getServer().get(0).getPassword(),
sms1.getGateway().get(0).getPassword(),
sms2.getGateway().get(0).getPassword());
System.out.println("New key name: " + newKeyName);
assertFalse("New and compromised key names are NOT different", compromisedKeyName.equals(newKeyName));
}

private String getKeyName(ProtectedStringType... values) {
Set<String> names = new HashSet<>();
for (ProtectedStringType value : values) {
names.add(value.getEncryptedDataType().getKeyInfo().getKeyName());
}
assertEquals("Wrong # of different key names: " + names, 1, names.size());
return names.iterator().next();
}

private void checkEncryptedObject(PrismObject<? extends ObjectType> object) {
try {
CryptoUtil.checkEncrypted(object);
Expand All @@ -145,4 +205,14 @@ private Protector createProtector() {
return protector;
}

private Protector createCompromisedProtector() {
ProtectorImpl protector = new ProtectorImpl();
protector.setKeyStorePassword(KEYSTORE_PASSWORD);
protector.setKeyStorePath(KEYSTORE_PATH);
protector.setEncryptionKeyAlias("compromised");
protector.setEncryptionAlgorithm(XMLCipher.AES_256);
protector.init();
return protector;
}

}
Binary file modified infra/common/src/test/resources/keystore.jceks
Binary file not shown.
Expand Up @@ -21,9 +21,11 @@

import javax.net.ssl.TrustManager;

import com.evolveum.prism.xml.ns._public.types_3.EncryptedDataType;
import com.evolveum.prism.xml.ns._public.types_3.ProtectedStringType;

import com.evolveum.midpoint.util.exception.SchemaException;
import org.jetbrains.annotations.NotNull;

public interface Protector {

Expand Down Expand Up @@ -74,4 +76,5 @@ public interface Protector {

boolean compare(ProtectedStringType a, ProtectedStringType b) throws EncryptionException, SchemaException;

boolean isEncryptedByCurrentKey(@NotNull EncryptedDataType data) throws EncryptionException;
}
Expand Up @@ -21,19 +21,14 @@
import com.evolveum.midpoint.util.exception.SystemException;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
import com.evolveum.prism.xml.ns._public.types_3.CipherDataType;
import com.evolveum.prism.xml.ns._public.types_3.DigestMethodType;
import com.evolveum.prism.xml.ns._public.types_3.EncryptedDataType;
import com.evolveum.prism.xml.ns._public.types_3.EncryptionMethodType;
import com.evolveum.prism.xml.ns._public.types_3.HashedDataType;
import com.evolveum.prism.xml.ns._public.types_3.KeyInfoType;
import com.evolveum.prism.xml.ns._public.types_3.ProtectedStringType;
import com.evolveum.prism.xml.ns._public.types_3.*;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.Validate;
import org.apache.xml.security.Init;
import org.apache.xml.security.algorithms.JCEMapper;
import org.apache.xml.security.encryption.XMLCipher;
import org.apache.xml.security.utils.Base64;
import org.jetbrains.annotations.NotNull;

import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
Expand Down Expand Up @@ -510,7 +505,7 @@ public <T> void hash(ProtectedData<T> protectedData) throws EncryptionException,
switch (algorithmNamespace) {
case PrismConstants.NS_CRYPTO_ALGORITHM_PBKD:
if (!protectedData.canSupportType(String.class)) {
throw new SchemaException("Non-string proteted data");
throw new SchemaException("Non-string protected data");
}
hashedDataType = hashPbkd((ProtectedData<String>) protectedData, algorithmUri, algorithmQName.getLocalPart());
break;
Expand Down Expand Up @@ -660,5 +655,15 @@ private boolean compareHashedPbkd(HashedDataType hashedDataType, String algorith
return Arrays.equals(digestValue, hashBytes);
}

@Override
public boolean isEncryptedByCurrentKey(@NotNull EncryptedDataType data) throws EncryptionException {
String encryptedUsingKeyName = data.getKeyInfo().getKeyName();
if (encryptedUsingKeyName == null) {
throw new IllegalStateException("No key name in encrypted data: " + data);
}
SecretKey encryptedUsingKey = getSecretKeyByDigest(encryptedUsingKeyName);
SecretKey currentKey = getSecretKeyByAlias(getEncryptionKeyAlias());
return currentKey.equals(encryptedUsingKey);
}

}

0 comments on commit 93b1f3b

Please sign in to comment.