diff --git a/damus/Core/Nostr/NostrEvent.swift b/damus/Core/Nostr/NostrEvent.swift index 225cd5f8..d3ea206d 100644 --- a/damus/Core/Nostr/NostrEvent.swift +++ b/damus/Core/Nostr/NostrEvent.swift @@ -334,6 +334,27 @@ func decode_nostr_event(txt: String) -> NostrResponse? { return NostrResponse.owned_from_json(json: txt) } +func decode_and_verify_nostr_response(txt: String) -> NostrResponse? { + guard let response = NostrResponse.owned_from_json(json: txt) else { return nil } + guard verify_nostr_response(response: response) == true else { return nil } + return response +} + +func verify_nostr_response(response: borrowing NostrResponse) -> Bool { + switch response { + case .event(_, let event): + return event.verify() + case .notice(_): + return true + case .eose(_): + return true + case .ok(_): + return true + case .auth(_): + return true + } +} + func encode_json(_ val: T) -> String? { let encoder = JSONEncoder() encoder.outputFormatting = .withoutEscapingSlashes diff --git a/damus/Core/Nostr/RelayConnection.swift b/damus/Core/Nostr/RelayConnection.swift index 1df94875..987e1bd5 100644 --- a/damus/Core/Nostr/RelayConnection.swift +++ b/damus/Core/Nostr/RelayConnection.swift @@ -9,8 +9,32 @@ import Combine import Foundation enum NostrConnectionEvent { - case ws_event(WebSocketEvent) + /// Other non-message websocket events + case ws_connection_event(WSConnectionEvent) + /// A nostr response case nostr_event(NostrResponse) + + /// Models non-messaging websocket events + /// + /// Implementation note: Messaging events should use `.nostr_event` in `NostrConnectionEvent` + enum WSConnectionEvent { + case connected + case disconnected(URLSessionWebSocketTask.CloseCode, String?) + case error(Error) + + static func from(full_ws_event: WebSocketEvent) -> Self? { + switch full_ws_event { + case .connected: + return .connected + case .message(_): + return nil + case .disconnected(let closeCode, let string): + return .disconnected(closeCode, string) + case .error(let error): + return .error(error) + } + } + } } final class RelayConnection: ObservableObject { @@ -31,11 +55,11 @@ final class RelayConnection: ObservableObject { init(url: RelayURL, handleEvent: @escaping (NostrConnectionEvent) -> (), - processEvent: @escaping (WebSocketEvent) -> ()) + processUnverifiedWSEvent: @escaping (WebSocketEvent) -> ()) { self.relay_url = url self.handleEvent = handleEvent - self.processEvent = processEvent + self.processEvent = processUnverifiedWSEvent } func ping() { @@ -115,6 +139,7 @@ final class RelayConnection: ObservableObject { } private func receive(event: WebSocketEvent) { + assert(!Thread.isMainThread, "This code must not be executed on the main thread") processEvent(event) switch event { case .connected: @@ -152,7 +177,8 @@ final class RelayConnection: ObservableObject { } } DispatchQueue.main.async { - self.handleEvent(.ws_event(event)) + guard let ws_connection_event = NostrConnectionEvent.WSConnectionEvent.from(full_ws_event: event) else { return } + self.handleEvent(.ws_connection_event(ws_connection_event)) } if let description = event.description { @@ -190,7 +216,9 @@ final class RelayConnection: ObservableObject { private func receive(message: URLSessionWebSocketTask.Message) { switch message { case .string(let messageString): - if let ev = decode_nostr_event(txt: messageString) { + // NOTE: Once we switch to the local relay model, + // we will not need to verify nostr events at this point. + if let ev = decode_and_verify_nostr_response(txt: messageString) { DispatchQueue.main.async { self.handleEvent(.nostr_event(ev)) } diff --git a/damus/Core/Nostr/RelayPool.swift b/damus/Core/Nostr/RelayPool.swift index 6aafda85..5f74727a 100644 --- a/damus/Core/Nostr/RelayPool.swift +++ b/damus/Core/Nostr/RelayPool.swift @@ -126,7 +126,7 @@ class RelayPool { } let conn = RelayConnection(url: desc.url, handleEvent: { event in self.handle_event(relay_id: relay_id, event: event) - }, processEvent: { wsev in + }, processUnverifiedWSEvent: { wsev in guard case .message(let msg) = wsev, case .string(let str) = msg else { return } @@ -214,9 +214,9 @@ class RelayPool { var eoseSent = false self.subscribe(sub_id: sub_id, filters: filters, handler: { (relayUrl, connectionEvent) in switch connectionEvent { - case .ws_event(let ev): + case .ws_connection_event(let ev): // Websocket events such as connect/disconnect/error are already handled in `RelayConnection`. Do not perform any handling here. - // For the future, perhaps we should abstract away `.ws_event` in `RelayPool`? Seems like something to be handled on the `RelayConnection` layer. + // For the future, perhaps we should abstract away `.ws_connection_event` in `RelayPool`? Seems like something to be handled on the `RelayConnection` layer. break case .nostr_event(let nostrResponse): guard nostrResponse.subid == sub_id else { return } // Do not stream items that do not belong in this subscription @@ -366,7 +366,7 @@ class RelayPool { record_seen(relay_id: relay_id, event: event) // run req queue when we reconnect - if case .ws_event(let ws) = event { + if case .ws_connection_event(let ws) = event { if case .connected = ws { run_queue(relay_id) } diff --git a/damus/Features/Onboarding/Views/SaveKeysView.swift b/damus/Features/Onboarding/Views/SaveKeysView.swift index 7ceb5765..3e92c8de 100644 --- a/damus/Features/Onboarding/Views/SaveKeysView.swift +++ b/damus/Features/Onboarding/Views/SaveKeysView.swift @@ -162,7 +162,7 @@ struct SaveKeysView: View { func handle_event(relay: RelayURL, ev: NostrConnectionEvent) { switch ev { - case .ws_event(let wsev): + case .ws_connection_event(let wsev): switch wsev { case .connected: let metadata = create_account_to_metadata(account) diff --git a/damus/Features/Profile/Models/ProfileModel.swift b/damus/Features/Profile/Models/ProfileModel.swift index b4f0eab7..0bfb5519 100644 --- a/damus/Features/Profile/Models/ProfileModel.swift +++ b/damus/Features/Profile/Models/ProfileModel.swift @@ -155,7 +155,7 @@ class ProfileModel: ObservableObject, Equatable { private func handle_event(relay_id: RelayURL, ev: NostrConnectionEvent) { switch ev { - case .ws_event: + case .ws_connection_event: return case .nostr_event(let resp): guard resp.subid == self.sub_id || resp.subid == self.prof_subid || resp.subid == self.conversations_subid else { diff --git a/damus/Features/Search/Models/SearchHomeModel.swift b/damus/Features/Search/Models/SearchHomeModel.swift index 3fd356cf..a8393865 100644 --- a/damus/Features/Search/Models/SearchHomeModel.swift +++ b/damus/Features/Search/Models/SearchHomeModel.swift @@ -150,7 +150,7 @@ func load_profiles(context: String, profiles_subid: String, relay_id: RelayUR let now = UInt64(Date.now.timeIntervalSince1970) switch conn_ev { - case .ws_event: + case .ws_connection_event: break case .nostr_event(let ev): guard ev.subid == profiles_subid, rid == relay_id else { return } diff --git a/damus/Features/Search/Models/SearchModel.swift b/damus/Features/Search/Models/SearchModel.swift index 03e1851c..fbb96ab9 100644 --- a/damus/Features/Search/Models/SearchModel.swift +++ b/damus/Features/Search/Models/SearchModel.swift @@ -109,7 +109,7 @@ func event_matches_filter(_ ev: NostrEvent, filter: NostrFilter) -> Bool { func handle_subid_event(pool: RelayPool, relay_id: RelayURL, ev: NostrConnectionEvent, handle: (String, NostrEvent) -> ()) -> (String?, Bool) { switch ev { - case .ws_event: + case .ws_connection_event: return (nil, false) case .nostr_event(let res): diff --git a/damus/Features/Timeline/Models/HomeModel.swift b/damus/Features/Timeline/Models/HomeModel.swift index 1cd31491..a18fd438 100644 --- a/damus/Features/Timeline/Models/HomeModel.swift +++ b/damus/Features/Timeline/Models/HomeModel.swift @@ -460,7 +460,7 @@ class HomeModel: ContactsDelegate { @MainActor func handle_event(relay_id: RelayURL, conn_event: NostrConnectionEvent) { switch conn_event { - case .ws_event(let ev): + case .ws_connection_event(let ev): switch ev { case .connected: if !done_init { diff --git a/damusTests/RequestTests.swift b/damusTests/RequestTests.swift index aa5348b6..b86bb981 100644 --- a/damusTests/RequestTests.swift +++ b/damusTests/RequestTests.swift @@ -22,7 +22,7 @@ final class RequestTests: XCTestCase { let url = RelayURL("wss://example.com")! let relayDescriptor = RelayPool.RelayDescriptor(url: url, info: .readWrite) let relayConnection = RelayConnection(url: url) { _ in - } processEvent: { _ in + } processUnverifiedWSEvent: { _ in } let relay = RelayPool.Relay(descriptor: relayDescriptor, connection: relayConnection) diff --git a/damusTests/damusTests.swift b/damusTests/damusTests.swift index 4c6d708c..1ee83b30 100644 --- a/damusTests/damusTests.swift +++ b/damusTests/damusTests.swift @@ -18,6 +18,51 @@ class damusTests: XCTestCase { override func tearDownWithError() throws { // Put teardown code here. This method is called after the invocation of each test method in the class. } + + func testEventVerify() throws { + let test_valid_note_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_sig_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f153","tags":[]} + """ + let test_invalid_note_tampered_id_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e600","created_at":1753898578,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_date_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898579,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_pubkey_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084b","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_content_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1103","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_kind_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1102","kind":2,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[]} + """ + let test_invalid_note_tampered_tags_text = """ + {"id":"f4a5635d78d4c1ec2bf7d15d33bd8d5e0afdb8a5a24047f095842281c744e6a3","created_at":1753898578,"content":"Test 1102","kind":1,"pubkey":"056b5b5966f500defb3b790a14633e5ec4a0e8883ca29bc23d0030553edb084a","sig":"d03f0beee7355a8b6ce437b43e01f2d3be8c0f3f17b41a8dec8a9b9804d44ab639b7906c545e4b51820f00b09d00cfa5058916e93126e8a11a65e2623f95f152","tags":[["t", "foo"]]} + """ + + let test_valid_note = NdbNote.owned_from_json(json: test_valid_note_text)! + let test_invalid_note_tampered_sig = NdbNote.owned_from_json(json: test_invalid_note_tampered_sig_text)! + var test_invalid_note_tampered_id = NdbNote.owned_from_json(json: test_invalid_note_tampered_id_text)! + let test_invalid_note_tampered_date = NdbNote.owned_from_json(json: test_invalid_note_tampered_date_text)! + let test_invalid_note_tampered_pubkey = NdbNote.owned_from_json(json: test_invalid_note_tampered_pubkey_text)! + let test_invalid_note_tampered_content = NdbNote.owned_from_json(json: test_invalid_note_tampered_content_text)! + let test_invalid_note_tampered_kind = NdbNote.owned_from_json(json: test_invalid_note_tampered_kind_text)! + let test_invalid_note_tampered_tags = NdbNote.owned_from_json(json: test_invalid_note_tampered_tags_text)! + + XCTAssertEqual(test_valid_note.verify(), true) + XCTAssertEqual(test_invalid_note_tampered_sig.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_id.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_date.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_pubkey.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_content.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_kind.verify(), false) + XCTAssertEqual(test_invalid_note_tampered_tags.verify(), false) + } func testIdEquality() throws { let pubkey = test_pubkey diff --git a/nostrdb/NdbNote.swift b/nostrdb/NdbNote.swift index 4ec2345b..2ce94553 100644 --- a/nostrdb/NdbNote.swift +++ b/nostrdb/NdbNote.swift @@ -345,6 +345,28 @@ class NdbNote: Codable, Equatable, Hashable { json: cstr, json_len: UInt32(json.utf8.count), bufsize: bufsize) } } + + func verify() -> Bool { + let scratch_buf_len = MAX_NOTE_SIZE + let scratch_buf = malloc(scratch_buf_len) + defer { free(scratch_buf) } // Ensure we deallocate as soon as we leave this scope, regardless of the outcome + + let current_id = self.id + + // Calculate the ID based on the content + guard ndb_calculate_id(self.note.ptr, scratch_buf, Int32(scratch_buf_len)) == 1 else { return false } + + let computed_id = self.id + + // Ensure computed ID matches given id to prevent ID tampering + guard computed_id == current_id else { return false } + + // Verify the signature against the pubkey and the computed ID, to verify the validity of the whole note + var ctx = secp256k1_context_create(UInt32(SECP256K1_CONTEXT_VERIFY)) + guard ndb_note_verify(&ctx, ndb_note_pubkey(self.note.ptr), ndb_note_id(self.note.ptr), ndb_note_sig(self.note.ptr)) == 1 else { return false } + + return true + } static func owned_from_json_cstr(json: UnsafePointer, json_len: UInt32, bufsize: Int = 2 << 18) -> NdbNote? { let data = malloc(bufsize)