ensure that sensitive information is not serialized (part 2)

This commit is contained in:
Vladimir Krivosheev
2018-11-29 14:19:02 +01:00
parent b8480c1d72
commit 706f58798c
5 changed files with 120 additions and 28 deletions
@@ -1,4 +1,6 @@
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
import com.intellij.configurationStore.BaseXmlOutputter
import com.intellij.openapi.application.ApplicationManager
@@ -69,8 +71,8 @@ class DoNotStorePasswordTest {
for (accessor in XmlSerializerUtil.getAccessors(clazz)) {
val name = accessor.name
if (name.contains("password", ignoreCase = true) && !BaseXmlOutputter.isSavePasswordField(name)) {
System.out.println("${clazz.typeName}.${accessor.name}")
if (BaseXmlOutputter.isNameIndicatesSensitiveInformation(name)) {
throw RuntimeException("${clazz.typeName}.${accessor.name}")
}
else if (!accessor.valueClass.isPrimitive) {
@Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN")
@@ -3,10 +3,7 @@
package com.intellij.configurationStore.xml
import com.intellij.configurationStore.StoredPropertyStateTest
import com.intellij.configurationStore.clearBindingCache
import com.intellij.configurationStore.deserialize
import com.intellij.configurationStore.serialize
import com.intellij.configurationStore.*
import com.intellij.openapi.util.text.StringUtil
import com.intellij.testFramework.UsefulTestCase
import com.intellij.testFramework.assertConcurrent
@@ -14,12 +11,15 @@ import com.intellij.testFramework.assertions.Assertions.assertThat
import com.intellij.util.loadElement
import com.intellij.util.xmlb.*
import com.intellij.util.xmlb.annotations.*
import com.intellij.util.xmlb.annotations.Property
import junit.framework.TestCase
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.intellij.lang.annotations.Language
import org.jdom.Element
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.Suite
import java.io.StringWriter
import java.util.*
@RunWith(Suite::class)
@@ -627,26 +627,80 @@ internal class XmlSerializerTest {
testSerializer("<BeanWithDefaultAttributeName foo=\"foo\" />", BeanWithDefaultAttributeName())
}
private class Bean2 {
@Attribute
var ab: String? = null
@Test
fun ordered() {
@Tag("bean")
class Bean {
@Attribute
var ab: String? = null
@Attribute
var module: String? = null
@Attribute
var module: String? = null
@Suppress("unused")
@Attribute
var ac: String? = null
}
@Suppress("unused")
@Attribute
var ac: String? = null
}
@Test fun ordered() {
val bean = Bean2()
val bean = Bean()
bean.module = "module"
bean.ab = "ab"
testSerializer("<Bean2 ab=\"ab\" module=\"module\" />", bean, SkipDefaultsSerializationFilter())
testSerializer("<bean ab=\"ab\" module=\"module\" />", bean, SkipDefaultsSerializationFilter())
}
@Test fun cdataAfterNewLine() {
@Test
fun `do not store password as attribute`() {
@Tag("bean")
class Bean {
@Attribute
var password: String? = null
@Attribute
var foo: String? = null
}
val bean = Bean()
bean.foo = "module"
bean.password = "ab"
// it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data
// so, testSerializer must not throw error
val element = assertSerializer(bean, "<bean password=\"ab\" foo=\"module\" />")
assertThatThrownBy {
val xmlWriter = JbXmlOutputter(true)
xmlWriter.output(element, StringWriter())
}.hasMessage("Attribute \"password\" probably contains sensitive information")
}
@Test
fun `do not store password as element`() {
@Tag("bean")
class Bean {
var password: String? = null
@Attribute
var foo: String? = null
}
val bean = Bean()
bean.foo = "module"
bean.password = "ab"
// it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data
// so, testSerializer must not throw error
val element = assertSerializer(bean, """
<bean foo="module">
<option name="password" value="ab" />
</bean>
""".trimIndent())
assertThatThrownBy {
val xmlWriter = JbXmlOutputter(true)
xmlWriter.output(element, StringWriter())
}.hasMessage("Element \"password\" probably contains sensitive information")
}
@Test
fun cdataAfterNewLine() {
@Tag("bean")
data class Bean(@Tag val description: String? = null)
@@ -687,7 +741,7 @@ internal class XmlSerializerTest {
// }
}
internal fun assertSerializer(bean: Any, expected: String, filter: SerializationFilter?, description: String = "Serialization failure"): Element {
internal fun assertSerializer(bean: Any, expected: String, filter: SerializationFilter? = null, description: String = "Serialization failure"): Element {
val element = bean.serialize(filter, createElementIfEmpty = true)!!
assertThat(element).`as`(description).isEqualTo(expected)
return element
@@ -51,7 +51,7 @@ internal class UnsavedProtectedValue(val secureString: StringProtectedByStreamCi
override fun getText() = throw IllegalStateException("Must be converted to ProtectedValue for serialization")
}
internal class ProtectedXmlWriter(private val streamCipher: SkippingStreamCipher) : JbXmlOutputter("\n", null, null, null) {
internal class ProtectedXmlWriter(private val streamCipher: SkippingStreamCipher) : JbXmlOutputter(false) {
override fun writeContent(out: Writer, element: Element, level: Int): Boolean {
if (element.name == KdbxEntryElementNames.value) {
val value = element.content.firstOrNull()
@@ -8,7 +8,11 @@ import java.io.Writer
abstract class BaseXmlOutputter(protected val lineSeparator: String) {
companion object {
fun isSavePasswordField(name: String) = name.contains("remember", ignoreCase = true) || name.contains("keep", ignoreCase = true) || name.contains("save", ignoreCase = true)
fun isNameIndicatesSensitiveInformation(name: String): Boolean {
return name.contains("password") && !(name.contains("remember", ignoreCase = true) ||
name.contains("keep", ignoreCase = true) ||
name.contains("save", ignoreCase = true))
}
}
/**
@@ -9,6 +9,7 @@ import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.util.JDOMUtil;
import com.intellij.openapi.util.SystemInfoRt;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.util.xmlb.Constants;
import org.jdom.*;
import org.jdom.output.Format;
import org.jetbrains.annotations.NotNull;
@@ -35,21 +36,36 @@ public class JbXmlOutputter extends BaseXmlOutputter {
@Nullable
private final PathMacroFilter macroFilter;
private final boolean isForbidSensitiveData;
public JbXmlOutputter(boolean isForbidSensitiveData) {
this("\n", null, null, null, isForbidSensitiveData);
}
public JbXmlOutputter(@NotNull String lineSeparator,
@Nullable JDOMUtil.ElementOutputFilter elementFilter,
@Nullable ReplacePathToMacroMap macroMap,
@Nullable PathMacroFilter macroFilter) {
this(lineSeparator, elementFilter, macroMap, macroFilter, true);
}
private JbXmlOutputter(@NotNull String lineSeparator,
@Nullable JDOMUtil.ElementOutputFilter elementFilter,
@Nullable ReplacePathToMacroMap macroMap,
@Nullable PathMacroFilter macroFilter) {
@Nullable PathMacroFilter macroFilter,
boolean isForbidSensitiveData) {
super(lineSeparator);
this.format = DEFAULT_FORMAT.getLineSeparator().equals(lineSeparator) ? DEFAULT_FORMAT : JDOMUtil.createFormat(lineSeparator);
this.elementFilter = elementFilter;
this.macroMap = macroMap;
this.macroFilter = macroFilter;
this.isForbidSensitiveData = isForbidSensitiveData;
}
public static void collapseMacrosAndWrite(@NotNull Element element, @NotNull ComponentManager project, @NotNull Writer writer) throws IOException {
PathMacroManager macroManager = PathMacroManager.getInstance(project);
JbXmlOutputter xmlWriter = new JbXmlOutputter("\n", null, macroManager.getReplacePathMap(), macroManager.getMacroFilter());
JbXmlOutputter xmlWriter = new JbXmlOutputter("\n", null, macroManager.getReplacePathMap(), macroManager.getMacroFilter(), true);
xmlWriter.output(element, writer);
}
@@ -275,9 +291,25 @@ public class JbXmlOutputter extends BaseXmlOutputter {
out.write('>');
}
private static void checkIsElementContainsSensitiveInformation(@NotNull Element element) {
String name = element.getName();
if (BaseXmlOutputter.Companion.isNameIndicatesSensitiveInformation(name)) {
logSensitiveInformationError(name, "Element");
}
name = element.getAttributeValue(Constants.NAME);
if (name != null && BaseXmlOutputter.Companion.isNameIndicatesSensitiveInformation(name)) {
logSensitiveInformationError(name, "Element");
}
}
private static void logSensitiveInformationError(@NotNull String name, @NotNull String elementKind) {
Logger.getInstance(JbXmlOutputter.class).error(elementKind + " \"" + name + "\" probably contains sensitive information");
}
protected boolean writeContent(@NotNull Writer out, @NotNull Element element, int level) throws IOException {
if (element.getName().contains("password") && !BaseXmlOutputter.Companion.isSavePasswordField(element.getName())) {
Logger.getInstance(JbXmlOutputter.class).error("Element " + element.getName() + " probably contains sensitive information");
if (isForbidSensitiveData) {
checkIsElementContainsSensitiveInformation(element);
}
List<Content> content = element.getContent();
@@ -452,8 +484,8 @@ public class JbXmlOutputter extends BaseXmlOutputter {
value = attribute.getValue();
}
if (attribute.getName().contains("password") && !BaseXmlOutputter.Companion.isSavePasswordField(attribute.getName())) {
Logger.getInstance(JbXmlOutputter.class).error("Attribute " + attribute.getName() + " probably contains sensitive information");
if (isForbidSensitiveData && BaseXmlOutputter.Companion.isNameIndicatesSensitiveInformation(attribute.getName())) {
logSensitiveInformationError(attribute.getName(), "Attribute");
}
out.write(escapeAttributeEntities(value));