Skip to content

support e2e encryption #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

support e2e encryption #1

wants to merge 26 commits into from

Conversation

DelmoreCooper
Copy link
Contributor

No description provided.

@@ -35,7 +34,6 @@ internal fun SharedSessionEntity.Companion.get(
return realm.where<SharedSessionEntity>()
.equalTo(SharedSessionEntityFields.ROOM_ID, roomId)
.equalTo(SharedSessionEntityFields.SESSION_ID, sessionId)
.equalTo(SharedSessionEntityFields.ALGORITHM, MXCRYPTO_ALGORITHM_MEGOLM)
Copy link

@chzhongsending chzhongsending Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line changed its behavior, it will return any SharedSessionEntity regardless it is a MXCRYPTO_ALGORITHM_MEGOLM or MXCRYPTO_ALGORITHM_RATCHET.

Try to add a parameter algorithm ?

@@ -46,7 +44,6 @@ internal fun SharedSessionEntity.Companion.get(realm: Realm, roomId: String?, se
return realm.where<SharedSessionEntity>()
.equalTo(SharedSessionEntityFields.ROOM_ID, roomId)
.equalTo(SharedSessionEntityFields.SESSION_ID, sessionId)
.equalTo(SharedSessionEntityFields.ALGORITHM, MXCRYPTO_ALGORITHM_MEGOLM)
Copy link

@chzhongsending chzhongsending Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above and all similar issues below.

@@ -61,7 +58,6 @@ internal fun SharedSessionEntity.Companion.create(
): SharedSessionEntity {
return realm.createObject<SharedSessionEntity>().apply {
this.roomId = roomId
this.algorithm = MXCRYPTO_ALGORITHM_MEGOLM

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure removing this field is OK.

@@ -176,7 +177,7 @@ internal class CreateRoomBodyBuilder @Inject constructor(
}
return params.algorithm
?.let {
if (it != MXCRYPTO_ALGORITHM_MEGOLM) {
if (!arrayOf(MXCRYPTO_ALGORITHM_MEGOLM, MXCRYPTO_ALGORITHM_RATCHET).contains(it)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arrayOf(MXCRYPTO_ALGORITHM_MEGOLM, MXCRYPTO_ALGORITHM_RATCHET) occurs multiple times, consider extract it as a const or static final

@@ -57,12 +58,13 @@ internal class DefaultRoomCryptoService @AssistedInject constructor(
}

override suspend fun enableEncryption(algorithm: String, force: Boolean) {
val supportedAlgorithm = arrayOf(MXCRYPTO_ALGORITHM_MEGOLM, MXCRYPTO_ALGORITHM_RATCHET)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this val/var to upper level?

private const val cipherMode = "AES/CBC/PKCS7Padding" //algorithm/mode/padding

fun encrypt(key: ByteArray, cleartext: String): String {
Timber.i("AES encrypt with key: ${key.decodeToString()}")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should NEVER print key to log.

@@ -205,6 +205,8 @@ internal class DeviceListManager @Inject constructor(
}

if (isUpdated) {
Timber.v("## CRYPTO: removeCurrentGroupSession on device changed")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please confirm while isUpdated=true is equivalent to the scenarios mentioned by @auruac

@DelmoreCooper DelmoreCooper force-pushed the e2e branch 2 times, most recently from 8d4db95 to e43771d Compare April 29, 2024 12:28
Copy link

@Mazzika1 Mazzika1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

@DelmoreCooper DelmoreCooper force-pushed the e2e branch 6 times, most recently from 87711f8 to 3919a13 Compare July 17, 2024 06:54
@DelmoreCooper DelmoreCooper force-pushed the e2e branch 3 times, most recently from 88b0363 to aaa92d7 Compare July 17, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants