-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implement e2ee for locations & journeys #133
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,16 @@ data class ApiLocation( | |
val created_at: Long? = System.currentTimeMillis() | ||
) | ||
|
||
@Keep | ||
@JsonClass(generateAdapter = true) | ||
data class EncryptedApiLocation( | ||
val id: String = UUID.randomUUID().toString(), | ||
val user_id: String = "", | ||
val encrypted_latitude: String = "", // Base64 encoded encrypted latitude | ||
val encrypted_longitude: String = "", // Base64 encoded encrypted longitude | ||
val created_at: Long? = System.currentTimeMillis() | ||
) | ||
|
||
fun ApiLocation.toLocation() = android.location.Location("").apply { | ||
latitude = [email protected] | ||
longitude = [email protected] | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,6 +10,7 @@ data class ApiSpace( | |||||||||||||||||||||
val id: String = UUID.randomUUID().toString(), | ||||||||||||||||||||||
val admin_id: String = "", | ||||||||||||||||||||||
val name: String = "", | ||||||||||||||||||||||
val encryptedSenderKeys: Map<String, Map<String, String>> = emptyMap(), // User-specific encrypted keys | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Document key format and add validation for encrypted sender keys The
- val encryptedSenderKeys: Map<String, Map<String, String>> = emptyMap(), // User-specific encrypted keys
+ /**
+ * User-specific encrypted keys for E2EE message exchange.
+ * Outer key: User ID
+ * Inner key: Key type (e.g., "preKey", "identityKey")
+ * Value: Base64 encoded encrypted key material
+ * @see SignalKeys for key types and formats
+ */
+ @Size(max = 1000) // Limit total number of users
+ val encryptedSenderKeys: Map<String, @Size(max = 5) Map<String, @Size(max = 64) String>> = emptyMap(), 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
val created_at: Long? = System.currentTimeMillis() | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,9 @@ package com.canopas.yourspace.data.models.user | |
import androidx.annotation.Keep | ||
import com.google.firebase.firestore.Exclude | ||
import com.squareup.moshi.JsonClass | ||
import org.signal.libsignal.protocol.IdentityKeyPair | ||
import org.signal.libsignal.protocol.state.PreKeyRecord | ||
import org.signal.libsignal.protocol.state.SignedPreKeyRecord | ||
import java.util.UUID | ||
|
||
const val LOGIN_TYPE_GOOGLE = 1 | ||
|
@@ -24,7 +27,12 @@ data class ApiUser( | |
val state: Int = USER_STATE_UNKNOWN, | ||
val battery_pct: Float? = 0f, | ||
val created_at: Long? = System.currentTimeMillis(), | ||
val updated_at: Long? = System.currentTimeMillis() | ||
val updated_at: Long? = System.currentTimeMillis(), | ||
val public_key: String? = null, // Identity public key (Base64-encoded) | ||
val private_key: String? = null, // Identity private key (Base64-encoded and encrypted) | ||
val pre_keys: List<String>? = null, // List of serialized PreKeys (Base64-encoded) | ||
val signed_pre_key: String? = null, // Serialized Signed PreKey (Base64-encoded) | ||
val registration_id: Int = 0 // Signal Protocol registration ID | ||
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review security implications of storing Signal Protocol keys Storing private keys and pre-keys in the user object raises security concerns:
Consider moving sensitive key material to secure storage and keeping only public information in the user object: - val private_key: String? = null, // Identity private key (Base64-encoded and encrypted)
- val pre_keys: List<String>? = null, // List of serialized PreKeys (Base64-encoded)
- val signed_pre_key: String? = null, // Serialized Signed PreKey (Base64-encoded)
+ // Reference to securely stored keys
+ val keyStoreId: String? = null,
|
||
) { | ||
@get:Exclude | ||
val fullName: String get() = "$first_name $last_name" | ||
|
@@ -39,6 +47,15 @@ data class ApiUser( | |
val locationPermissionDenied: Boolean get() = state == USER_STATE_LOCATION_PERMISSION_DENIED | ||
} | ||
|
||
@Keep | ||
@JsonClass(generateAdapter = false) | ||
data class SignalKeys( | ||
val identityKeyPair: IdentityKeyPair, | ||
val signedPreKey: SignedPreKeyRecord, | ||
val preKeys: List<PreKeyRecord>, | ||
val registrationId: Int | ||
) | ||
Comment on lines
+52
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add security measures for Signal Protocol key handling The
+/**
+ * Internal class for handling Signal Protocol keys.
+ * WARNING: Contains sensitive cryptographic material that must be handled securely.
+ * Keys should be generated using SignalKeyHelper and stored in secure storage.
+ */
+internal
data class SignalKeys(
val identityKeyPair: IdentityKeyPair,
val signedPreKey: SignedPreKeyRecord,
val preKeys: List<PreKeyRecord>,
val registrationId: Int
-)
+) {
+ // Ensure proper cleanup of sensitive data
+ protected fun finalize() {
+ // Clear sensitive fields
+ }
+}
|
||
|
||
@Keep | ||
@JsonClass(generateAdapter = true) | ||
data class ApiUserSession( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.canopas.yourspace.data.security.entity | ||
|
||
import org.signal.libsignal.protocol.SignalProtocolAddress | ||
|
||
abstract class BaseEncryptedEntity protected constructor( | ||
val registrationId: Int, | ||
val signalProtocolAddress: SignalProtocolAddress | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.canopas.yourspace.data.security.helper | ||
|
||
import android.util.Base64 | ||
|
||
object Helper { | ||
fun encodeToBase64(value: ByteArray?): String { | ||
return Base64.encodeToString(value, Base64.NO_WRAP) | ||
} | ||
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add proper error handling and security measures The current implementation has several security and robustness concerns:
Here's a suggested implementation with proper error handling: - fun encodeToBase64(value: ByteArray?): String {
- return Base64.encodeToString(value, Base64.NO_WRAP)
- }
+ fun encodeToBase64(value: ByteArray?): String {
+ try {
+ if (value == null) return ""
+ if (value.size > MAX_INPUT_SIZE) {
+ throw IllegalArgumentException("Input size exceeds maximum allowed size")
+ }
+ return Base64.encodeToString(value, Base64.NO_WRAP)
+ } catch (e: Exception) {
+ throw SecurityException("Failed to encode data", e)
+ } finally {
+ // Clear sensitive data from memory
+ value?.fill(0)
+ }
+ }
+
+ private const val MAX_INPUT_SIZE = 10 * 1024 * 1024 // 10MB limit
|
||
|
||
fun decodeToByteArray(base64: String?): ByteArray { | ||
return Base64.decode(base64, Base64.NO_WRAP) | ||
} | ||
Comment on lines
+10
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance input validation and error handling for secure decoding The current implementation has several security and robustness concerns:
Here's a suggested implementation with proper security measures: - fun decodeToByteArray(base64: String?): ByteArray {
- return Base64.decode(base64, Base64.NO_WRAP)
- }
+ fun decodeToByteArray(base64: String?): ByteArray {
+ try {
+ if (base64.isNullOrEmpty()) return ByteArray(0)
+
+ // Validate base64 format
+ if (!base64.matches(BASE64_PATTERN)) {
+ throw IllegalArgumentException("Invalid Base64 format")
+ }
+
+ return Base64.decode(base64, Base64.NO_WRAP)
+ } catch (e: Exception) {
+ throw SecurityException("Failed to decode data", e)
+ }
+ }
+
+ private val BASE64_PATTERN =
+ "^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)?$".toRegex()
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,192 @@ | ||
package com.canopas.yourspace.data.security.helper | ||
|
||
import android.util.Base64 | ||
import com.canopas.yourspace.data.models.user.ApiUser | ||
import com.canopas.yourspace.data.models.user.SignalKeys | ||
import org.signal.libsignal.protocol.IdentityKey | ||
import org.signal.libsignal.protocol.IdentityKeyPair | ||
import org.signal.libsignal.protocol.InvalidKeyException | ||
import org.signal.libsignal.protocol.SignalProtocolAddress | ||
import org.signal.libsignal.protocol.ecc.Curve | ||
import org.signal.libsignal.protocol.groups.GroupSessionBuilder | ||
import org.signal.libsignal.protocol.groups.state.SenderKeyRecord | ||
import org.signal.libsignal.protocol.state.PreKeyRecord | ||
import org.signal.libsignal.protocol.state.SignedPreKeyRecord | ||
import org.signal.libsignal.protocol.state.impl.InMemorySignalProtocolStore | ||
import org.signal.libsignal.protocol.util.KeyHelper | ||
import org.signal.libsignal.protocol.util.Medium | ||
import java.util.LinkedList | ||
import java.util.Random | ||
import java.util.UUID | ||
import javax.crypto.Cipher | ||
import javax.crypto.KeyGenerator | ||
import javax.crypto.SecretKey | ||
import javax.crypto.spec.SecretKeySpec | ||
import javax.inject.Inject | ||
import javax.inject.Singleton | ||
|
||
@Singleton | ||
class SignalKeyHelper @Inject constructor() { | ||
|
||
private fun generateIdentityKeyPair(): IdentityKeyPair { | ||
val djbKeyPair = Curve.generateKeyPair() | ||
val djbIdentityKey = IdentityKey(djbKeyPair.publicKey) | ||
val djbPrivateKey = djbKeyPair.privateKey | ||
|
||
return IdentityKeyPair(djbIdentityKey, djbPrivateKey) | ||
} | ||
|
||
@Throws(InvalidKeyException::class) | ||
fun generateSignedPreKey( | ||
identityKeyPair: IdentityKeyPair, | ||
signedPreKeyId: Int | ||
): SignedPreKeyRecord { | ||
val keyPair = Curve.generateKeyPair() | ||
val signature = | ||
Curve.calculateSignature(identityKeyPair.privateKey, keyPair.publicKey.serialize()) | ||
return SignedPreKeyRecord(signedPreKeyId, System.currentTimeMillis(), keyPair, signature) | ||
} | ||
|
||
private fun generatePreKeys(): List<PreKeyRecord> { | ||
val records: MutableList<PreKeyRecord> = LinkedList() | ||
val preKeyIdOffset = Random().nextInt(Medium.MAX_VALUE - 101) | ||
for (i in 0 until 100) { | ||
val preKeyId = (preKeyIdOffset + i) % Medium.MAX_VALUE | ||
Comment on lines
+52
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use SecureRandom for Cryptographic Operations Using Apply this diff to use -import java.util.Random
+import java.security.SecureRandom
...
val preKeyIdOffset = SecureRandom().nextInt(Medium.MAX_VALUE - 101)
...
val signedPreKey = generateSignedPreKey(identityKeyPair, SecureRandom().nextInt(Medium.MAX_VALUE - 1)) Also applies to: 66-66 |
||
val keyPair = Curve.generateKeyPair() | ||
val record = PreKeyRecord(preKeyId, keyPair) | ||
|
||
records.add(record) | ||
} | ||
|
||
return records | ||
} | ||
|
||
fun generateSignalKeys(): SignalKeys { | ||
val identityKeyPair = generateIdentityKeyPair() | ||
val signedPreKey = generateSignedPreKey(identityKeyPair, Random().nextInt(Medium.MAX_VALUE - 1)) | ||
val preKeys = generatePreKeys() | ||
val registrationId = KeyHelper.generateRegistrationId(false) | ||
|
||
return SignalKeys( | ||
identityKeyPair = identityKeyPair, | ||
signedPreKey = signedPreKey, | ||
preKeys = preKeys, | ||
registrationId = registrationId | ||
) | ||
} | ||
|
||
fun createDistributionKey( | ||
user: ApiUser, | ||
deviceId: String, | ||
spaceId: String | ||
): Pair<String, String> { | ||
val signalProtocolAddress = SignalProtocolAddress(user.id, deviceId.hashCode()) | ||
val identityKeyPair = IdentityKeyPair( | ||
IdentityKey(Curve.decodePoint(Base64.decode(user.public_key, Base64.DEFAULT), 0)), | ||
Curve.decodePrivatePoint(Base64.decode(user.private_key, Base64.DEFAULT)) | ||
) | ||
val signalProtocolStore = InMemorySignalProtocolStore(identityKeyPair, user.registration_id) | ||
val signedPreKeyId = SignedPreKeyRecord(Helper.decodeToByteArray(user.signed_pre_key)).id | ||
val preKeys = SignedPreKeyRecord(Helper.decodeToByteArray(user.signed_pre_key)) | ||
signalProtocolStore.storeSignedPreKey(signedPreKeyId, preKeys) | ||
|
||
user.pre_keys?.let { preKeyRecords -> | ||
val deserializedPreKeys = | ||
preKeyRecords.map { PreKeyRecord(Helper.decodeToByteArray(it)) } | ||
for (record in deserializedPreKeys) { | ||
signalProtocolStore.storePreKey(record.id, record) | ||
} | ||
} | ||
val validSpaceId = try { | ||
UUID.fromString(spaceId) // Validate if it's a proper UUID string | ||
} catch (e: IllegalArgumentException) { | ||
UUID.randomUUID() // Fallback to a new valid UUID if parsing fails | ||
} | ||
signalProtocolStore.storeSenderKey( | ||
Comment on lines
+103
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid Swallowing Exceptions Without Handling The caught Apply this diff to log the exception: } catch (e: IllegalArgumentException) {
+ Timber.e(e, "Invalid UUID format for spaceId: $spaceId. Generating a new UUID.")
UUID.randomUUID() // Fallback to a new valid UUID if parsing fails
}
|
||
signalProtocolAddress, | ||
validSpaceId, | ||
SenderKeyRecord(Helper.decodeToByteArray(user.signed_pre_key)) | ||
) | ||
|
||
val sessionBuilder = GroupSessionBuilder(signalProtocolStore) | ||
val senderKeyDistributionMessage = | ||
sessionBuilder.create(signalProtocolAddress, validSpaceId) | ||
val senderKeyRecord = signalProtocolStore.loadSenderKey(signalProtocolAddress, validSpaceId) | ||
|
||
return Pair( | ||
Helper.encodeToBase64(senderKeyDistributionMessage.serialize()), | ||
Helper.encodeToBase64(senderKeyRecord.serialize()) | ||
) | ||
} | ||
|
||
private fun encryptAESKeyWithECDH( | ||
aesKey: SecretKey, | ||
publicKey: String, | ||
senderPrivateKey: String | ||
): String { | ||
val ecPublicKey = Curve.decodePoint(Base64.decode(publicKey, Base64.DEFAULT), 0) | ||
val ecPrivateKey = Curve.decodePrivatePoint(Base64.decode(senderPrivateKey, Base64.DEFAULT)) | ||
val sharedSecret = Curve.calculateAgreement(ecPublicKey, ecPrivateKey) | ||
val secretKeySpec = SecretKeySpec(sharedSecret, 0, 32, "AES") | ||
val cipher = Cipher.getInstance("AES") | ||
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec) | ||
val encryptedAESKey = cipher.doFinal(aesKey.encoded) | ||
|
||
return Base64.encodeToString(encryptedAESKey, Base64.NO_WRAP) | ||
} | ||
|
||
private fun decryptAESKeyWithECDH( | ||
encryptedAESKey: String, | ||
privateKey: String, | ||
senderPublicKey: String | ||
): SecretKey { | ||
val ecPublicKey = Curve.decodePoint(Base64.decode(senderPublicKey, Base64.DEFAULT), 0) | ||
val ecPrivateKey = Curve.decodePrivatePoint(Base64.decode(privateKey, Base64.DEFAULT)) | ||
val sharedSecret = Curve.calculateAgreement(ecPublicKey, ecPrivateKey) | ||
val secretKeySpec = SecretKeySpec(sharedSecret, 0, 32, "AES") | ||
val cipher = Cipher.getInstance("AES") | ||
cipher.init(Cipher.DECRYPT_MODE, secretKeySpec) | ||
val decryptedAESKeyBytes = cipher.doFinal(Base64.decode(encryptedAESKey, Base64.DEFAULT)) | ||
|
||
return SecretKeySpec(decryptedAESKeyBytes, "AES") | ||
} | ||
|
||
fun encryptSenderKeyForGroup( | ||
senderKey: String, | ||
senderPrivateKey: String, | ||
recipients: List<ApiUser?> | ||
): Map<String, Pair<String, String>> { | ||
val encryptedKeys = mutableMapOf<String, Pair<String, String>>() | ||
val keyGen = KeyGenerator.getInstance("AES") | ||
val aesKey: SecretKey = keyGen.generateKey() | ||
val cipher = Cipher.getInstance("AES") | ||
cipher.init(Cipher.ENCRYPT_MODE, aesKey) | ||
val encryptedSenderKey = | ||
Base64.encodeToString(cipher.doFinal(senderKey.toByteArray()), Base64.NO_WRAP) | ||
recipients.forEach { recipient -> | ||
recipient?.let { | ||
val recipientPublicKey = recipient.public_key!! | ||
val encryptedAESKey = | ||
encryptAESKeyWithECDH(aesKey, recipientPublicKey, senderPrivateKey) | ||
encryptedKeys[recipient.id] = Pair(encryptedSenderKey, encryptedAESKey) | ||
} | ||
} | ||
|
||
return encryptedKeys | ||
} | ||
|
||
fun decryptSenderKey( | ||
encryptedSenderKey: String, | ||
encryptedAESKey: String, | ||
recipientPrivateKey: String, | ||
senderPublicKey: String | ||
): String { | ||
val aesKey = decryptAESKeyWithECDH(encryptedAESKey, recipientPrivateKey, senderPublicKey) | ||
val cipher = Cipher.getInstance("AES") | ||
cipher.init(Cipher.DECRYPT_MODE, aesKey) | ||
val decryptedSenderKeyBytes = | ||
cipher.doFinal(Base64.decode(encryptedSenderKey, Base64.DEFAULT)) | ||
|
||
return String(decryptedSenderKeyBytes) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation and documentation for encrypted fields
The
EncryptedApiLocation
class handles sensitive location data. Consider the following improvements:📝 Committable suggestion