Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -250,15 +250,11 @@ class TinkCryptoManager(
}
}

// TODO(SEC4-A-07): Add cross-signature validation for wrapped DEKs. Currently, any device
// in the bucket can wrap a DEK for any other device without proof of authorization. A future
// protocol enhancement should include a signature from the wrapping device over the wrapped
// payload, allowing the recipient to verify that the DEK was wrapped by an authorized
// (attested) device. This requires:
// 1. The wrapper signs (wrappedDekBlob || recipientDeviceId || keyEpoch) with its Ed25519 key
// 2. The signature is transmitted alongside the wrapped DEK
// 3. The recipient verifies the signature against the wrapper's attested signing key
// 4. Key attestation chain must be validated before trusting the signature
// DEFERRED(SEC4-A-07): Cross-signature validation for wrapped DEKs. Any bucket device
// can currently wrap a DEK without proof of authorization. Requires protocol design:
// wrapper signs (wrappedDekBlob || recipientDeviceId || keyEpoch) with Ed25519 key,
// signature is transmitted alongside wrapped DEK, recipient verifies against attested
// signing key. Blocked on attestation signature format specification.
override fun unwrapDek(
wrappedDek: String,
devicePrivateKey: PrivateKey,
Expand Down Expand Up @@ -449,9 +445,10 @@ class TinkCryptoManager(
}
}

// TODO(SEC6-A-05): The DEK epoch should come from the server's wrapped key response, not
// from the locally stored currentEpoch. Using local epoch means a replayed wrapped DEK
// could overwrite a newer epoch's key. Accept keyEpoch as a parameter instead.
// DEFERRED(SEC6-A-05): DEK epoch from server. The epoch should come from the server's
// wrapped key response, not local currentEpoch. Using local epoch means a replayed
// wrapped DEK could overwrite a newer epoch's key. Requires server API change to
// include keyEpoch in the wrapped key response.
override suspend fun unwrapAndStoreDek(
bucketId: String,
wrappedDek: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@ import okhttp3.Response
* Session tokens are stored in EncryptedSharedPreferences to prevent extraction
* from device backups or root access.
*
* TODO(SEC3-A-21): Implement an OkHttp Authenticator (TokenAuthenticator) to handle
* 401 responses with automatic re-authentication via challenge-response. This would
* transparently retry failed requests after obtaining a new session token, avoiding
* the need for callers to handle 401s explicitly. Implementation requires injecting
* AuthRepository or KeyManager + ApiService (careful to avoid circular dependencies
* with Hilt). Consider using OkHttp's `Authenticator` interface which is specifically
* designed for this purpose.
* SEC3-A-21: 401 responses are now handled by [TokenAuthenticator], which automatically
* re-authenticates via challenge-response and retries the failed request. The circular
* Hilt dependency (NetworkModule -> OkHttpClient -> TokenAuthenticator -> AuthRepository
* -> ApiService -> OkHttpClient) is broken using [dagger.Lazy].
*/
class AuthInterceptor(
private val prefs: SharedPreferences
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package com.kidsync.app.data.remote.interceptor

import android.content.SharedPreferences
import com.kidsync.app.domain.repository.AuthRepository
import kotlinx.coroutines.runBlocking
import okhttp3.Authenticator
import okhttp3.Request
import okhttp3.Response
import okhttp3.Route
import javax.inject.Inject
import javax.inject.Named

/**
* OkHttp Authenticator that handles 401 responses by re-authenticating
* via the Ed25519 challenge-response flow and retrying the failed request.
*
* This replaces the need for callers to handle 401s explicitly. When the
* server returns a 401 Unauthorized, OkHttp invokes this authenticator
* which:
* 1. Checks if a retry has already been attempted (prevents infinite loops)
* 2. Synchronizes concurrent 401s so only one re-authentication occurs
* 3. Performs challenge-response auth via AuthRepository
* 4. Retries the original request with the new session token
*
* Uses [dagger.Lazy] for AuthRepository to break the circular Hilt dependency:
* NetworkModule -> OkHttpClient -> TokenAuthenticator -> AuthRepository -> ApiService -> OkHttpClient
*/
class TokenAuthenticator @Inject constructor(
private val authRepository: dagger.Lazy<AuthRepository>,
@Named("encrypted_prefs") private val prefs: SharedPreferences
) : Authenticator {

private val lock = Object()

/**
* Header added to retried requests to prevent infinite retry loops.
* If the retried request also gets a 401, we return null (give up).
*/
companion object {
internal const val HEADER_AUTH_RETRY = "X-Auth-Retry"
}

override fun authenticate(route: Route?, response: Response): Request? {
// Prevent infinite retry loops: if we already retried, give up
if (response.request.header(HEADER_AUTH_RETRY) != null) {
return null
}

synchronized(lock) {
// Check if another thread already refreshed the token since our
// request was made. Compare the token we sent with the current one.
val currentToken = prefs.getString(AuthInterceptor.PREF_SESSION_TOKEN, null)
val requestToken = response.request.header(AuthInterceptor.HEADER_AUTHORIZATION)
?.removePrefix("Bearer ")

if (currentToken != null && currentToken != requestToken) {
// Token was already refreshed by another thread -- retry with it
return response.request.newBuilder()
.header(AuthInterceptor.HEADER_AUTHORIZATION, "Bearer $currentToken")
.header(HEADER_AUTH_RETRY, "true")
.build()
}

// Perform challenge-response authentication
val result = runBlocking {
authRepository.get().authenticate()
}
Comment thread
CybotTM marked this conversation as resolved.

val session = result.getOrNull() ?: return null

return response.request.newBuilder()
.header(AuthInterceptor.HEADER_AUTHORIZATION, "Bearer ${session.sessionToken}")
.header(HEADER_AUTH_RETRY, "true")
.build()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ object DatabaseModule {
@Suppress("DEPRECATION")
builder.fallbackToDestructiveMigration()
}
// TODO: Add migration objects here for each version bump in release builds:
// builder.addMigrations(MIGRATION_4_5, MIGRATION_5_6, ...)
// DEFERRED: Room migration objects. Add migrations here for each schema version
// bump in release builds: builder.addMigrations(MIGRATION_X_Y, ...).
// Currently no pending migrations — destructive fallback handles debug builds
// above, and release builds will crash visibly if a migration is missing.

return builder.build()
}
Expand Down
6 changes: 4 additions & 2 deletions android/app/src/main/java/com/kidsync/app/di/NetworkModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.kidsync.app.di
import android.content.SharedPreferences
import com.kidsync.app.data.remote.api.ApiService
import com.kidsync.app.data.remote.interceptor.AuthInterceptor
import com.kidsync.app.data.remote.interceptor.TokenAuthenticator
import com.kidsync.app.BuildConfig
import dagger.Module
import dagger.Provides
Expand Down Expand Up @@ -47,9 +48,10 @@ object NetworkModule {
@Singleton
fun provideOkHttpClientManager(
authInterceptor: AuthInterceptor,
loggingInterceptor: HttpLoggingInterceptor
loggingInterceptor: HttpLoggingInterceptor,
tokenAuthenticator: TokenAuthenticator
): OkHttpClientManager {
return OkHttpClientManager(authInterceptor, loggingInterceptor)
return OkHttpClientManager(authInterceptor, loggingInterceptor, tokenAuthenticator)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.kidsync.app.di

import com.kidsync.app.BuildConfig
import com.kidsync.app.data.remote.interceptor.AuthInterceptor
import com.kidsync.app.data.remote.interceptor.TokenAuthenticator
import okhttp3.CertificatePinner
import okhttp3.ConnectionSpec
import okhttp3.OkHttpClient
Expand Down Expand Up @@ -35,7 +36,8 @@ import kotlin.concurrent.write
@Singleton
class OkHttpClientManager @Inject constructor(
private val authInterceptor: AuthInterceptor,
private val loggingInterceptor: HttpLoggingInterceptor
private val loggingInterceptor: HttpLoggingInterceptor,
private val tokenAuthenticator: TokenAuthenticator
) {
private val lock = ReentrantReadWriteLock()

Expand Down Expand Up @@ -80,6 +82,7 @@ class OkHttpClientManager @Inject constructor(
.connectionSpecs(listOf(tlsSpec))
.addInterceptor(authInterceptor)
.addInterceptor(loggingInterceptor)
.authenticator(tokenAuthenticator)
.connectTimeout(30, TimeUnit.SECONDS)
.readTimeout(30, TimeUnit.SECONDS)
.writeTimeout(30, TimeUnit.SECONDS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class SnapshotUseCase @Inject constructor(
private val custodyScheduleDao: CustodyScheduleDao,
private val overrideDao: OverrideDao,
private val expenseDao: ExpenseDao,
private val calendarEventDao: CalendarEventDao,
private val infoBankDao: InfoBankDao,
private val opLogDao: OpLogDao,
private val syncStateDao: SyncStateDao,
private val cryptoManager: CryptoManager,
Expand Down Expand Up @@ -121,10 +123,13 @@ class SnapshotUseCase @Inject constructor(
/**
* Compute SHA-256 hash of the current materialized state.
*
* TODO(SEC6-A-16): Incomplete entity coverage - this hash only covers CustodySchedule,
* ScheduleOverride, and Expense entities. CalendarEvent and InfoBankEntry entities are
* missing, meaning state divergence in those entity types would not be detected by
* snapshot comparison. Add them to the hash computation for full coverage.
* Covers all materialized entity types: CustodySchedule, ScheduleOverride,
* Expense, CalendarEvent, and InfoBankEntry (non-deleted only).
*
* Design: Hashes identity + core value fields per entity (not all columns).
* Metadata fields (createdBy, timestamps, description, location, notes) are
* excluded so the hash detects structural divergence without false positives
* from metadata-only changes that don't affect the materialized schedule.
*/
private suspend fun computeStateHash(): String {
val digest = MessageDigest.getInstance("SHA-256")
Expand All @@ -150,6 +155,25 @@ class SnapshotUseCase @Inject constructor(
digest.update(expense.amountCents.toString().toByteArray())
}

// Hash all calendar events (SEC6-A-16)
val calendarEvents = calendarEventDao.getAllEvents()
for (event in calendarEvents.sortedBy { it.eventId }) {
digest.update(event.eventId.toByteArray())
digest.update(event.title.toByteArray())
digest.update(event.startTime.toByteArray())
digest.update(event.endTime.toByteArray())
}

// Hash all non-deleted info bank entries (SEC6-A-16)
val infoBankEntries = infoBankDao.getAllEntries()
for (entry in infoBankEntries.sortedBy { it.entryId.toString() }) {
digest.update(entry.entryId.toString().toByteArray())
digest.update(entry.childId.toString().toByteArray())
digest.update(entry.category.toByteArray())
Comment thread
CybotTM marked this conversation as resolved.
entry.title?.let { digest.update(it.toByteArray()) }
entry.content?.let { digest.update(it.toByteArray()) }
}

return digest.digest().joinToString("") { "%02x".format(it) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ class AuthViewModel @Inject constructor(
* 3. Register as new device, authenticate
* 4. Unwrap DEK using recovery key
*
* TODO(SEC6-A-08): Key ordering during recovery flow - the recovery restores the
* device seed before registering new encryption keys with the server. If restoration
* fails mid-flow, the local seed may not match the server's registered keys. Consider
* ordering: register first, then restore seed, then unwrap DEK.
* DEFERRED(SEC6-A-08): Key ordering during recovery flow. Seed is restored before
* registering new keys with the server — if restoration fails mid-flow, local seed
* may not match server's registered keys. Fix requires an "update device keys" server
* endpoint to re-register keys after seed restore, which is a protocol change.
*/
fun restoreFromRecovery() {
val state = _uiState.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,17 @@ import javax.inject.Inject
* QR code payload for pairing.
* Contains connection info and initiator's key fingerprint but never the DEK.
*
* TODO(SEC4-A-10): The invite token [t] is transmitted in plaintext within the QR code.
* This is by design: the QR code is displayed briefly for the co-parent to scan in person,
* and the token is single-use (the server invalidates it after redemption). Additionally,
* PairingScreen sets FLAG_SECURE to prevent screenshots. However, for defense in depth,
* a future enhancement could encrypt the token field using the recipient's public key
* (requires the scanner's public key to be known before pairing, e.g., via a two-phase
* handshake or pre-shared key exchange).
* DEFERRED(SEC4-A-10): Invite token is plaintext in QR code. Acceptable because: QR is
* displayed briefly, PairingScreen sets FLAG_SECURE, and the token is single-use (server
* invalidates after redemption). Encrypting the token would require the scanner's public
* key before pairing — needs a two-phase handshake or pre-shared key exchange protocol.
*/
@Serializable
data class QrPairingPayload(
val v: Int = 1,
val s: String, // serverUrl
val b: String, // bucketId
val t: String, // inviteToken (plaintext -- see SEC4-A-10 TODO above)
val t: String, // inviteToken (plaintext -- see DEFERRED SEC4-A-10 above)
val f: String // signingKeyFingerprint of initiator
)

Expand Down Expand Up @@ -163,13 +160,9 @@ class BucketViewModel @Inject constructor(
)
inviteResult.getOrThrow()

// Build QR payload
// TODO(SC-03): The spec says the QR code should contain the signing key fingerprint,
// but we currently use the encryption key fingerprint. Both sides (QR generation and
// verification) consistently use the encryption key, so this works. If the spec is
// updated or the server changes to use signing key fingerprints, update both sides.
// Build QR payload with signing key fingerprint per spec (SC-03)
val serverUrl = authRepository.getServerUrl()
val fingerprint = keyManager.getEncryptionKeyFingerprint()
val fingerprint = keyManager.getSigningKeyFingerprint()

val payload = QrPairingPayload(
v = 1,
Expand Down Expand Up @@ -390,7 +383,7 @@ class BucketViewModel @Inject constructor(
val peerDevicesResult = bucketRepository.getBucketDevices(payload.b)
val peerDevices = peerDevicesResult.getOrThrow()
val peerVerified = peerDevices.any { device ->
cryptoManager.computeKeyFingerprint(device.encryptionKey) == payload.f
cryptoManager.computeKeyFingerprint(device.signingKey) == payload.f
}

if (!peerVerified) {
Expand Down Expand Up @@ -426,7 +419,7 @@ class BucketViewModel @Inject constructor(

// Cross-sign the peer's key
val peerDevice = peerDevices.first { device ->
cryptoManager.computeKeyFingerprint(device.encryptionKey) == payload.f
cryptoManager.computeKeyFingerprint(device.signingKey) == payload.f
}
val attestation = keyManager.createKeyAttestation(
attestedDeviceId = peerDevice.deviceId,
Expand Down
Loading