diff --git a/changelog.d/security_fixes.bugfix b/changelog.d/security_fixes.bugfix new file mode 100644 index 00000000000..429af15c5f2 --- /dev/null +++ b/changelog.d/security_fixes.bugfix @@ -0,0 +1 @@ +Comprehensive security hardening: E2EE decryption trust settings strengthened, WebView file access restricted, ImporterService export locked down, key material secure erasure improved, forwarded room key audit logging added, JJWT upgraded to 0.12.6 (CVE-2024-31033), SSO CSRF state validation added, key export encryption upgraded to AES-GCM, MD5 migrated to SHA-256 for database alias, and clipboard auto-clear after 30 seconds. \ No newline at end of file diff --git a/dependencies.gradle b/dependencies.gradle index 3eaedb3617d..d43e33f8c1a 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -23,7 +23,7 @@ def epoxy = "5.0.0" def mavericks = "3.0.9" def glide = "4.16.0" def bigImageViewer = "1.8.1" -def jjwt = "0.11.5" +def jjwt = "0.12.6" def vanniktechEmoji = "0.16.0" def sentry = "8.31.0" def fragment = "1.8.6" diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt index c6fab7762f3..b5c78d22169 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/auth/AuthenticationService.kt @@ -46,6 +46,13 @@ interface AuthenticationService { */ fun getSsoUrl(redirectUrl: String, deviceId: String?, providerId: String?, action: SSOAction): String? + /** + * Verify the SSO state parameter returned in the callback to prevent CSRF attacks. + * @param state the state parameter from the SSO callback URI. + * @return true if the state matches the one generated during [getSsoUrl], false otherwise. + */ + fun verifySsoState(state: String?): Boolean + /** * Get the sign in or sign up fallback URL. */ diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt index 6b9a06a6046..a423099fe72 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt @@ -74,6 +74,11 @@ internal class DefaultAuthenticationService @Inject constructor( private var currentLoginWizard: LoginWizard? = null private var currentRegistrationWizard: RegistrationWizard? = null + /** + * The current SSO state parameter, used to prevent CSRF attacks. + */ + private var currentSsoState: String? = null + override fun hasAuthenticatedSessions(): Boolean { return sessionParamsStore.getLast() != null } @@ -111,9 +116,20 @@ internal class DefaultAuthenticationService @Inject constructor( // stable param: appendParamToUrl("action", action.value) + + // CSRF protection: append a random state parameter + val state = java.util.UUID.randomUUID().toString() + currentSsoState = state + appendParamToUrl("state", state) } } + override fun verifySsoState(state: String?): Boolean { + val expected = currentSsoState + currentSsoState = null // One-time use + return expected != null && expected == state + } + override fun getFallbackUrl(forSignIn: Boolean, deviceId: String?): String? { val homeServerUrlBase = getHomeServerUrlBase() ?: return null diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXMegolmExportEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXMegolmExportEncryption.kt index e0d6c25d701..4f1da69396f 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXMegolmExportEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXMegolmExportEncryption.kt @@ -20,10 +20,12 @@ import android.util.Base64 import org.matrix.android.sdk.internal.extensions.toUnsignedInt import timber.log.Timber import java.io.ByteArrayOutputStream +import java.nio.ByteBuffer import java.nio.charset.Charset import java.security.SecureRandom import javax.crypto.Cipher import javax.crypto.Mac +import javax.crypto.spec.GCMParameterSpec import javax.crypto.spec.IvParameterSpec import javax.crypto.spec.SecretKeySpec import kotlin.experimental.and @@ -38,6 +40,10 @@ internal object MXMegolmExportEncryption { private const val HEADER_LINE = "-----BEGIN MEGOLM SESSION DATA-----" private const val TRAILER_LINE = "-----END MEGOLM SESSION DATA-----" + private const val EXPORT_VERSION_1 = 1 // AES-CTR + HMAC-SHA256 (legacy) + private const val EXPORT_VERSION_2 = 2 // AES-GCM (current) + private const val AES_KEY_LENGTH_BITS = 256 + // we split into lines before base64ing, because encodeBase64 doesn't deal // terribly well with large arrays. private const val LINE_LENGTH = 72 * 4 / 3 @@ -83,21 +89,33 @@ internal object MXMegolmExportEncryption { throw Exception("Invalid file: too short") } - val version = body[0] - if (version.toInt() != 1) { - Timber.e("## decryptMegolmKeyFile() : Invalid file: too short") - throw Exception("Unsupported version") + if (password.isEmpty()) { + throw Exception("Empty password is not supported") } + val version = body[0].toInt() + + return when (version) { + EXPORT_VERSION_2 -> decryptWithAesGcm(body, password) + EXPORT_VERSION_1 -> decryptLegacy(body, password) + else -> { + // Unknown version byte - assume legacy format (version 1) + Timber.w("## decryptMegolmKeyFile() : Unknown version $version, trying legacy decryption") + decryptLegacy(body, password) + } + } + } + + /** + * Decrypt using legacy AES-CTR + HMAC-SHA256 format (version 1). + */ + @Throws(Exception::class) + private fun decryptLegacy(body: ByteArray, password: String): String { val ciphertextLength = body.size - (1 + 16 + 16 + 4 + 32) if (ciphertextLength < 0) { throw Exception("Invalid file: too short") } - if (password.isEmpty()) { - throw Exception("Empty password is not supported") - } - val salt = body.copyOfRange(1, 1 + 16) val iv = body.copyOfRange(17, 17 + 16) val iterations = @@ -115,13 +133,14 @@ internal object MXMegolmExportEncryption { val digest = mac.doFinal(toVerify) if (!hmac.contentEquals(digest)) { - Timber.e("## decryptMegolmKeyFile() : Authentication check failed: incorrect password?") + Timber.e("## decryptLegacy() : Authentication check failed: incorrect password?") throw Exception("Authentication check failed: incorrect password?") } val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") - val secretKeySpec = SecretKeySpec(getAesKey(deriveKey), "AES") + val aesKey = getAesKey(deriveKey) + val secretKeySpec = SecretKeySpec(aesKey, "AES") val ivParameterSpec = IvParameterSpec(iv) decryptCipher.init(Cipher.DECRYPT_MODE, secretKeySpec, ivParameterSpec) @@ -132,11 +151,47 @@ internal object MXMegolmExportEncryption { val decodedString = String(outStream.toByteArray(), Charset.defaultCharset()) outStream.close() + // Securely wipe derived key material from memory + deriveKey.fill(0) + aesKey.fill(0) + return decodedString } + /** + * Decrypt using AES-GCM format (version 2). + * Format: version(1B) + salt(16B) + iv(12B) + iterations(4B) + ciphertext_with_tag + */ + @Throws(Exception::class) + private fun decryptWithAesGcm(packed: ByteArray, password: String): String { + // Minimum size: 1(version) + 16(salt) + 12(iv) + 4(iterations) + 16(tag) = 49 + if (packed.size < 49) { + throw Exception("Invalid file: too short for AES-GCM format") + } + + var offset = 1 // skip version byte + val salt = packed.copyOfRange(offset, offset + 16); offset += 16 + val iv = packed.copyOfRange(offset, offset + 12); offset += 12 + val iterations = ByteBuffer.wrap(packed.copyOfRange(offset, offset + 4)).int; offset += 4 + val ciphertext = packed.copyOfRange(offset, packed.size) + + val prf = Mac.getInstance("HmacSHA512") + val deriveKey = pbkdf2(password, salt, iterations, AES_KEY_LENGTH_BITS, prf) + + try { + val secretKey = SecretKeySpec(deriveKey, "AES") + val cipher = Cipher.getInstance("AES/GCM/NoPadding") + val gcmSpec = GCMParameterSpec(128, iv) + cipher.init(Cipher.DECRYPT_MODE, secretKey, gcmSpec) + return String(cipher.doFinal(ciphertext), Charsets.UTF_8) + } finally { + deriveKey.fill(0) + } + } + /** * Encrypt a string into the megolm export format. + * Uses AES-GCM (version 2) for authenticated encryption. * * @param data the data to encrypt. * @param password the password @@ -151,61 +206,41 @@ internal object MXMegolmExportEncryption { throw Exception("Empty password is not supported") } + val resultBuffer = encryptWithAesGcm(data.toByteArray(Charsets.UTF_8), password, kdfRounds) + return packMegolmKeyFile(resultBuffer) + } + + /** + * Encrypt data using AES-GCM (version 2). + * Output format: version(1B) + salt(16B) + iv(12B) + iterations(4B big-endian) + ciphertext_with_tag + */ + @Throws(Exception::class) + private fun encryptWithAesGcm(data: ByteArray, password: String, iterations: Int = DEFAULT_ITERATION_COUNT): ByteArray { val secureRandom = SecureRandom() val salt = ByteArray(16) secureRandom.nextBytes(salt) - val iv = ByteArray(16) + val iv = ByteArray(12) // GCM uses 12-byte IV secureRandom.nextBytes(iv) - // clear bit 63 of the salt to stop us hitting the 64-bit counter boundary - // (which would mean we wouldn't be able to decrypt on Android). The loss - // of a single bit of salt is a price we have to pay. - iv[9] = iv[9] and 0x7f - - val deriveKey = deriveKeys(salt, kdfRounds, password) - - val decryptCipher = Cipher.getInstance("AES/CTR/NoPadding") - - val secretKeySpec = SecretKeySpec(getAesKey(deriveKey), "AES") - val ivParameterSpec = IvParameterSpec(iv) - decryptCipher.init(Cipher.ENCRYPT_MODE, secretKeySpec, ivParameterSpec) - - val outStream = ByteArrayOutputStream() - outStream.write(decryptCipher.update(data.toByteArray(charset("UTF-8")))) - outStream.write(decryptCipher.doFinal()) - - val cipherArray = outStream.toByteArray() - val bodyLength = 1 + salt.size + iv.size + 4 + cipherArray.size + 32 - - val resultBuffer = ByteArray(bodyLength) - var idx = 0 - resultBuffer[idx++] = 1 // version - - System.arraycopy(salt, 0, resultBuffer, idx, salt.size) - idx += salt.size - - System.arraycopy(iv, 0, resultBuffer, idx, iv.size) - idx += iv.size - - resultBuffer[idx++] = (kdfRounds shr 24 and 0xff).toByte() - resultBuffer[idx++] = (kdfRounds shr 16 and 0xff).toByte() - resultBuffer[idx++] = (kdfRounds shr 8 and 0xff).toByte() - resultBuffer[idx++] = (kdfRounds and 0xff).toByte() - - System.arraycopy(cipherArray, 0, resultBuffer, idx, cipherArray.size) - idx += cipherArray.size - - val toSign = resultBuffer.copyOfRange(0, idx) - - val macKey = SecretKeySpec(getHmacKey(deriveKey), "HmacSHA256") - val mac = Mac.getInstance("HmacSHA256") - mac.init(macKey) - val digest = mac.doFinal(toSign) - System.arraycopy(digest, 0, resultBuffer, idx, digest.size) - - return packMegolmKeyFile(resultBuffer) + // Derive 256-bit key using PBKDF2 + val prf = Mac.getInstance("HmacSHA512") + val deriveKey = pbkdf2(password, salt, iterations, AES_KEY_LENGTH_BITS, prf) + + try { + val secretKey = SecretKeySpec(deriveKey, "AES") + val cipher = Cipher.getInstance("AES/GCM/NoPadding") + val gcmSpec = GCMParameterSpec(128, iv) // 128-bit auth tag + cipher.init(Cipher.ENCRYPT_MODE, secretKey, gcmSpec) + val ciphertext = cipher.doFinal(data) + + // Pack: version + salt + iv + iterations(big-endian 4 bytes) + ciphertext(includes tag) + val iterBytes = ByteBuffer.allocate(4).putInt(iterations).array() + return byteArrayOf(EXPORT_VERSION_2.toByte()) + salt + iv + iterBytes + ciphertext + } finally { + deriveKey.fill(0) + } } /** @@ -302,12 +337,12 @@ internal object MXMegolmExportEncryption { } /** - * Derive the AES and HMAC-SHA-256 keys for the file. + * Derive the AES and HMAC-SHA-256 keys for the file (legacy, 512-bit output). * * @param salt salt for pbkdf * @param iterations number of pbkdf iterations * @param password password - * @return the derived keys + * @return the derived keys (64 bytes: 32 AES + 32 HMAC) */ @Throws(Exception::class) private fun deriveKeys(salt: ByteArray, iterations: Int, password: String): ByteArray { @@ -348,4 +383,55 @@ internal object MXMegolmExportEncryption { return key } + + /** + * PBKDF2 key derivation with configurable output length. + * Uses HMAC-SHA512 as PRF. Output is truncated to the requested bit length. + * + * @param password the password + * @param salt the salt + * @param iterations number of iterations + * @param keyLengthBits desired key length in bits (must be <= 512) + * @param prf the HMAC instance (must be initialized externally or will be initialized here) + * @return the derived key bytes + */ + @Throws(Exception::class) + private fun pbkdf2(password: String, salt: ByteArray, iterations: Int, keyLengthBits: Int, prf: Mac): ByteArray { + require(keyLengthBits in 1..512) { "Key length must be between 1 and 512 bits" } + + val keyLengthBytes = keyLengthBits / 8 + val fullKey = ByteArray(64) + + measureTimeMillis { + prf.init(SecretKeySpec(password.toByteArray(Charsets.UTF_8), "HmacSHA512")) + + val uc = ByteArray(64) + + // U1 = PRF(Password, Salt || INT_32_BE(1)) + prf.update(salt) + val int32BE = ByteArray(4) { 0.toByte() } + int32BE[3] = 1.toByte() + prf.update(int32BE) + prf.doFinal(uc, 0) + + // copy to the key + System.arraycopy(uc, 0, fullKey, 0, uc.size) + + for (index in 2..iterations) { + // Uc = PRF(Password, Uc-1) + prf.update(uc) + prf.doFinal(uc, 0) + + // F(Password, Salt, c, i) = U1 ^ U2 ^ ... ^ Uc + for (byteIndex in uc.indices) { + fullKey[byteIndex] = fullKey[byteIndex] xor uc[byteIndex] + } + } + }.also { + Timber.v("## pbkdf2() : $iterations iterations in $it ms") + } + + // Truncate to requested key length + return fullKey.copyOfRange(0, keyLengthBytes) + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt index 2b68e228b9c..d795291de73 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt @@ -293,7 +293,7 @@ internal class OlmMachine @Inject constructor( keyCounts = counts, unusedFallbackKeys = deviceUnusedFallbackKeyTypes, nextBatchToken = nextBatch ?: "", - decryptionSettings = DecryptionSettings(TrustRequirement.UNTRUSTED), + decryptionSettings = DecryptionSettings(TrustRequirement.CROSS_SIGNED_OR_LEGACY), ) val outAdapter = moshi.adapter(Event::class.java) @@ -462,8 +462,8 @@ internal class OlmMachine @Inject constructor( val decrypted = inner.decryptRoomEvent( serializedEvent, event.roomId, handleVerificationEvents = false, - strictShields = false, - decryptionSettings = DecryptionSettings(TrustRequirement.UNTRUSTED) + strictShields = true, + decryptionSettings = DecryptionSettings(TrustRequirement.CROSS_SIGNED_OR_LEGACY) ) val deserializationAdapter = diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt index 01327cce86e..65f77f0779c 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt @@ -665,6 +665,9 @@ internal class RustCryptoService @Inject constructor( val roomId = content.roomId val sessionId = content.sessionId ?: return@forEach + // Security: Log forwarded key reception for audit trail + Timber.d("Received forwarded room key for room=${roomId?.take(8)}*** session=${sessionId.take(8)}*** from sender=${event.senderId?.take(8)}***") + notifyRoomKeyReceived(roomId, sessionId) matrixConfiguration.cryptoAnalyticsPlugin?.onRoomKeyImported(sessionId, EventType.FORWARDED_ROOM_KEY) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustEncryptionConfiguration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustEncryptionConfiguration.kt index f86e76b78e3..5ba46e79782 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustEncryptionConfiguration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/RustEncryptionConfiguration.kt @@ -20,6 +20,7 @@ import org.matrix.android.sdk.api.util.toBase64NoPadding import org.matrix.android.sdk.internal.database.RealmKeysUtils import org.matrix.android.sdk.internal.di.UserMd5 import org.matrix.android.sdk.internal.session.SessionScope +import java.security.MessageDigest import javax.inject.Inject @SessionScope @@ -28,8 +29,23 @@ internal class RustEncryptionConfiguration @Inject constructor( private val realmKeyUtil: RealmKeysUtils, ) { + // New SHA-256 based alias derived from the legacy MD5 hash + private val userSha256: String by lazy { + val digest = MessageDigest.getInstance("SHA-256") + digest.update(userMd5.toByteArray()) + digest.digest().joinToString("") { "%02x".format(it) } + } + + private val legacyAlias = "crypto_module_rust_$userMd5" + private val newAlias: String get() = "crypto_module_rust_sha256_$userSha256" + fun getDatabasePassphrase(): String { - // let's reuse the code for realm that creates a random 64 bytes array. - return realmKeyUtil.getRealmEncryptionKey("crypto_module_rust_$userMd5").toBase64NoPadding() + // Migration strategy: if a key exists under the legacy MD5 alias, keep using it + // for backward compatibility. New installations will use the SHA-256 alias. + return if (realmKeyUtil.hasRealmEncryptionKey(legacyAlias)) { + realmKeyUtil.getRealmEncryptionKey(legacyAlias).toBase64NoPadding() + } else { + realmKeyUtil.getRealmEncryptionKey(newAlias).toBase64NoPadding() + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt index 86355ceaa84..5eafb2292f4 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmKeysUtils.kt @@ -75,6 +75,9 @@ internal class RealmKeysUtils @Inject constructor( sharedPreferences.edit { putString("${ENCRYPTED_KEY_PREFIX}_$alias", Base64.encodeToString(toStore, Base64.NO_PADDING)) } + // Securely wipe intermediate key material from memory + // Note: encodedKey is a String and cannot be securely erased in JVM + toStore.fill(0) return key } @@ -86,7 +89,11 @@ internal class RealmKeysUtils @Inject constructor( val encryptedB64 = sharedPreferences.getString("${ENCRYPTED_KEY_PREFIX}_$alias", null) val encryptedKey = Base64.decode(encryptedB64, Base64.NO_PADDING) val b64 = secretStoringUtils.loadSecureSecretBytes(encryptedKey, alias) - return Base64.decode(b64, Base64.NO_PADDING) + val key = Base64.decode(b64, Base64.NO_PADDING) + // Securely wipe intermediate key material from memory + encryptedKey.fill(0) + b64.fill(0) + return key } fun configureEncryption(realmConfigurationBuilder: RealmConfiguration.Builder, alias: String) { @@ -95,6 +102,13 @@ internal class RealmKeysUtils @Inject constructor( realmConfigurationBuilder.encryptionKey(key) } + /** + * Check if a key for the given alias already exists. + */ + fun hasRealmEncryptionKey(alias: String): Boolean { + return hasKeyForDatabase(alias) + } + // Expose to handle Realm migration to riotX fun getRealmEncryptionKey(alias: String): ByteArray { val key = if (hasKeyForDatabase(alias)) { diff --git a/vector/src/main/AndroidManifest.xml b/vector/src/main/AndroidManifest.xml index b035f866fe3..fc6bbe611b4 100644 --- a/vector/src/main/AndroidManifest.xml +++ b/vector/src/main/AndroidManifest.xml @@ -417,8 +417,7 @@ + android:exported="false" /> diff --git a/vector/src/main/java/im/vector/app/core/utils/CopyToClipboardUseCase.kt b/vector/src/main/java/im/vector/app/core/utils/CopyToClipboardUseCase.kt index 8a4181b07bc..3df52d0cf13 100644 --- a/vector/src/main/java/im/vector/app/core/utils/CopyToClipboardUseCase.kt +++ b/vector/src/main/java/im/vector/app/core/utils/CopyToClipboardUseCase.kt @@ -10,6 +10,9 @@ package im.vector.app.core.utils import android.content.ClipData import android.content.ClipboardManager import android.content.Context +import android.os.Build +import android.os.Handler +import android.os.Looper import androidx.core.content.getSystemService import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject @@ -18,8 +21,25 @@ class CopyToClipboardUseCase @Inject constructor( @ApplicationContext private val context: Context, ) { + private val handler = Handler(Looper.getMainLooper()) + private val clearClipboardRunnable = Runnable { + val clipboard = context.getSystemService() ?: return@Runnable + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + clipboard.clearPrimaryClip() + } else { + clipboard.setPrimaryClip(ClipData.newPlainText("", "")) + } + } + fun execute(text: CharSequence) { - context.getSystemService() - ?.setPrimaryClip(ClipData.newPlainText("", text)) + val clipboard = context.getSystemService() ?: return + clipboard.setPrimaryClip(ClipData.newPlainText("", text)) + // Cancel any previous clear task and schedule a new one from the latest copy time + handler.removeCallbacks(clearClipboardRunnable) + handler.postDelayed(clearClipboardRunnable, CLIPBOARD_CLEAR_DELAY_MS) + } + + companion object { + private const val CLIPBOARD_CLEAR_DELAY_MS = 30_000L } } diff --git a/vector/src/main/java/im/vector/app/features/call/conference/jwt/JitsiJWTFactory.kt b/vector/src/main/java/im/vector/app/features/call/conference/jwt/JitsiJWTFactory.kt index 83a06ba19d8..2f36d1f7433 100644 --- a/vector/src/main/java/im/vector/app/features/call/conference/jwt/JitsiJWTFactory.kt +++ b/vector/src/main/java/im/vector/app/features/call/conference/jwt/JitsiJWTFactory.kt @@ -9,9 +9,7 @@ package im.vector.app.features.call.conference.jwt import im.vector.app.core.utils.ensureProtocol import io.jsonwebtoken.Jwts -import io.jsonwebtoken.SignatureAlgorithm import io.jsonwebtoken.io.Encoders -import io.jsonwebtoken.security.Keys import org.matrix.android.sdk.api.session.openid.OpenIdToken import javax.crypto.Mac import javax.crypto.spec.SecretKeySpec @@ -35,8 +33,7 @@ class JitsiJWTFactory @Inject constructor() { // we cannot use random secret keys anymore. But the JWT library `jjwt` doesn't accept the hardcoded key `notused` // from the module `prosody-mod-auth-matrix-user-verification` since it's too short and thus insecure. So, we // create a new token using a random key and then re-sign the token manually with the 'weak' key. - val signatureAlgorithm = SignatureAlgorithm.HS256 - val key = Keys.secretKeyFor(signatureAlgorithm) + val key = Jwts.SIG.HS256.key().build() val context = mapOf( "matrix" to mapOf( "token" to openIdToken.accessToken, @@ -52,10 +49,10 @@ class JitsiJWTFactory @Inject constructor() { // JWT generating side and Prosody config. Since we have no configuration for // the widgets, we can't set one anywhere. Using the Jitsi domain here probably makes sense. val token = Jwts.builder() - .setHeaderParam("typ", "JWT") - .setIssuer(jitsiServerDomain) - .setSubject(jitsiServerDomain) - .setAudience(jitsiServerDomain.ensureProtocol()) + .header().type("JWT").and() + .issuer(jitsiServerDomain) + .subject(jitsiServerDomain) + .audience().add(jitsiServerDomain.ensureProtocol()).and() // room is not used at the moment, a * works here. .claim("room", "*") .claim("context", context) @@ -63,7 +60,7 @@ class JitsiJWTFactory @Inject constructor() { .compact() // Re-sign token with the hardcoded key val toSign = token.substring(0, token.lastIndexOf('.')) - val mac = Mac.getInstance(signatureAlgorithm.jcaName) + val mac = Mac.getInstance("HmacSHA256") mac.init(SecretKeySpec("notused".toByteArray(), mac.algorithm)) val prosodySignature = Encoders.BASE64URL.encode(mac.doFinal(toSign.toByteArray())) return "$toSign.$prosodySignature" diff --git a/vector/src/main/java/im/vector/app/features/login/LoginActivity.kt b/vector/src/main/java/im/vector/app/features/login/LoginActivity.kt index 2419ba7bfc1..a0f0c6360e3 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginActivity.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginActivity.kt @@ -43,6 +43,7 @@ import org.matrix.android.sdk.api.auth.registration.FlowResult import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.auth.toLocalizedLoginTerms import org.matrix.android.sdk.api.extensions.tryOrNull +import timber.log.Timber /** * The LoginActivity manages the fragment navigation and also display the loading View. @@ -308,9 +309,15 @@ open class LoginActivity : VectorBaseActivity(), UnlockedA override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) - intent.data - ?.let { tryOrNull { it.getQueryParameter("loginToken") } } - ?.let { loginViewModel.handle(LoginAction.LoginWithToken(it)) } + intent.data?.let { uri -> + val returnedState = tryOrNull { uri.getQueryParameter("state") } + if (!loginViewModel.verifySsoState(returnedState)) { + Timber.e("SSO state mismatch, possible CSRF attack. Got=$returnedState") + return + } + tryOrNull { uri.getQueryParameter("loginToken") } + ?.let { loginViewModel.handle(LoginAction.LoginWithToken(it)) } + } } @Suppress("OVERRIDE_DEPRECATION") diff --git a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt index 60a45b4ac31..598aa239c54 100644 --- a/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/login/LoginViewModel.kt @@ -844,6 +844,10 @@ class LoginViewModel @AssistedInject constructor( return authenticationService.getSsoUrl(redirectUrl, deviceId, providerId, action) } + fun verifySsoState(state: String?): Boolean { + return authenticationService.verifySsoState(state) + } + fun getFallbackUrl(forSignIn: Boolean, deviceId: String?): String? { return authenticationService.getFallbackUrl(forSignIn, deviceId) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index d81d55da477..579f68556a8 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -838,6 +838,10 @@ class OnboardingViewModel @AssistedInject constructor( return authenticationService.getSsoUrl(redirectUrl, deviceId, provider?.id, action) } + fun verifySsoState(state: String?): Boolean { + return authenticationService.verifySsoState(state) + } + fun getFallbackUrl(forSignIn: Boolean, deviceId: String?): String? { return authenticationService.getFallbackUrl(forSignIn, deviceId) } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index b6d73676565..a82c01fd0de 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -51,6 +51,7 @@ import im.vector.lib.strings.CommonStrings import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.auth.toLocalizedLoginTerms import org.matrix.android.sdk.api.extensions.tryOrNull +import timber.log.Timber private const val FRAGMENT_REGISTRATION_STAGE_TAG = "FRAGMENT_REGISTRATION_STAGE_TAG" private const val FRAGMENT_LOGIN_TAG = "FRAGMENT_LOGIN_TAG" @@ -373,9 +374,15 @@ class FtueAuthVariant( * Handle the SSO redirection here. */ override fun onNewIntent(intent: Intent?) { - intent?.data - ?.let { tryOrNull { it.getQueryParameter("loginToken") } } - ?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) } + intent?.data?.let { uri -> + val returnedState = tryOrNull { uri.getQueryParameter("state") } + if (!onboardingViewModel.verifySsoState(returnedState)) { + Timber.e("SSO state mismatch, possible CSRF attack. Got=$returnedState") + return + } + tryOrNull { uri.getQueryParameter("loginToken") } + ?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) } + } } private fun doStage(stage: Stage) { diff --git a/vector/src/main/java/im/vector/app/features/webview/VectorWebViewActivity.kt b/vector/src/main/java/im/vector/app/features/webview/VectorWebViewActivity.kt index 6e7cb9e4684..d36fd59b67b 100644 --- a/vector/src/main/java/im/vector/app/features/webview/VectorWebViewActivity.kt +++ b/vector/src/main/java/im/vector/app/features/webview/VectorWebViewActivity.kt @@ -56,9 +56,9 @@ class VectorWebViewActivity : VectorBaseActivity() domStorageEnabled = true @Suppress("DEPRECATION") - allowFileAccessFromFileURLs = true + allowFileAccessFromFileURLs = false @Suppress("DEPRECATION") - allowUniversalAccessFromFileURLs = true + allowUniversalAccessFromFileURLs = false displayZoomControls = false } diff --git a/vector/src/main/java/im/vector/app/features/widgets/webview/WidgetWebView.kt b/vector/src/main/java/im/vector/app/features/widgets/webview/WidgetWebView.kt index 657f63cea7e..f71643805cc 100644 --- a/vector/src/main/java/im/vector/app/features/widgets/webview/WidgetWebView.kt +++ b/vector/src/main/java/im/vector/app/features/widgets/webview/WidgetWebView.kt @@ -43,9 +43,9 @@ fun WebView.setupForWidget(activity: Activity, settings.domStorageEnabled = true @Suppress("DEPRECATION") - settings.allowFileAccessFromFileURLs = true + settings.allowFileAccessFromFileURLs = false @Suppress("DEPRECATION") - settings.allowUniversalAccessFromFileURLs = true + settings.allowUniversalAccessFromFileURLs = false settings.displayZoomControls = false