Feature/fragmentation fixes (#453)

* Fix fragmentation + BLE long-write + padding

- Accumulate CBATTRequest long writes by offset and decode once per central
- Decode original packet after fragment reassembly (preserve flags/compression)
- Switch MessagePadding to strict PKCS#7 and validate before unpadding
- Make BinaryProtocol.decode robust: try raw first, then unpad fallback
- Unpad frames before BLE notify; fragment when exceeding centrals' max update length
- Skip notify path when max update length < 21 bytes (protocol minimum)

Verified large PMs and announces route without decode errors and peers show reliably.

* Tests: fix weak delegate lifetime, legacy constants, and unused vars; fragment unpadded frames

- BLEServiceTests: hold strong reference to MockBitchatDelegate
- IntegrationTests: fix inline comment braces; replace removed types with test-safe values; use numeric 0x06 for legacy handshake resp checks
- BinaryProtocolTests: remove unused minResult variable
- BLEService: fragment the unpadded frame so fragments are efficient

* tests: add Noise rehandshake recovery test; document in-memory test bus and autoFlood; stabilize large-network broadcast

* tests: align suite with current behavior (compression, padding, routing, nonce); deflake and stabilize

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
This commit is contained in:
jack
2025-08-19 01:22:21 +02:00
committed by GitHub
co-authored by jack
parent 4c0bb5f93a
commit a30b73dd99
14 changed files with 741 additions and 517 deletions
+12 -5
View File
@@ -209,16 +209,23 @@ struct BinaryProtocol {
// Decode binary data to BitchatPacket
static func decode(_ data: Data) -> BitchatPacket? {
// Remove padding first and create a defensive copy
let unpaddedData = MessagePadding.unpad(data)
// Try decode as-is first (robust when padding wasn't applied)
if let pkt = decodeCore(data) { return pkt }
// If that fails, try after removing padding
let unpadded = MessagePadding.unpad(data)
if unpadded as NSData === data as NSData { return nil }
return decodeCore(unpadded)
}
// Core decoding implementation used by decode(_:) with and without padding removal
private static func decodeCore(_ raw: Data) -> BitchatPacket? {
// Minimum size check: header + senderID
guard unpaddedData.count >= headerSize + senderIDSize else {
guard raw.count >= headerSize + senderIDSize else {
return nil
}
// Convert to array for safer indexed access
let dataArray = Array(unpaddedData)
let dataArray = Array(raw)
var offset = 0
// Header parsing with bounds checks
+13 -28
View File
@@ -74,42 +74,27 @@ struct MessagePadding {
guard data.count < targetSize else { return data }
let paddingNeeded = targetSize - data.count
// PKCS#7 only supports padding up to 255 bytes
// If we need more padding than that, don't pad - return original data
guard paddingNeeded <= 255 else { return data }
// Constrain to 255 to fit a single-byte pad length marker
guard paddingNeeded > 0 && paddingNeeded <= 255 else { return data }
var padded = data
// Standard PKCS#7 padding
var randomBytes = [UInt8](repeating: 0, count: paddingNeeded - 1)
_ = SecRandomCopyBytes(kSecRandomDefault, paddingNeeded - 1, &randomBytes)
padded.append(contentsOf: randomBytes)
padded.append(UInt8(paddingNeeded))
// PKCS#7: All pad bytes are equal to the pad length
padded.append(contentsOf: Array(repeating: UInt8(paddingNeeded), count: paddingNeeded))
return padded
}
// Remove padding from data
static func unpad(_ data: Data) -> Data {
guard !data.isEmpty else { return data }
// Last byte tells us how much padding to remove
let lastIndex = data.count - 1
guard lastIndex >= 0 else { return data }
let paddingLength = Int(data[lastIndex])
guard paddingLength > 0 && paddingLength <= data.count else {
// No valid padding, return original data
return data
}
// Create a new Data object (not a subsequence) for thread safety
let unpaddedLength = data.count - paddingLength
guard unpaddedLength >= 0 else { return Data() }
// Return a proper copy, not a subsequence
return Data(data.prefix(unpaddedLength))
let last = data.last!
let paddingLength = Int(last)
// Must have at least 1 pad byte and not exceed data length
guard paddingLength > 0 && paddingLength <= data.count else { return data }
// Verify PKCS#7: all last N bytes equal to pad length
let start = data.count - paddingLength
let tail = data[start...]
for b in tail { if b != last { return data } }
return Data(data[..<start])
}
// Find optimal block size for data
+116 -63
View File
@@ -96,6 +96,9 @@ final class BLEService: NSObject {
// Queue for notifications that failed due to full queue
private var pendingNotifications: [(data: Data, centrals: [CBCentral]?)] = []
// Accumulate long write chunks per central until a full frame decodes
private var pendingWriteBuffers: [String: Data] = [:]
// MARK: - Maintenance Timer
@@ -690,10 +693,12 @@ final class BLEService: NSObject {
// MARK: - Packet Broadcasting
private func broadcastPacket(_ packet: BitchatPacket) {
guard let data = packet.toBinaryData() else {
guard let rawData = packet.toBinaryData() else {
SecureLogger.log("❌ Failed to convert packet to binary data", category: SecureLogger.session, level: .error)
return
}
// Avoid sending padded data over BLE to reduce truncation risk
let data = MessagePadding.unpad(rawData)
// Only log broadcasts for non-announce packets
// Log encrypted and relayed packets for debugging
@@ -795,6 +800,17 @@ final class BLEService: NSObject {
packet.type == MessageType.leave.rawValue ||
packet.type == MessageType.noiseHandshake.rawValue
if isBroadcastType, let characteristic = characteristic, !subscribedCentrals.isEmpty {
// If value exceeds minimum allowed by connected centrals, handle per constraints
let minAllowed = subscribedCentrals.map { $0.maximumUpdateValueLength }.min() ?? 20
// Minimum BitChat frame = 13 (header) + 8 (senderID) = 21 bytes
if minAllowed < 21 {
// Cannot deliver any BitChat frame via notifications on this link; skip notify
SecureLogger.log("⚠️ Skipping notify: central max update length (\(minAllowed)) < 21", category: SecureLogger.session, level: .debug)
} else if data.count > minAllowed {
// Fragment via protocol
sendFragmentedPacket(packet)
return
}
let success = peripheralManager?.updateValue(data, for: characteristic, onSubscribedCentrals: nil) ?? false
if success {
sentToCentrals = subscribedCentrals.count
@@ -845,7 +861,9 @@ final class BLEService: NSObject {
// MARK: - Fragmentation (Required for messages > BLE MTU)
private func sendFragmentedPacket(_ packet: BitchatPacket) {
guard let fullData = packet.toBinaryData() else { return }
guard let encoded = packet.toBinaryData() else { return }
// Fragment the unpadded frame; each fragment will be encoded (and padded) independently
let fullData = MessagePadding.unpad(encoded)
let fragmentID = Data((0..<8).map { _ in UInt8.random(in: 0...255) })
let fragments = stride(from: 0, to: fullData.count, by: maxFragmentSize).map { offset in
@@ -881,23 +899,34 @@ final class BLEService: NSObject {
return
}
guard packet.payload.count > 13 else { return }
// Minimum header: 8 bytes ID + 2 index + 2 total + 1 type
guard packet.payload.count >= 13 else { return }
let senderHex = packet.senderID.hexEncodedString()
let fragmentID = packet.payload[0..<8].map { String(format: "%02x", $0) }.joined()
let index = Int(packet.payload[8..<10].withUnsafeBytes { $0.load(as: UInt16.self).bigEndian })
let total = Int(packet.payload[10..<12].withUnsafeBytes { $0.load(as: UInt16.self).bigEndian })
// Parse big-endian UInt16 safely without alignment assumptions
let idxHi = UInt16(packet.payload[8])
let idxLo = UInt16(packet.payload[9])
let index = Int((idxHi << 8) | idxLo)
let totHi = UInt16(packet.payload[10])
let totLo = UInt16(packet.payload[11])
let total = Int((totHi << 8) | totLo)
let originalType = packet.payload[12]
let fragmentData = packet.payload[13...]
let fragmentData = packet.payload.suffix(from: 13)
// Sanity checks
guard total > 0 && index >= 0 && index < total else { return }
// Store fragment
if incomingFragments[fragmentID] == nil {
incomingFragments[fragmentID] = [:]
fragmentMetadata[fragmentID] = (originalType, total, Date())
let key = "\(senderHex):\(fragmentID)"
if incomingFragments[key] == nil {
incomingFragments[key] = [:]
fragmentMetadata[key] = (originalType, total, Date())
}
incomingFragments[fragmentID]?[index] = Data(fragmentData)
incomingFragments[key]?[index] = Data(fragmentData)
// Check if complete
if let fragments = incomingFragments[fragmentID],
if let fragments = incomingFragments[key],
fragments.count == total {
// Reassemble
var reassembled = Data()
@@ -907,23 +936,16 @@ final class BLEService: NSObject {
}
}
// Process reassembled packet
if let metadata = fragmentMetadata[fragmentID] {
let reassembledPacket = BitchatPacket(
type: metadata.type,
senderID: packet.senderID,
recipientID: packet.recipientID,
timestamp: packet.timestamp,
payload: reassembled,
signature: packet.signature,
ttl: packet.ttl > 0 ? packet.ttl - 1 : 0
)
handleReceivedPacket(reassembledPacket, from: peerID)
// Decode the original packet bytes we reassembled, so flags/compression are preserved
if let originalPacket = BinaryProtocol.decode(reassembled) {
handleReceivedPacket(originalPacket, from: peerID)
} else {
SecureLogger.log("❌ Failed to decode reassembled packet (type=\(originalType), total=\(total))", category: SecureLogger.session, level: .error)
}
// Cleanup
incomingFragments.removeValue(forKey: fragmentID)
fragmentMetadata.removeValue(forKey: fragmentID)
incomingFragments.removeValue(forKey: key)
fragmentMetadata.removeValue(forKey: key)
}
}
@@ -943,7 +965,8 @@ final class BLEService: NSObject {
}
// Efficient deduplication
if messageDeduplicator.isDuplicate(messageID) {
// Important: do not dedup fragment packets globally (each piece must pass)
if packet.type != MessageType.fragment.rawValue && messageDeduplicator.isDuplicate(messageID) {
// Announce packets (type 1) are sent every 10 seconds for peer discovery
// It's normal to see these as duplicates - don't log them to reduce noise
if packet.type != MessageType.announce.rawValue {
@@ -1629,6 +1652,21 @@ extension BLEService: CBCentralManagerDelegate {
}
}
#if DEBUG
// Test-only helper to inject packets into the receive pipeline
extension BLEService {
func _test_handlePacket(_ packet: BitchatPacket, fromPeerID: String) {
if DispatchQueue.getSpecific(key: messageQueueKey) != nil {
handleReceivedPacket(packet, from: fromPeerID)
} else {
messageQueue.async { [weak self] in
self?.handleReceivedPacket(packet, from: fromPeerID)
}
}
}
}
#endif
// MARK: - CBPeripheralDelegate
extension BLEService: CBPeripheralDelegate {
@@ -1942,52 +1980,67 @@ extension BLEService: CBPeripheralManagerDelegate {
peripheral.respond(to: request, withResult: .success)
}
// Process directly on main thread to avoid deadlocks (matches original implementation)
for request in requests {
guard let data = request.value, !data.isEmpty else {
SecureLogger.log("⚠️ Empty write request", category: SecureLogger.session, level: .warning)
continue
// Process writes. For long writes, CoreBluetooth may deliver multiple CBATTRequest values with offsets.
// Combine per-central request values by offset before decoding.
// Process directly on our message queue to match transport context
let grouped = Dictionary(grouping: requests, by: { $0.central.identifier.uuidString })
for (centralUUID, group) in grouped {
// Sort by offset ascending
let sorted = group.sorted { $0.offset < $1.offset }
let hasMultiple = sorted.count > 1 || (sorted.first?.offset ?? 0) > 0
// Always merge into a persistent per-central buffer to handle multi-callback long writes
var combined = pendingWriteBuffers[centralUUID] ?? Data()
var appendedBytes = 0
var offsets: [Int] = []
for r in sorted {
guard let chunk = r.value, !chunk.isEmpty else { continue }
offsets.append(r.offset)
let end = r.offset + chunk.count
if combined.count < end {
combined.append(Data(repeating: 0, count: end - combined.count))
}
// Write chunk into the correct position (supports out-of-order and overlapping writes)
combined.replaceSubrange(r.offset..<end, with: chunk)
appendedBytes += chunk.count
}
// Suppress logs for announce packets to reduce noise
// Peek at packet type without full decode
if data.count > 0 && data[0] != MessageType.announce.rawValue {
SecureLogger.log("📥 Processing write from central: \(data.count) bytes", category: SecureLogger.session, level: .debug)
pendingWriteBuffers[centralUUID] = combined
// Peek type byte for debug: version is at 0, type at 1 when well-formed
if combined.count >= 2 {
let peekType = combined[1]
if peekType != MessageType.announce.rawValue {
SecureLogger.log("📥 Accumulated write from central \(centralUUID): size=\(combined.count) (+\(appendedBytes)) bytes (type=\(peekType)), offsets=\(offsets)", category: SecureLogger.session, level: .debug)
}
}
if let packet = BinaryProtocol.decode(data) {
// Use the packet's senderID as the peer identifier
// Try decode the accumulated buffer
if let packet = BinaryProtocol.decode(combined) {
// Clear buffer on success
pendingWriteBuffers.removeValue(forKey: centralUUID)
let senderID = packet.senderID.hexEncodedString()
// Only log non-announce packets
if packet.type != MessageType.announce.rawValue {
SecureLogger.log("📦 Decoded packet type: \(packet.type) from sender: \(senderID)", category: SecureLogger.session, level: .debug)
SecureLogger.log("📦 Decoded (combined) packet type: \(packet.type) from sender: \(senderID)", category: SecureLogger.session, level: .debug)
}
// Store central in our list if not already there
if !subscribedCentrals.contains(request.central) {
subscribedCentrals.append(request.central)
if !subscribedCentrals.contains(sorted[0].central) {
subscribedCentrals.append(sorted[0].central)
}
let centralUUID = request.central.identifier.uuidString
// Update mapping ONLY for announce packets that come directly from the peer (not relayed)
if packet.type == MessageType.announce.rawValue {
// Only update mapping if this is a direct announce (TTL == messageTTL means not relayed)
if packet.ttl == messageTTL {
centralToPeerID[centralUUID] = senderID
// Mapping update - direct announce from peer
}
// Process the announce packet regardless of whether we updated the mapping
if packet.ttl == messageTTL { centralToPeerID[centralUUID] = senderID }
handleReceivedPacket(packet, from: senderID)
} else {
// For non-announce packets, DO NOT update the mapping
// These could be relayed packets from other peers in the mesh
// The centralToPeerID mapping should only be set by announce packets
// Always use the packet's original senderID
handleReceivedPacket(packet, from: senderID)
}
} else {
SecureLogger.log("❌ Failed to decode packet from central, full data: \(data.map { String(format: "%02x", $0) }.joined(separator: " "))", category: SecureLogger.session, level: .error)
// If buffer grows suspiciously large, reset to avoid memory leak
if combined.count > 1_000_000 { // 1MB cap for safety
pendingWriteBuffers.removeValue(forKey: centralUUID)
SecureLogger.log("⚠️ Dropping oversized pending write buffer (\(combined.count) bytes) for central \(centralUUID)", category: SecureLogger.session, level: .warning)
}
// If this was a single short write and still failed, log the raw chunk for debugging
if !hasMultiple, let only = sorted.first, let raw = only.value {
SecureLogger.log("❌ Failed to decode packet from central, full data: \(raw.map { String(format: "%02x", $0) }.joined(separator: " "))", category: SecureLogger.session, level: .error)
}
}
}
}
+2 -2
View File
@@ -13,7 +13,7 @@ struct CompressionUtil {
// Compression threshold - don't compress if data is smaller than this
static let compressionThreshold = 100 // bytes
// Compress data using LZ4 algorithm (fast compression/decompression)
// Compress data using zlib algorithm (most compatible)
static func compress(_ data: Data) -> Data? {
// Skip compression for small data
guard data.count >= compressionThreshold else { return nil }
@@ -36,7 +36,7 @@ struct CompressionUtil {
return Data(bytes: destinationBuffer, count: compressedSize)
}
// Decompress LZ4 compressed data
// Decompress zlib compressed data
static func decompress(_ compressedData: Data, originalSize: Int) -> Data? {
let destinationBuffer = UnsafeMutablePointer<UInt8>.allocate(capacity: originalSize)
defer { destinationBuffer.deallocate() }