mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-25 22:05:21 +00:00
Refactor/repo hardening 01 (#462)
* BinaryProtocol: add optional padding control; BitchatPacket API for padded/unpadded bytes; BLEService: use unpadded encoding and remove ad-hoc unpadding on BLE writes * BLEService: balance BLE padding — pad Noise handshake/encrypted frames; leave public/announce/leave unpadded; keep fragmentation consistent with chosen padding * BLEService: replace Timer with DispatchSourceTimer on bleQueue; NostrTransport: cache placeholder NoiseEncryptionService to avoid reallocation * Unify peerID validation: InputValidator handles 16-hex, 64-hex, or alnum-/_; NoiseSecurityValidator now delegates to InputValidator * UI: use standard green for geohash toolbar badge and count (less bright in light mode) * UI: standardize geohash sheet green to app standard (dark: system green, light: darker green) for buttons and checkmark * Docs: align BinaryProtocol compression docs to zlib; Logs: reduce NostrTransport DELIVERED ack logs to debug to cut noise * Tests: add InputValidator peerID coverage and BinaryProtocol padding round-trip/length tests * Project: ensure Xcode project reflects new tests (references added) --------- Co-authored-by: jack <jackjackbits@users.noreply.github.com>
This commit is contained in:
@@ -53,13 +53,9 @@ struct NoiseSecurityValidator {
|
||||
return data.count <= NoiseSecurityConstants.maxHandshakeMessageSize
|
||||
}
|
||||
|
||||
/// Validate peer ID format
|
||||
/// Validate peer ID format using unified validator
|
||||
static func validatePeerID(_ peerID: String) -> Bool {
|
||||
// Peer ID should be reasonable length and contain valid characters
|
||||
let validCharset = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "-_"))
|
||||
return peerID.count > 0 &&
|
||||
peerID.count <= 64 &&
|
||||
peerID.rangeOfCharacter(from: validCharset.inverted) == nil
|
||||
return InputValidator.validatePeerID(peerID)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@
|
||||
///
|
||||
/// ## Compression Strategy
|
||||
/// - Automatic compression for payloads > 256 bytes
|
||||
/// - LZ4 algorithm for speed over ratio
|
||||
/// - zlib compression for broad compatibility on Apple platforms
|
||||
/// - Original size stored for decompression
|
||||
/// - Flag bit indicates compressed payload
|
||||
///
|
||||
@@ -117,7 +117,7 @@ struct BinaryProtocol {
|
||||
}
|
||||
|
||||
// Encode BitchatPacket to binary format
|
||||
static func encode(_ packet: BitchatPacket) -> Data? {
|
||||
static func encode(_ packet: BitchatPacket, padding: Bool = true) -> Data? {
|
||||
var data = Data()
|
||||
|
||||
|
||||
@@ -200,11 +200,14 @@ struct BinaryProtocol {
|
||||
|
||||
|
||||
// Apply padding to standard block sizes for traffic analysis resistance
|
||||
let optimalSize = MessagePadding.optimalBlockSize(for: data.count)
|
||||
let paddedData = MessagePadding.pad(data, toSize: optimalSize)
|
||||
|
||||
|
||||
return paddedData
|
||||
if padding {
|
||||
let optimalSize = MessagePadding.optimalBlockSize(for: data.count)
|
||||
let paddedData = MessagePadding.pad(data, toSize: optimalSize)
|
||||
return paddedData
|
||||
} else {
|
||||
// Caller explicitly requested no padding (e.g., BLE write path)
|
||||
return data
|
||||
}
|
||||
}
|
||||
|
||||
// Decode binary data to BitchatPacket
|
||||
|
||||
@@ -232,8 +232,13 @@ struct BitchatPacket: Codable {
|
||||
BinaryProtocol.encode(self)
|
||||
}
|
||||
|
||||
func toBinaryData(padding: Bool = true) -> Data? {
|
||||
BinaryProtocol.encode(self, padding: padding)
|
||||
}
|
||||
|
||||
// Backward-compatible helper (defaults to padded encoding)
|
||||
func toBinaryData() -> Data? {
|
||||
BinaryProtocol.encode(self)
|
||||
toBinaryData(padding: true)
|
||||
}
|
||||
|
||||
/// Create binary representation for signing (without signature and TTL fields)
|
||||
|
||||
@@ -106,7 +106,7 @@ final class BLEService: NSObject {
|
||||
|
||||
// MARK: - Maintenance Timer
|
||||
|
||||
private weak var maintenanceTimer: Timer? // Single timer for all maintenance tasks
|
||||
private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks
|
||||
private var maintenanceCounter = 0 // Track maintenance cycles
|
||||
|
||||
// MARK: - Peer snapshots publisher (non-UI convenience)
|
||||
@@ -206,10 +206,14 @@ final class BLEService: NSObject {
|
||||
centralManager = CBCentralManager(delegate: self, queue: bleQueue)
|
||||
peripheralManager = CBPeripheralManager(delegate: self, queue: bleQueue)
|
||||
|
||||
// Single maintenance timer for all periodic tasks
|
||||
maintenanceTimer = Timer.scheduledTimer(withTimeInterval: 10.0, repeats: true) { [weak self] _ in
|
||||
// Single maintenance timer for all periodic tasks (dispatch-based for determinism)
|
||||
let timer = DispatchSource.makeTimerSource(queue: bleQueue)
|
||||
timer.schedule(deadline: .now() + 10.0, repeating: 10.0, leeway: .seconds(1))
|
||||
timer.setEventHandler { [weak self] in
|
||||
self?.performMaintenance()
|
||||
}
|
||||
timer.resume()
|
||||
maintenanceTimer = timer
|
||||
|
||||
// Publish initial empty state
|
||||
publishFullPeerData()
|
||||
@@ -224,7 +228,7 @@ final class BLEService: NSObject {
|
||||
// No advertising policy to set; we never include Local Name in adverts.
|
||||
|
||||
deinit {
|
||||
maintenanceTimer?.invalidate()
|
||||
maintenanceTimer?.cancel()
|
||||
centralManager?.stopScan()
|
||||
peripheralManager?.stopAdvertising()
|
||||
#if os(iOS)
|
||||
@@ -308,7 +312,7 @@ final class BLEService: NSObject {
|
||||
)
|
||||
|
||||
// Send immediately to all connected peers
|
||||
if let data = leavePacket.toBinaryData() {
|
||||
if let data = leavePacket.toBinaryData(padding: false) {
|
||||
// Send to peripherals we're connected to as central
|
||||
for state in peripherals.values where state.isConnected {
|
||||
if let characteristic = state.characteristic {
|
||||
@@ -331,7 +335,7 @@ final class BLEService: NSObject {
|
||||
}
|
||||
|
||||
// Stop timer
|
||||
maintenanceTimer?.invalidate()
|
||||
maintenanceTimer?.cancel()
|
||||
maintenanceTimer = nil
|
||||
|
||||
centralManager?.stopScan()
|
||||
@@ -718,12 +722,24 @@ final class BLEService: NSObject {
|
||||
// MARK: - Packet Broadcasting
|
||||
|
||||
private func broadcastPacket(_ packet: BitchatPacket) {
|
||||
guard let rawData = packet.toBinaryData() else {
|
||||
// Balanced privacy: pad sensitive payloads (Noise handshake/encrypted),
|
||||
// keep public/announce/leave unpadded to reduce airtime.
|
||||
let padForBLE: Bool = {
|
||||
if let t = MessageType(rawValue: packet.type) {
|
||||
switch t {
|
||||
case .noiseEncrypted, .noiseHandshake:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
return false
|
||||
}()
|
||||
|
||||
guard let data = packet.toBinaryData(padding: padForBLE) 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
|
||||
@@ -737,7 +753,7 @@ final class BLEService: NSObject {
|
||||
// Check if application-level fragmentation needed for large messages
|
||||
// (CoreBluetooth only handles ATT-level fragmentation for single writes)
|
||||
if data.count > 512 && packet.type != MessageType.fragment.rawValue {
|
||||
sendFragmentedPacket(packet)
|
||||
sendFragmentedPacket(packet, pad: padForBLE)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -832,8 +848,8 @@ final class BLEService: NSObject {
|
||||
// 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)
|
||||
// Fragment via protocol (preserve chosen padding for BLE)
|
||||
sendFragmentedPacket(packet, pad: padForBLE)
|
||||
return
|
||||
}
|
||||
let success = peripheralManager?.updateValue(data, for: characteristic, onSubscribedCentrals: nil) ?? false
|
||||
@@ -886,10 +902,9 @@ final class BLEService: NSObject {
|
||||
|
||||
// MARK: - Fragmentation (Required for messages > BLE MTU)
|
||||
|
||||
private func sendFragmentedPacket(_ packet: BitchatPacket) {
|
||||
guard let encoded = packet.toBinaryData() else { return }
|
||||
// Fragment the unpadded frame; each fragment will be encoded (and padded) independently
|
||||
let fullData = MessagePadding.unpad(encoded)
|
||||
private func sendFragmentedPacket(_ packet: BitchatPacket, pad: Bool) {
|
||||
guard let fullData = packet.toBinaryData(padding: pad) else { return }
|
||||
// Fragment the unpadded frame; each fragment will be encoded independently
|
||||
|
||||
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
|
||||
|
||||
@@ -37,7 +37,11 @@ final class NostrTransport: Transport {
|
||||
func getFingerprint(for peerID: String) -> String? { nil }
|
||||
func getNoiseSessionState(for peerID: String) -> LazyHandshakeState { .none }
|
||||
func triggerHandshake(with peerID: String) { /* no-op */ }
|
||||
func getNoiseService() -> NoiseEncryptionService { NoiseEncryptionService() }
|
||||
// Nostr does not use Noise sessions here; return a cached placeholder to avoid reallocation
|
||||
private static var cachedNoiseService: NoiseEncryptionService = {
|
||||
NoiseEncryptionService()
|
||||
}()
|
||||
func getNoiseService() -> NoiseEncryptionService { Self.cachedNoiseService }
|
||||
|
||||
// Public broadcast not supported over Nostr here
|
||||
func sendMessage(_ content: String, mentions: [String]) { /* no-op */ }
|
||||
@@ -193,7 +197,7 @@ final class NostrTransport: Transport {
|
||||
guard let recipientNpub = recipientNostrPubkey else { return }
|
||||
guard let senderIdentity = try? NostrIdentityBridge.getCurrentNostrIdentity() else { return }
|
||||
SecureLogger.log("NostrTransport: preparing DELIVERED ack for id=\(messageID.prefix(8))… to \(recipientNpub.prefix(16))…",
|
||||
category: SecureLogger.session, level: .info)
|
||||
category: SecureLogger.session, level: .debug)
|
||||
let recipientHex: String
|
||||
do {
|
||||
let (hrp, data) = try Bech32.decode(recipientNpub)
|
||||
@@ -209,7 +213,7 @@ final class NostrTransport: Transport {
|
||||
return
|
||||
}
|
||||
SecureLogger.log("NostrTransport: sending DELIVERED ack giftWrap id=\(event.id.prefix(16))…",
|
||||
category: SecureLogger.session, level: .info)
|
||||
category: SecureLogger.session, level: .debug)
|
||||
NostrRelayManager.shared.sendEvent(event)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,19 +16,17 @@ struct InputValidator {
|
||||
|
||||
// MARK: - Peer ID Validation
|
||||
|
||||
/// Validates a peer ID from any source
|
||||
/// Validates a peer ID from any source (short 16-hex, full 64-hex, or internal alnum/-/_ up to 64)
|
||||
static func validatePeerID(_ peerID: String) -> Bool {
|
||||
// Handle both hex-encoded (from network) and alphanumeric (internal) formats
|
||||
if peerID.count == Limits.hexPeerIDLength {
|
||||
// Network format: 16 hex characters
|
||||
return peerID.allSatisfy { $0.isHexDigit }
|
||||
} else {
|
||||
// Internal format: alphanumeric + dash/underscore
|
||||
let validCharset = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "-_"))
|
||||
return peerID.count > 0 &&
|
||||
peerID.count <= Limits.maxPeerIDLength &&
|
||||
peerID.rangeOfCharacter(from: validCharset.inverted) == nil
|
||||
}
|
||||
// Accept short routing IDs (16-hex)
|
||||
if PeerIDResolver.isShortID(peerID) { return true }
|
||||
// Accept full Noise key hex (64-hex)
|
||||
if PeerIDResolver.isNoiseKeyHex(peerID) { return true }
|
||||
// Internal format: alphanumeric + dash/underscore up to 64
|
||||
let validCharset = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "-_"))
|
||||
return !peerID.isEmpty &&
|
||||
peerID.count <= Limits.maxPeerIDLength &&
|
||||
peerID.rangeOfCharacter(from: validCharset.inverted) == nil
|
||||
}
|
||||
|
||||
// MARK: - String Content Validation
|
||||
|
||||
@@ -938,7 +938,9 @@ struct ContentView: View {
|
||||
switch locationManager.selectedChannel {
|
||||
case .location:
|
||||
let n = viewModel.geohashPeople.count
|
||||
return (n, n > 0 ? Color.green : Color.secondary)
|
||||
// Use standard green (dark: system green; light: custom darker green)
|
||||
let standardGreen = (colorScheme == .dark) ? Color.green : Color(red: 0, green: 0.5, blue: 0)
|
||||
return (n, n > 0 ? standardGreen : Color.secondary)
|
||||
case .mesh:
|
||||
let counts = viewModel.allPeers.reduce(into: (others: 0, mesh: 0)) { counts, peer in
|
||||
guard peer.id != viewModel.meshService.myPeerID else { return }
|
||||
@@ -1037,7 +1039,8 @@ struct ContentView: View {
|
||||
case .mesh:
|
||||
return Color.blue
|
||||
case .location:
|
||||
return Color.green
|
||||
// Standard green to avoid overly bright appearance in light mode
|
||||
return (colorScheme == .dark) ? Color.green : Color(red: 0, green: 0.5, blue: 0)
|
||||
}
|
||||
}()
|
||||
Text(badgeText)
|
||||
|
||||
@@ -6,6 +6,7 @@ struct LocationChannelsSheet: View {
|
||||
@Binding var isPresented: Bool
|
||||
@ObservedObject private var manager = LocationChannelManager.shared
|
||||
@EnvironmentObject var viewModel: ChatViewModel
|
||||
@Environment(\.colorScheme) var colorScheme
|
||||
@State private var customGeohash: String = ""
|
||||
@State private var customError: String? = nil
|
||||
|
||||
@@ -24,10 +25,10 @@ struct LocationChannelsSheet: View {
|
||||
Button(action: { manager.enableLocationChannels() }) {
|
||||
Text("get location and my geohashes")
|
||||
.font(.system(size: 12, design: .monospaced))
|
||||
.foregroundColor(Color.green)
|
||||
.foregroundColor(standardGreen)
|
||||
.frame(maxWidth: .infinity)
|
||||
.padding(.vertical, 6)
|
||||
.background(Color.green.opacity(0.12))
|
||||
.background(standardGreen.opacity(0.12))
|
||||
.cornerRadius(6)
|
||||
}
|
||||
.buttonStyle(.plain)
|
||||
@@ -220,7 +221,7 @@ struct LocationChannelsSheet: View {
|
||||
if isSelected {
|
||||
Text("✔︎")
|
||||
.font(.system(size: 16, design: .monospaced))
|
||||
.foregroundColor(.green)
|
||||
.foregroundColor(standardGreen)
|
||||
}
|
||||
}
|
||||
.frame(maxWidth: .infinity, alignment: .leading)
|
||||
@@ -274,4 +275,11 @@ struct LocationChannelsSheet: View {
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Standardized Colors
|
||||
extension LocationChannelsSheet {
|
||||
private var standardGreen: Color {
|
||||
(colorScheme == .dark) ? Color.green : Color(red: 0, green: 0.5, blue: 0)
|
||||
}
|
||||
}
|
||||
|
||||
#endif
|
||||
|
||||
Reference in New Issue
Block a user