diff --git a/platform/configuration-store-impl/testSrc/DoNotStorePasswordTest.kt b/platform/configuration-store-impl/testSrc/DoNotStorePasswordTest.kt index 9aa91cd7e4d4..9344bd503717 100644 --- a/platform/configuration-store-impl/testSrc/DoNotStorePasswordTest.kt +++ b/platform/configuration-store-impl/testSrc/DoNotStorePasswordTest.kt @@ -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") diff --git a/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt b/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt index 1cd4ca08b134..936d28ace831 100644 --- a/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt +++ b/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt @@ -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()) } - 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("", bean, SkipDefaultsSerializationFilter()) + testSerializer("", 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, "") + + 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, """ + + + """.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 diff --git a/platform/credential-store/src/kdbx/ProtectedValue.kt b/platform/credential-store/src/kdbx/ProtectedValue.kt index e62ce9a24488..9bad55928026 100644 --- a/platform/credential-store/src/kdbx/ProtectedValue.kt +++ b/platform/credential-store/src/kdbx/ProtectedValue.kt @@ -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() diff --git a/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt b/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt index 942f455d0491..20f2b3179d74 100644 --- a/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt +++ b/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt @@ -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)) + } } /** diff --git a/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.java b/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.java index a1e1902ce6a7..23f6c49a74fe 100644 --- a/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.java +++ b/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.java @@ -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 = 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));