fix: critical security issues from code review #25

Closed
eddy wants to merge 2 commits from fix/code-review-critical-issues into main
13 changed files with 522 additions and 46 deletions

View file

@ -0,0 +1,53 @@
<h2>Voorgestelde Flow: De "Server-as-Broker" methode</h2>
<p class="subtitle">Gebruik de SylDa_Server als ontmoetingsplaats voor versie-informatie</p>
<div class="mockup">
<div class="mockup-header">Interactie Diagram</div>
<div class="mockup-body" style="background: #1e1e1e; color: #d4d4d4; padding: 20px; font-family: monospace; line-height: 1.4;">
<div style="margin-bottom: 10px; color: #569cd6;">// 1. Client meldt zich aan</div>
[Client] --(Write version: "5")--> [Server: ClientStatusChar]
[Client] --(Subscribe)------------> [Server: ClientControlChar]
<div style="margin-top: 20px; margin-bottom: 10px; color: #569cd6;">// 2. App checkt status</div>
[App] --(Read ClientStatus)----> [Server]
<span style="color: #ce9178;">App denkt: "Client heeft v5, ik heb v6. Update nodig!"</span>
<div style="margin-top: 20px; margin-bottom: 10px; color: #569cd6;">// 3. App start de dans</div>
[App] --(Write: "START_OTA")---> [Server: ClientControlChar]
[Server] --(Notify)---------------> [Client]
<div style="margin-top: 20px; margin-bottom: 10px; color: #569cd6;">// 4. De wissel</div>
<span style="color: #4ec9b0;">Client disconnect van Server -> Start Advertising</span>
<span style="color: #4ec9b0;">App disconnect van Server -> Scant naar Client OTA</span>
<div style="margin-top: 20px; margin-bottom: 10px; color: #569cd6;">// 5. De eigenlijke OTA</div>
[App] --(Direct BLE OTA)-------> [Client (nu in Server mode)]
</div>
</div>
<div class="section">
<h3>Technische details om te bespreken:</h3>
<div class="options" data-multiselect>
<div class="option" data-choice="uuid-tracking" onclick="toggleSelect(this)">
<div class="letter">1</div>
<div class="content">
<h3>UUID Tracking</h3>
<p>Aangezien de Server 2 clients ondersteunt, moet de Server de versie per UUID opslaan zodat de App weet welke client welke versie heeft.</p>
</div>
</div>
<div class="option" data-choice="auth" onclick="toggleSelect(this)">
<div class="letter">2</div>
<div class="content">
<h3>Beveiliging (HMAC/AES)</h3>
<p>Moet de "START_OTA" opdracht ook versleuteld zijn? (Waarschijnlijk wel, om te voorkomen dat een vreemde de clients platlegt).</p>
</div>
</div>
<div class="option" data-choice="reconnect" onclick="toggleSelect(this)">
<div class="letter">3</div>
<div class="content">
<h3>Auto-reconnect</h3>
<p>Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.</p>
</div>
</div>
</div>
</div>

View file

@ -0,0 +1,48 @@
<h2>Ontwerpkeuzes BLE OTA voor de Client</h2>
<p class="subtitle">Hoe krijgt de Client (ESP32) de nieuwe firmware?</p>
<div class="options">
<div class="option" data-choice="android-direct" onclick="toggleSelect(this)">
<div class="letter">A</div>
<div class="content">
<h3>Android App Direct</h3>
<p>De Client ESP32 start een tijdelijke BLE Server (OTA Service). De Android app verbindt direct met de Client om de firmware te pushen.</p>
<div class="label">Voordeel: Simpel, hergebruikt Android OTA code.</div>
<div class="label">Nadeel: Handmatige actie nodig op Client (bijv. knop indrukken).</div>
</div>
</div>
<div class="option" data-choice="server-proxy" onclick="toggleSelect(this)">
<div class="letter">B</div>
<div class="content">
<h3>Server Proxy / Relay</h3>
<p>De Android app stuurt de firmware naar de SylDa_Server. De Server stuurt het door naar de Client via de bestaande BLE verbinding.</p>
<div class="label">Voordeel: Geen directe verbinding Android-Client nodig.</div>
<div class="label">Nadeel: Complexer, Server moet grote buffers hebben of flow control.</div>
</div>
</div>
<div class="option" data-choice="client-pull" onclick="toggleSelect(this)">
<div class="letter">C</div>
<div class="content">
<h3>Client Pull (via Server)</h3>
<p>De Server kondigt een update aan. De Client haalt de firmware "chunks" op bij de Server via een nieuwe OTA service op de Server.</p>
<div class="label">Voordeel: Client heeft controle over tempo.</div>
<div class="label">Nadeel: Server moet de firmware ergens opslaan (Flash/NVS/SPIFFS?) voordat hij het kan doorgeven.</div>
</div>
</div>
</div>
<div class="section">
<h3>Huidige Situatie</h3>
<div class="mockup">
<div class="mockup-header">Architectuur</div>
<div class="mockup-body">
<pre>
[Android App] --(BLE OTA)--> [SylDa_Server (ESP32)]
|
? --(BLE OTA?)--> [SylDa_Client (ESP32)]
</pre>
</div>
</div>
</div>

View file

@ -0,0 +1,2 @@
{"type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849065}
{"type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849785}

View file

@ -0,0 +1 @@
{"reason":"idle timeout","timestamp":1785872682901}

View file

@ -0,0 +1,7 @@
{"type":"server-started","port":56787,"host":"0.0.0.0","url_host":"100.97.199.84","url":"http://100.97.199.84:56787","screen_dir":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content","state_dir":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/state"}
{"type":"screen-added","file":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content/ota_options.html"}
{"source":"user-event","type":"click","text":"A\n \n Android App Direct\n De Client ESP32 start een tijdelijke BLE Server (OTA Service). De Android app verbindt direct met de Client om de firmware te pushen.\n Voordeel: Simpel, hergebruikt Android OTA code.\n Nadeel: Handmatige actie nodig op Client (bijv. knop indrukken).","choice":"android-direct","id":null,"timestamp":1785870382036}
{"type":"screen-added","file":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content/broker_flow.html"}
{"source":"user-event","type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849065}
{"source":"user-event","type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849785}
{"type":"server-stopped","reason":"idle timeout"}

View file

@ -0,0 +1 @@
124706

View file

@ -22,8 +22,10 @@ import androidx.core.app.ActivityCompat
import java.nio.ByteBuffer import java.nio.ByteBuffer
import java.nio.ByteOrder import java.nio.ByteOrder
import java.security.KeyPairGenerator import java.security.KeyPairGenerator
import java.security.KeyFactory
import java.security.SecureRandom import java.security.SecureRandom
import java.security.spec.ECGenParameterSpec import java.security.spec.ECGenParameterSpec
import java.security.spec.X509EncodedKeySpec
import java.security.interfaces.ECPublicKey import java.security.interfaces.ECPublicKey
import java.util.UUID import java.util.UUID
import javax.crypto.Cipher import javax.crypto.Cipher
@ -61,6 +63,25 @@ class BleRelayManager(
private const val ECDH_PKT_SIZE = 97 private const val ECDH_PKT_SIZE = 97
private const val BLE_MTU = 512 private const val BLE_MTU = 512
private const val ECDH_TIMEOUT_MS = 5000L private const val ECDH_TIMEOUT_MS = 5000L
private val SECP256R1_SPKI_HEADER = byteArrayOf(
0x30.toByte(), 0x59.toByte(), 0x30.toByte(), 0x13.toByte(),
0x06.toByte(), 0x07.toByte(), 0x2A.toByte(), 0x86.toByte(),
0x48.toByte(), 0xCE.toByte(), 0x3D.toByte(), 0x02.toByte(),
0x01.toByte(), 0x06.toByte(), 0x08.toByte(), 0x2A.toByte(),
0x86.toByte(), 0x48.toByte(), 0xCE.toByte(), 0x3D.toByte(),
0x03.toByte(), 0x01.toByte(), 0x07.toByte(), 0x03.toByte(),
0x42.toByte(), 0x00.toByte()
)
private fun decodeEcPublicKey(uncompressedKey: ByteArray): ECPublicKey {
require(uncompressedKey.size == ECDH_PUBKEY_LEN && uncompressedKey[0] == 0x04.toByte()) {
"Invalid uncompressed EC key format"
}
val spkiEncoded = SECP256R1_SPKI_HEADER + uncompressedKey
return KeyFactory.getInstance("EC")
.generatePublic(X509EncodedKeySpec(spkiEncoded)) as ECPublicKey
}
} }
private var bluetoothGatt: BluetoothGatt? = null private var bluetoothGatt: BluetoothGatt? = null
@ -84,6 +105,16 @@ class BleRelayManager(
private var ecdhNotifyBuf: ByteArray = ByteArray(0) private var ecdhNotifyBuf: ByteArray = ByteArray(0)
private var ecdhCccdPending = false private var ecdhCccdPending = false
@Synchronized
private fun setEcdhNotifyBuf(value: ByteArray) {
ecdhNotifyBuf = value.copyOf()
}
@Synchronized
private fun getEcdhNotifyBuf(): ByteArray {
return ecdhNotifyBuf.copyOf()
}
interface OtaNotifyCallback { interface OtaNotifyCallback {
fun onOtaNotify(status: Int, seq: Int) fun onOtaNotify(status: Int, seq: Int)
} }
@ -404,7 +435,7 @@ class BleRelayManager(
Log.d(TAG, "OTA notify: status=$statusName seq=$seqVal raw=${value.joinToString("") { "%02X".format(it) }}") Log.d(TAG, "OTA notify: status=$statusName seq=$seqVal raw=${value.joinToString("") { "%02X".format(it) }}")
handler.post { otaNotifyCallback?.onOtaNotify(statusVal, seqVal) } handler.post { otaNotifyCallback?.onOtaNotify(statusVal, seqVal) }
} else if (characteristic.uuid == ECDH_UUID) { } else if (characteristic.uuid == ECDH_UUID) {
ecdhNotifyBuf = value.copyOf() setEcdhNotifyBuf(value)
ecdhDeferred?.complete(true) ecdhDeferred?.complete(true)
Log.d(TAG, "ECDH notify received (${value.size} bytes)") Log.d(TAG, "ECDH notify received (${value.size} bytes)")
} }
@ -506,7 +537,7 @@ class BleRelayManager(
return@Thread return@Thread
} }
val resp = ecdhNotifyBuf val resp = getEcdhNotifyBuf()
if (resp.size != ECDH_PKT_SIZE) { if (resp.size != ECDH_PKT_SIZE) {
Log.w(TAG, "ECDH: bad response size ${resp.size}") Log.w(TAG, "ECDH: bad response size ${resp.size}")
handler.post { handler.post {
@ -530,28 +561,7 @@ class BleRelayManager(
val ka = KeyAgreement.getInstance("ECDH") val ka = KeyAgreement.getInstance("ECDH")
ka.init(keyPair.private) ka.init(keyPair.private)
val spec = java.security.spec.ECPublicKeySpec( val serverKey = decodeEcPublicKey(serverPub)
java.security.spec.ECPoint(
java.math.BigInteger(1, serverPub.copyOfRange(1, 33)),
java.math.BigInteger(1, serverPub.copyOfRange(33, 65))
),
java.security.spec.ECParameterSpec(
java.security.spec.EllipticCurve(
java.security.spec.ECFieldFp(
java.math.BigInteger("FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF", 16)
),
java.math.BigInteger("FFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFC", 16),
java.math.BigInteger("5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B", 16)
),
java.security.spec.ECPoint(
java.math.BigInteger("6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296", 16),
java.math.BigInteger("4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5", 16)
),
java.math.BigInteger("FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551", 16),
1
)
)
val serverKey = java.security.KeyFactory.getInstance("EC").generatePublic(spec)
ka.doPhase(serverKey, true) ka.doPhase(serverKey, true)
val shared = ka.generateSecret() val shared = ka.generateSecret()
@ -606,13 +616,13 @@ class BleRelayManager(
} }
private fun loadState() { private fun loadState() {
val prefs = PrefsHelper.getPrefs(context) PrefsHelper.migrateCredentialsIfNeeded(context)
val hexKey = prefs.getString(PrefsHelper.PREF_KEY, null) val hexKey = PrefsHelper.getPsk(context)
psk = hexKey?.let { hexStringToBytes(it) } psk = hexKey?.let { hexStringToBytes(it) }
val hexUuid = PrefsHelper.getServerUuid(context) val hexUuid = PrefsHelper.getServerUuid(context)
?: prefs.getString(PrefsHelper.PREF_UUID, null) ?: PrefsHelper.getDeviceCredentialUuid(context)
deviceUuid = hexUuid?.let { hexStringToBytes(it) } deviceUuid = hexUuid?.let { hexStringToBytes(it) }
seq = prefs.getInt(PrefsHelper.PREF_SEQ, 1).coerceAtMost(PrefsHelper.SEQ_MAX) seq = PrefsHelper.getPrefs(context).getInt(PrefsHelper.PREF_SEQ, 1).coerceAtMost(PrefsHelper.SEQ_MAX)
Log.d(TAG, "PSK loaded: ${psk?.size ?: "none"} bytes, UUID: ${deviceUuid?.size ?: "none"} bytes, seq=$seq") Log.d(TAG, "PSK loaded: ${psk?.size ?: "none"} bytes, UUID: ${deviceUuid?.size ?: "none"} bytes, seq=$seq")
} }

View file

@ -3,12 +3,16 @@ package nl.eddydevink.sylda
import android.content.Context import android.content.Context
import android.content.SharedPreferences import android.content.SharedPreferences
import android.util.Log
import androidx.appcompat.app.AppCompatDelegate import androidx.appcompat.app.AppCompatDelegate
import androidx.security.crypto.EncryptedSharedPreferences
import androidx.security.crypto.MasterKey
object PrefsHelper { object PrefsHelper {
private const val TAG = "PrefsHelper" private const val TAG = "PrefsHelper"
const val PREFS_NAME = "relay_prefs" const val PREFS_NAME = "relay_prefs"
private const val ENCRYPTED_PREFS_NAME = "encrypted_prefs"
const val PREF_KEY = "psk" const val PREF_KEY = "psk"
const val PREF_UUID = "device_uuid" const val PREF_UUID = "device_uuid"
const val PREF_SEQ = "seq_counter" const val PREF_SEQ = "seq_counter"
@ -20,6 +24,49 @@ object PrefsHelper {
const val KEY_CLIENT_UUID = "clientUuid" const val KEY_CLIENT_UUID = "clientUuid"
const val SEQ_MAX = 1_000_000 const val SEQ_MAX = 1_000_000
private var masterKey: MasterKey? = null
private fun getMasterKey(context: Context): MasterKey {
return masterKey ?: run {
MasterKey.Builder(context)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build().also { masterKey = it }
}
}
fun getEncryptedPrefs(context: Context): SharedPreferences {
return EncryptedSharedPreferences.create(
context,
ENCRYPTED_PREFS_NAME,
getMasterKey(context),
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)
}
fun migrateCredentialsIfNeeded(context: Context) {
val plainPrefs = getPrefs(context)
val encPrefs = getEncryptedPrefs(context)
val legacyKey = plainPrefs.getString(PREF_KEY, null)
val legacyUuid = plainPrefs.getString(PREF_UUID, null)
if (legacyKey != null && !encPrefs.contains(PREF_KEY)) {
Log.i(TAG, "Migrating credentials to encrypted storage")
encPrefs.edit().apply {
putString(PREF_KEY, legacyKey)
if (legacyUuid != null) putString(PREF_UUID, legacyUuid)
apply()
}
plainPrefs.edit().apply {
remove(PREF_KEY)
remove(PREF_UUID)
apply()
}
Log.i(TAG, "Credentials migration complete")
}
}
fun getDeviceMode(context: Context): String = getPrefs(context).getString(KEY_DEVICE_MODE, "server") ?: "server" fun getDeviceMode(context: Context): String = getPrefs(context).getString(KEY_DEVICE_MODE, "server") ?: "server"
fun setDeviceMode(context: Context, mode: String) = getPrefs(context).edit().putString(KEY_DEVICE_MODE, mode).apply() fun setDeviceMode(context: Context, mode: String) = getPrefs(context).edit().putString(KEY_DEVICE_MODE, mode).apply()
fun getTransportMode(context: Context): String = getPrefs(context).getString("transportMode", "ble") ?: "ble" fun getTransportMode(context: Context): String = getPrefs(context).getString("transportMode", "ble") ?: "ble"
@ -29,6 +76,11 @@ object PrefsHelper {
fun getClientUuid(context: Context): String? = getPrefs(context).getString(KEY_CLIENT_UUID, null) fun getClientUuid(context: Context): String? = getPrefs(context).getString(KEY_CLIENT_UUID, null)
fun setClientUuid(context: Context, uuid: String) = getPrefs(context).edit().putString(KEY_CLIENT_UUID, uuid).apply() fun setClientUuid(context: Context, uuid: String) = getPrefs(context).edit().putString(KEY_CLIENT_UUID, uuid).apply()
fun getPsk(context: Context): String? = getEncryptedPrefs(context).getString(PREF_KEY, null)
fun setPsk(context: Context, psk: String) = getEncryptedPrefs(context).edit().putString(PREF_KEY, psk).apply()
fun getDeviceCredentialUuid(context: Context): String? = getEncryptedPrefs(context).getString(PREF_UUID, null)
fun setDeviceCredentialUuid(context: Context, uuid: String) = getEncryptedPrefs(context).edit().putString(PREF_UUID, uuid).apply()
fun applyTheme(context: Context) { fun applyTheme(context: Context) {
val prefs = getPrefs(context) val prefs = getPrefs(context)
val mode = when (prefs.getString(PREF_THEME, "system")) { val mode = when (prefs.getString(PREF_THEME, "system")) {

View file

@ -372,9 +372,8 @@ class SettingsActivity : AppCompatActivity() {
private fun doKeyExchange(usbManager: UsbManager, device: UsbDevice) { private fun doKeyExchange(usbManager: UsbManager, device: UsbDevice) {
showProgressDialog(getString(R.string.title_key_exchange), getString(R.string.msg_reading_key), STYLE_SPINNER) showProgressDialog(getString(R.string.title_key_exchange), getString(R.string.msg_reading_key), STYLE_SPINNER)
val prefs = PrefsHelper.getPrefs(this) val appUuid = PrefsHelper.getDeviceCredentialUuid(this)
val appUuid = prefs.getString(PrefsHelper.PREF_UUID, null) val appKey = PrefsHelper.getPsk(this)
val appKey = prefs.getString(PrefsHelper.PREF_KEY, null)
lifecycleScope.launch(Dispatchers.IO) { lifecycleScope.launch(Dispatchers.IO) {
val result = readKeyFromEsp(usbManager, device, appUuid, appKey) val result = readKeyFromEsp(usbManager, device, appUuid, appKey)
@ -384,10 +383,8 @@ class SettingsActivity : AppCompatActivity() {
progressDialog = null progressDialog = null
when (result) { when (result) {
is KeyResult.Success -> { is KeyResult.Success -> {
prefs.edit() PrefsHelper.setDeviceCredentialUuid(this@SettingsActivity, result.deviceUuid)
.putString(PrefsHelper.PREF_UUID, result.deviceUuid) PrefsHelper.setPsk(this@SettingsActivity, result.hexKey)
.putString(PrefsHelper.PREF_KEY, result.hexKey)
.apply()
if (PrefsHelper.getDeviceMode(this@SettingsActivity) == "server") { if (PrefsHelper.getDeviceMode(this@SettingsActivity) == "server") {
PrefsHelper.setServerUuid(this@SettingsActivity, result.deviceUuid) PrefsHelper.setServerUuid(this@SettingsActivity, result.deviceUuid)
} else { } else {
@ -838,8 +835,7 @@ class SettingsActivity : AppCompatActivity() {
private fun showConflictDialog(usbManager: UsbManager, device: UsbDevice, private fun showConflictDialog(usbManager: UsbManager, device: UsbDevice,
espUuid: String, espKey: String, appKey: String) { espUuid: String, espKey: String, appKey: String) {
val prefs = PrefsHelper.getPrefs(this) val appUuid = PrefsHelper.getDeviceCredentialUuid(this)
val appUuid = prefs.getString(PrefsHelper.PREF_UUID, null)
val newAppKey = appKey.ifEmpty { generateRandomHexKey() } val newAppKey = appKey.ifEmpty { generateRandomHexKey() }
val newAppUuid = appUuid ?: generateRandomHex(UUID_BYTE_COUNT) val newAppUuid = appUuid ?: generateRandomHex(UUID_BYTE_COUNT)
@ -946,11 +942,8 @@ class SettingsActivity : AppCompatActivity() {
} }
} }
val prefs = PrefsHelper.getPrefs(this) PrefsHelper.setDeviceCredentialUuid(this, newUuid)
prefs.edit() PrefsHelper.setPsk(this, appKey)
.putString(PrefsHelper.PREF_UUID, newUuid)
.putString(PrefsHelper.PREF_KEY, appKey)
.apply()
if (PrefsHelper.getDeviceMode(this) == "server") { if (PrefsHelper.getDeviceMode(this) == "server") {
PrefsHelper.setServerUuid(this, newUuid) PrefsHelper.setServerUuid(this, newUuid)
} else { } else {

View file

@ -0,0 +1,305 @@
# Code Review Fixes (Batch 2) Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development
> (recommended) or superpowers:executing-plans to implement this plan task-by-task.
> Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Fix 6 issues found in code review: broken client OTA UI flow (SettingsActivity button), missing timeout in OTA polling, recomputeClientOta race condition, committed upload_port, wrong LED startup sequence, and noisy ESP_LOGI diagnostics.
**Architecture:** All fixes are isolated single-file edits. No new files. No architecture changes. Version bumps: server 21→22, client 11→12.
**Tech Stack:** C++ (Arduino/ESP-IDF), Kotlin (Android), PlatformIO
---
### Task 1: Fix SettingsActivity button — client+BLE now routes to startClientOtaUpdate
**Files:**
- Modify: `android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt:167-174`
- [ ] **Step 1: Replace button click handler**
In `setupButtons()` (or wherever the click listener is set), replace lines 167-174 with the corrected logic:
```kotlin
findViewById<Button>(R.id.updateModuleButton).setOnClickListener {
val mode = PrefsHelper.getDeviceMode(this)
val transportId = findViewById<RadioGroup>(R.id.radioTransport).checkedRadioButtonId
if (transportId == R.id.radioUsb) {
startFlashing()
} else if (mode == "client") {
startClientOtaUpdate(BleRelayManager.instance)
} else {
startServerOtaUpdate(BleRelayManager.instance)
}
}
```
- [ ] **Step 2: Verify the change compiles**
```bash
cd android && ./gradlew compileDebugKotlin 2>&1 | tail -5
```
- [ ] **Step 3: Commit**
```bash
git add android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt
git commit -m "fix(android): client+BLE button now routes to client OTA instead of USB flash"
```
---
### Task 2: Add timeout to waitForClientOtaReady polling loop
**Files:**
- Modify: `android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt:393-405`
- [ ] **Step 1: Add `import kotlinx.coroutines.withTimeoutOrNull`**
The import `kotlinx.coroutines.isActive` already exists at line 35. Just add the timeout:
```kotlin
import kotlinx.coroutines.withTimeoutOrNull
```
- [ ] **Step 2: Wrap polling loop in withTimeoutOrNull**
Replace `waitForClientOtaReady` (lines 393-405) with:
```kotlin
private suspend fun waitForClientOtaReady(serverManager: BleRelayManager): Boolean {
return withTimeoutOrNull(30_000L) {
var status = -1
serverManager.setClientOtaStatusListener { s -> status = s }
try {
while (status != 0x01) {
serverManager.readClientOtaStatus()
delay(2000)
}
} finally {
serverManager.setClientOtaStatusListener(null)
}
} ?: false
}
```
- [ ] **Step 3: Verify the change compiles**
```bash
cd android && ./gradlew compileDebugKotlin 2>&1 | tail -5
```
- [ ] **Step 4: Commit**
```bash
git add android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt
git commit -m "fix(android): add 30s timeout to client OTA ready polling"
```
---
### Task 3: Fix recomputeClientOta() race — preserve 0x01 status
**Files:**
- Modify: `src/main.cpp:251`
- [ ] **Step 1: Add 0x01 exclusion to recomputeClientOta**
Change line 251 from:
```cpp
if (!clientOtaAvailable && clientOtaStatus != 0x00 && clientOtaStatus != 0x02) {
```
to:
```cpp
if (!clientOtaAvailable && clientOtaStatus != 0x00 && clientOtaStatus != 0x01 && clientOtaStatus != 0x02) {
```
- [ ] **Step 2: Commit**
```bash
git add src/main.cpp
git commit -m "fix(server): avoid resetting CLIENT_OTA_STATUS 0x01 in recomputeClientOta"
```
---
### Task 4: Remove committed upload_port from platformio.ini
**Files:**
- Modify: `platformio.ini:16,38`
- [ ] **Step 1: Remove both upload_port lines**
Delete line 16:
```
upload_port = /dev/serial/by-id/usb-Espressif_USB_JTAG_serial_debug_unit_44:1B:F6:F4:CF:14-if00
```
Delete line 38:
```
upload_port = /dev/serial/by-id/usb-Espressif_USB_JTAG_serial_debug_unit_44:1B:F6:F5:D1:E0-if00
```
- [ ] **Step 2: Verify platformio.ini is still valid**
```bash
grep -n "upload_port" platformio.ini
```
Expected: no output.
- [ ] **Step 3: Commit**
```bash
git add platformio.ini
git commit -m "fix: remove committed upload_port from platformio.ini"
```
---
### Task 5: Restore LED startup sequence to Blue-Red-Blue
**Files:**
- Modify: `src/client.cpp:603-605`
- [ ] **Step 1: Change LED startup sequence**
Replace lines 603-605:
```cpp
leds[0] = CRGB::Red; FastLED.show(); delay(500);
leds[0] = CRGB::Green; FastLED.show(); delay(500);
leds[0] = CRGB::Red; FastLED.show(); delay(500);
leds[0] = CRGB::Black; FastLED.show();
```
with:
```cpp
leds[0] = CRGB::Blue; FastLED.show(); delay(500);
leds[0] = CRGB::Red; FastLED.show(); delay(500);
leds[0] = CRGB::Blue; FastLED.show(); delay(500);
leds[0] = CRGB::Black; FastLED.show();
```
- [ ] **Step 2: Commit**
```bash
git add src/client.cpp
git commit -m "fix(client): restore LED startup sequence to blue-red-blue"
```
---
### Task 6: Downgrade diagnostic ESP_LOGI to ESP_LOGD
**Files:**
- Modify: `src/client.cpp:907`
- Modify: `src/main.cpp:474`
- [ ] **Step 1: Change client.cpp diagnostic log level**
Change line 907 from:
```cpp
ESP_LOGI(TAG, "OTA: status read value=%d bytes empty=%d firstByte=0x%02X",
```
to:
```cpp
ESP_LOGD(TAG, "OTA: status read value=%d bytes empty=%d firstByte=0x%02X",
```
- [ ] **Step 2: Change server.cpp onRead diagnostic log level**
Change line 474 from:
```cpp
ESP_LOGI(TAG, "CLIENT_OTA_STATUS: read by conn=%d", connInfo.getConnHandle());
```
to:
```cpp
ESP_LOGD(TAG, "CLIENT_OTA_STATUS: read by conn=%d", connInfo.getConnHandle());
```
- [ ] **Step 3: Commit**
```bash
git add src/client.cpp src/main.cpp
git commit -m "chore: downgrade diagnostic OTA logs from INFO to DEBUG"
```
---
### Task 7: Bump versions and rebuild
**Files:**
- Modify: `version.txt`
- [ ] **Step 1: Bump versions**
```
server=22
client=12
```
- [ ] **Step 2: Commit version bump**
```bash
git add version.txt
git commit -m "chore: bump versions server 21->22, client 11->12"
```
- [ ] **Step 3: Rebuild both firmware variants**
```bash
pio run
```
Verify both variants compile: `[env:s3lite-server] SUCCESS` and `[env:s3lite-client] SUCCESS`.
- [ ] **Step 4: Copy built firmware to Android assets**
```bash
cp .pio/build/s3lite-server/firmware.bin android/app/src/main/assets/firmware/server/firmware.bin
cp .pio/build/s3lite-client/firmware.bin android/app/src/main/assets/firmware/client/firmware.bin
```
- [ ] **Step 5: Commit firmware binaries**
```bash
git add android/app/src/main/assets/firmware/
git commit -m "chore: bundle server v22 and client v12 firmware in APK"
```
---
### Verification
After all tasks are complete:
1. **Check git log:**
```bash
git log --oneline -8
```
Expected: 7 new commits on top of b8facac.
2. **Flash server v22:**
```bash
# Find ESP32 server port
ls /dev/serial/by-id/ | grep -i espressif
# Update upload_port in platformio.ini (or use override), then:
pio run -t upload -e s3lite-server
```
3. **Flash client v12:**
```bash
pio run -t upload -e s3lite-client
```
4. **Verification test — Server serial:**
Connect to server serial. Confirm startup message shows v22. Confirm no "CLIENT_OTA_STATUS: read by conn=..." spam (only on CORE_DEBUG_LEVEL=4).
5. **Verification test — Client LED startup:**
Power cycle client. Confirm LED shows Blue → Red → Blue → off.
6. **Verification test — Client OTA flow via BLE:**
- Build and install APK: `cd android && ./gradlew assembleDebug`
- Deploy APK to phone via ADB
- In MainActivity, verify client OTA available notification
- Tap notification → SettingsActivity opens with client+BLE pre-selected
- Tap "Update module" → must show "Wachten op client..." progress dialog (NOT USB dialog)
- If client has older version, OTA should proceed; if same version, toast "Firmware is up-to-date"

View file

@ -260,7 +260,7 @@ static bool performEcdhHandshake(NimBLERemoteService *pSvc) {
return false; return false;
} }
memcpy(aesKey, ecdh.aes_key, AES_KEY_LEN); memcpy(aesKey, ecdh.getAesKey(), AES_KEY_LEN);
ecdh.free(); ecdh.free();
ecdhReady = true; ecdhReady = true;
ESP_LOGI(TAG, "ECDH: handshake complete"); ESP_LOGI(TAG, "ECDH: handshake complete");

View file

@ -21,6 +21,9 @@ private:
mbedtls_ctr_drbg_context ctr_drbg; mbedtls_ctr_drbg_context ctr_drbg;
bool initialized = false; bool initialized = false;
uint8_t shared_secret[32];
uint8_t aes_key[AES_KEY_LEN];
CryptoECDH(const CryptoECDH&) = delete; CryptoECDH(const CryptoECDH&) = delete;
CryptoECDH& operator=(const CryptoECDH&) = delete; CryptoECDH& operator=(const CryptoECDH&) = delete;
@ -29,8 +32,9 @@ public:
uint8_t public_key[ECDH_PUBKEY_LEN]; uint8_t public_key[ECDH_PUBKEY_LEN];
size_t pub_len = 0; size_t pub_len = 0;
uint8_t shared_secret[32];
uint8_t aes_key[AES_KEY_LEN]; const uint8_t* getAesKey() const { return aes_key; }
size_t getAesKeyLen() const { return AES_KEY_LEN; }
bool init() { bool init() {
mbedtls_ecdh_init(&ecdh); mbedtls_ecdh_init(&ecdh);

View file

@ -406,7 +406,7 @@ class EcdhCharacteristicCallbacks : public NimBLECharacteristicCallbacks {
return; return;
} }
memcpy(clients[idx].aesKey, clients[idx].ecdh.aes_key, AES_KEY_LEN); memcpy(clients[idx].aesKey, clients[idx].ecdh.getAesKey(), AES_KEY_LEN);
uint8_t resp[ECDH_PKT_SIZE]; uint8_t resp[ECDH_PKT_SIZE];
memcpy(resp, clients[idx].ecdh.public_key, ECDH_PUBKEY_LEN); memcpy(resp, clients[idx].ecdh.public_key, ECDH_PUBKEY_LEN);