From 68a2fd45185d11a25b88027b60c79f99b1bde1cd Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Fri, 26 Aug 2016 13:43:10 +0200 Subject: [PATCH] IDEA-160341 secure implementation of joinData --- .../credential-store/src/credentialStore.kt | 20 ++++++++++++++++--- .../src/libraries/linuxSecretLibrary.kt | 7 +++++-- .../test/CredentialSerializeTest.kt | 2 +- .../credentialStore/CredentialAttributes.kt | 7 +++++++ .../openapi/util/text/StringUtil.java | 2 +- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/platform/credential-store/src/credentialStore.kt b/platform/credential-store/src/credentialStore.kt index 9cb6ac9298bd..316b38b8e874 100644 --- a/platform/credential-store/src/credentialStore.kt +++ b/platform/credential-store/src/credentialStore.kt @@ -17,6 +17,8 @@ package com.intellij.credentialStore import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.util.text.StringUtil +import org.jetbrains.io.toByteArray +import java.nio.CharBuffer import java.security.MessageDigest import java.util.* @@ -28,11 +30,23 @@ internal fun toOldKeyAsIdentity(hash: ByteArray) = CredentialAttributes("Intelli fun toOldKey(requestor: Class<*>, userName: String) = CredentialAttributes("IntelliJ Platform", toOldKey(MessageDigest.getInstance("SHA-256").digest("${requestor.name}/$userName".toByteArray()))) -fun joinData(user: String?, password: OneTimeString?): String? { +fun joinData(user: String?, password: OneTimeString?): ByteArray? { if (user == null && password == null) { return null } - return "${StringUtil.escapeChars(user.orEmpty(), '\\', '@')}${if (password == null) "" else "@$password"}" + + val builder = StringBuilder(user.orEmpty()) + StringUtil.escapeChar(builder, '\\') + StringUtil.escapeChar(builder, '@') + if (password != null) { + builder.append('@') + password.appendTo(builder) + } + + val buffer = Charsets.UTF_8.encode(CharBuffer.wrap(builder)) + // clear password + builder.setLength(0) + return buffer.toByteArray() } fun splitData(data: String?): Credentials? { @@ -78,4 +92,4 @@ private fun parseString(data: String, delimiter: Char): List { } // check isEmpty before -fun Credentials.serialize() = joinData(userName, password)!!.toByteArray() \ No newline at end of file +fun Credentials.serialize() = joinData(userName, password)!! \ No newline at end of file diff --git a/platform/credential-store/src/libraries/linuxSecretLibrary.kt b/platform/credential-store/src/libraries/linuxSecretLibrary.kt index ad8426bbe577..7dd5f59f3220 100644 --- a/platform/credential-store/src/libraries/linuxSecretLibrary.kt +++ b/platform/credential-store/src/libraries/linuxSecretLibrary.kt @@ -13,10 +13,13 @@ private const val SECRET_SCHEMA_NONE = 0 private const val SECRET_SCHEMA_ATTRIBUTE_STRING = 0 // explicitly create pointer to be explicitly dispose it to avoid sensitive data in the memory -internal fun stringPointer(data: ByteArray): DisposableMemory { +internal fun stringPointer(data: ByteArray, clearInput: Boolean = false): DisposableMemory { val pointer = DisposableMemory(data.size + 1L) pointer.write(0, data, 0, data.size) pointer.setByte(data.size.toLong(), 0.toByte()) + if (clearInput) { + data.fill(0) + } return pointer } @@ -74,7 +77,7 @@ internal class SecretCredentialStore(schemeName: String) : CredentialStore { return } - val passwordPointer = stringPointer(credentials!!.serialize()) + val passwordPointer = stringPointer(credentials!!.serialize(), true) checkError("secret_password_store_sync") { errorRef -> try { if (accountName == null) { diff --git a/platform/credential-store/test/CredentialSerializeTest.kt b/platform/credential-store/test/CredentialSerializeTest.kt index 1d5df051ae01..85f97a3b9f61 100644 --- a/platform/credential-store/test/CredentialSerializeTest.kt +++ b/platform/credential-store/test/CredentialSerializeTest.kt @@ -27,7 +27,7 @@ class CredentialSerializeTest { private fun test(u: String?, p: String?, joined: String) { val pass = p?.let(::OneTimeString) - assertThat(joinData(u, pass)).isEqualTo(joined) + assertThat(joinData(u, pass)).isEqualTo(joined.toByteArray()) assertThat(splitData(joined)).isEqualTo(Credentials(u, pass)) } } \ No newline at end of file diff --git a/platform/platform-api/src/com/intellij/credentialStore/CredentialAttributes.kt b/platform/platform-api/src/com/intellij/credentialStore/CredentialAttributes.kt index 292b66e3e42a..6add844d923b 100644 --- a/platform/platform-api/src/com/intellij/credentialStore/CredentialAttributes.kt +++ b/platform/platform-api/src/com/intellij/credentialStore/CredentialAttributes.kt @@ -127,4 +127,11 @@ class OneTimeString @JvmOverloads constructor(value: CharArray, offset: Int = 0, } return super.equals(other) } + + fun appendTo(builder: StringBuilder) { + if (consumed.get()) { + throw Error("Already consumed") + } + builder.append(myChars, myStart, length) + } } \ No newline at end of file diff --git a/platform/util/src/com/intellij/openapi/util/text/StringUtil.java b/platform/util/src/com/intellij/openapi/util/text/StringUtil.java index 45bfc407462a..60a758a6ae6d 100644 --- a/platform/util/src/com/intellij/openapi/util/text/StringUtil.java +++ b/platform/util/src/com/intellij/openapi/util/text/StringUtil.java @@ -2135,7 +2135,7 @@ public class StringUtil extends StringUtilRt { return buf.toString(); } - private static void escapeChar(@NotNull final StringBuilder buf, final char character) { + public static void escapeChar(@NotNull final StringBuilder buf, final char character) { int idx = 0; while ((idx = indexOf(buf, character, idx)) >= 0) { buf.insert(idx, "\\");