Fixed an edge-case crash, a couple of minor bugs and made future-proofing tweaks

Fixed a bit of the OnionRequest error handling to better send through server error messages for debugging
Fixed a bug where the initial offset could be negative if the number of messages was less than the page size resulting in a crash
Fixed a crash due to a code path which was thought to be impossible exiting but is actually possible (so just erroring)
Added the 'expire' SnodeAPI endpoint
Removed the 'openGroupServerTimestamp' property (was unused and just added confusion)
Updated the logic to always handle the 'fileId' for uploads/downloads as a string instead of casting it to an Int64
Updated the OpenGroup room parsing to support either Int or String values for image ids
pull/612/head
Morgan Pretty 3 years ago
parent b4ab521713
commit 5b6be3912d

@ -1461,10 +1461,15 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers
var lastSize: CGSize = .zero var lastSize: CGSize = .zero
self.tableView.afterNextLayoutSubviews( self.tableView.afterNextLayoutSubviews(
when: { [weak self] a, b, updatedContentSize in when: { [weak self] numSections, numRowInSections, updatedContentSize in
guard (CACurrentMediaTime() - initialUpdateTime) < 2 && lastSize != updatedContentSize else { // If too much time has passed or the section/row count doesn't match then
return true // just stop the callback
} guard
(CACurrentMediaTime() - initialUpdateTime) < 2 &&
lastSize != updatedContentSize &&
numSections > targetIndexPath.section &&
numRowInSections[targetIndexPath.section] > targetIndexPath.row
else { return true }
lastSize = updatedContentSize lastSize = updatedContentSize

@ -245,7 +245,7 @@ extension OpenGroupSuggestionGrid {
label.text = room.name label.text = room.name
// Only continue if we have a room image // Only continue if we have a room image
guard let imageId: Int64 = room.imageId else { guard let imageId: String = room.imageId else {
imageView.isHidden = true imageView.isHidden = true
return return
} }

@ -141,7 +141,7 @@ public enum SMKLegacy {
internal var sender: String? internal var sender: String?
internal var groupPublicKey: String? internal var groupPublicKey: String?
internal var openGroupServerMessageID: UInt64? internal var openGroupServerMessageID: UInt64?
internal var openGroupServerTimestamp: UInt64? internal var openGroupServerTimestamp: UInt64? // Not used for anything
internal var serverHash: String? internal var serverHash: String?
// MARK: NSCoding // MARK: NSCoding
@ -175,7 +175,6 @@ public enum SMKLegacy {
result.sender = self.sender result.sender = self.sender
result.groupPublicKey = self.groupPublicKey result.groupPublicKey = self.groupPublicKey
result.openGroupServerMessageId = self.openGroupServerMessageID result.openGroupServerMessageId = self.openGroupServerMessageID
result.openGroupServerTimestamp = self.openGroupServerTimestamp
result.serverHash = self.serverHash result.serverHash = self.serverHash
return result return result

@ -41,11 +41,11 @@ public final class FileServerAPI: NSObject {
.decoded(as: FileUploadResponse.self, on: .global(qos: .userInitiated)) .decoded(as: FileUploadResponse.self, on: .global(qos: .userInitiated))
} }
public static func download(_ file: Int64, useOldServer: Bool) -> Promise<Data> { public static func download(_ fileId: String, useOldServer: Bool) -> Promise<Data> {
let serverPublicKey: String = (useOldServer ? oldServerPublicKey : serverPublicKey) let serverPublicKey: String = (useOldServer ? oldServerPublicKey : serverPublicKey)
let request = Request<NoBody, Endpoint>( let request = Request<NoBody, Endpoint>(
server: (useOldServer ? oldServer : server), server: (useOldServer ? oldServer : server),
endpoint: .fileIndividual(fileId: file) endpoint: .fileIndividual(fileId: fileId)
) )
return send(request, serverPublicKey: serverPublicKey) return send(request, serverPublicKey: serverPublicKey)

@ -5,7 +5,7 @@ import Foundation
extension FileServerAPI { extension FileServerAPI {
public enum Endpoint: EndpointType { public enum Endpoint: EndpointType {
case file case file
case fileIndividual(fileId: Int64) case fileIndividual(fileId: String)
case sessionVersion case sessionVersion
var path: String { var path: String {

@ -87,8 +87,10 @@ public enum AttachmentDownloadJob: JobExecutor {
let downloadPromise: Promise<Data> = { let downloadPromise: Promise<Data> = {
guard guard
let downloadUrl: String = attachment.downloadUrl, let downloadUrl: String = attachment.downloadUrl,
let fileAsString: String = downloadUrl.split(separator: "/").last.map({ String($0) }), let fileId: String = downloadUrl
let file: Int64 = Int64(fileAsString) .split(separator: "/")
.last
.map({ String($0) })
else { else {
return Promise(error: AttachmentDownloadError.invalidUrl) return Promise(error: AttachmentDownloadError.invalidUrl)
} }
@ -98,13 +100,13 @@ public enum AttachmentDownloadJob: JobExecutor {
return nil // Not an open group so just use standard FileServer upload return nil // Not an open group so just use standard FileServer upload
} }
return OpenGroupAPI.downloadFile(db, fileId: file, from: openGroup.roomToken, on: openGroup.server) return OpenGroupAPI.downloadFile(db, fileId: fileId, from: openGroup.roomToken, on: openGroup.server)
.map { _, data in data } .map { _, data in data }
}) })
return ( return (
maybeOpenGroupDownloadPromise ?? maybeOpenGroupDownloadPromise ??
FileServerAPI.download(file, useOldServer: downloadUrl.contains(FileServerAPI.oldServer)) FileServerAPI.download(fileId, useOldServer: downloadUrl.contains(FileServerAPI.oldServer))
) )
}() }()

@ -14,7 +14,6 @@ public class Message: Codable {
public var sender: String? public var sender: String?
public var groupPublicKey: String? public var groupPublicKey: String?
public var openGroupServerMessageId: UInt64? public var openGroupServerMessageId: UInt64?
public var openGroupServerTimestamp: UInt64?
public var serverHash: String? public var serverHash: String?
public var ttl: UInt64 { 14 * 24 * 60 * 60 * 1000 } public var ttl: UInt64 { 14 * 24 * 60 * 60 * 1000 }
@ -41,7 +40,6 @@ public class Message: Codable {
sender: String? = nil, sender: String? = nil,
groupPublicKey: String? = nil, groupPublicKey: String? = nil,
openGroupServerMessageId: UInt64? = nil, openGroupServerMessageId: UInt64? = nil,
openGroupServerTimestamp: UInt64? = nil,
serverHash: String? = nil serverHash: String? = nil
) { ) {
self.id = id self.id = id
@ -52,7 +50,6 @@ public class Message: Codable {
self.sender = sender self.sender = sender
self.groupPublicKey = groupPublicKey self.groupPublicKey = groupPublicKey
self.openGroupServerMessageId = openGroupServerMessageId self.openGroupServerMessageId = openGroupServerMessageId
self.openGroupServerTimestamp = openGroupServerTimestamp
self.serverHash = serverHash self.serverHash = serverHash
} }

@ -70,7 +70,7 @@ extension OpenGroupAPI {
/// File ID of an uploaded file containing the room's image /// File ID of an uploaded file containing the room's image
/// ///
/// Omitted if there is no image /// Omitted if there is no image
public let imageId: Int64? public let imageId: String?
/// Array of pinned message information (omitted entirely if there are no pinned messages) /// Array of pinned message information (omitted entirely if there are no pinned messages)
public let pinnedMessages: [PinnedMessage]? public let pinnedMessages: [PinnedMessage]?
@ -150,6 +150,12 @@ extension OpenGroupAPI.Room {
public init(from decoder: Decoder) throws { public init(from decoder: Decoder) throws {
let container: KeyedDecodingContainer<CodingKeys> = try decoder.container(keyedBy: CodingKeys.self) let container: KeyedDecodingContainer<CodingKeys> = try decoder.container(keyedBy: CodingKeys.self)
// This logic is to future-proof the transition from int-based to string-based image ids
let maybeImageId: String? = (
((try? container.decode(Int64.self, forKey: .imageId)).map { "\($0)" }) ??
(try? container.decode(String.self, forKey: .imageId))
)
self = OpenGroupAPI.Room( self = OpenGroupAPI.Room(
token: try container.decode(String.self, forKey: .token), token: try container.decode(String.self, forKey: .token),
name: try container.decode(String.self, forKey: .name), name: try container.decode(String.self, forKey: .name),
@ -160,7 +166,7 @@ extension OpenGroupAPI.Room {
activeUsers: try container.decode(Int64.self, forKey: .activeUsers), activeUsers: try container.decode(Int64.self, forKey: .activeUsers),
activeUsersCutoff: try container.decode(Int64.self, forKey: .activeUsersCutoff), activeUsersCutoff: try container.decode(Int64.self, forKey: .activeUsersCutoff),
imageId: try? container.decode(Int64.self, forKey: .imageId), imageId: maybeImageId,
pinnedMessages: try? container.decode([OpenGroupAPI.PinnedMessage].self, forKey: .pinnedMessages), pinnedMessages: try? container.decode([OpenGroupAPI.PinnedMessage].self, forKey: .pinnedMessages),
admin: ((try? container.decode(Bool.self, forKey: .admin)) ?? false), admin: ((try? container.decode(Bool.self, forKey: .admin)) ?? false),

@ -700,7 +700,7 @@ public enum OpenGroupAPI {
public static func downloadFile( public static func downloadFile(
_ db: Database, _ db: Database,
fileId: Int64, fileId: String,
from roomToken: String, from roomToken: String,
on server: String, on server: String,
using dependencies: SMKDependencies = SMKDependencies() using dependencies: SMKDependencies = SMKDependencies()

@ -446,10 +446,10 @@ public final class OpenGroupManager: NSObject {
/// Start downloading the room image (if we don't have one or it's been updated) /// Start downloading the room image (if we don't have one or it's been updated)
if if
let imageId: Int64 = pollInfo.details?.imageId, let imageId: String = pollInfo.details?.imageId,
( (
openGroup.imageData == nil || openGroup.imageData == nil ||
openGroup.imageId != "\(imageId)" openGroup.imageId != imageId
) )
{ {
OpenGroupManager.roomImage(db, fileId: imageId, for: roomToken, on: server, using: dependencies) OpenGroupManager.roomImage(db, fileId: imageId, for: roomToken, on: server, using: dependencies)
@ -786,7 +786,7 @@ public final class OpenGroupManager: NSObject {
.done(on: OpenGroupAPI.workQueue) { items in .done(on: OpenGroupAPI.workQueue) { items in
dependencies.storage.writeAsync { db in dependencies.storage.writeAsync { db in
items items
.compactMap { room -> (Int64, String)? in .compactMap { room -> (String, String)? in
// Try to insert an inactive version of the OpenGroup (use 'insert' rather than 'save' // Try to insert an inactive version of the OpenGroup (use 'insert' rather than 'save'
// as we want it to fail if the room already exists) // as we want it to fail if the room already exists)
do { do {
@ -797,7 +797,7 @@ public final class OpenGroupManager: NSObject {
isActive: false, isActive: false,
name: room.name, name: room.name,
roomDescription: room.roomDescription, roomDescription: room.roomDescription,
imageId: room.imageId.map { "\($0)" }, imageId: room.imageId,
imageData: nil, imageData: nil,
userCount: room.activeUsers, userCount: room.activeUsers,
infoUpdates: room.infoUpdates, infoUpdates: room.infoUpdates,
@ -809,7 +809,7 @@ public final class OpenGroupManager: NSObject {
} }
catch {} catch {}
guard let imageId: Int64 = room.imageId else { return nil } guard let imageId: String = room.imageId else { return nil }
return (imageId, room.token) return (imageId, room.token)
} }
@ -845,7 +845,7 @@ public final class OpenGroupManager: NSObject {
public static func roomImage( public static func roomImage(
_ db: Database, _ db: Database,
fileId: Int64, fileId: String,
for roomToken: String, for roomToken: String,
on server: String, on server: String,
using dependencies: OGMDependencies = OGMDependencies() using dependencies: OGMDependencies = OGMDependencies()

@ -35,7 +35,7 @@ extension OpenGroupAPI {
// Files // Files
case roomFile(String) case roomFile(String)
case roomFileIndividual(String, Int64) case roomFileIndividual(String, String)
// Inbox/Outbox (Message Requests) // Inbox/Outbox (Message Requests)

@ -146,7 +146,6 @@ public enum MessageReceiver {
message.sentTimestamp = envelope.timestamp message.sentTimestamp = envelope.timestamp
message.receivedTimestamp = UInt64((Date().timeIntervalSince1970) * 1000) message.receivedTimestamp = UInt64((Date().timeIntervalSince1970) * 1000)
message.groupPublicKey = groupPublicKey message.groupPublicKey = groupPublicKey
message.openGroupServerTimestamp = (isOpenGroupMessage ? envelope.serverTimestamp : nil)
message.openGroupServerMessageId = openGroupMessageServerId.map { UInt64($0) } message.openGroupServerMessageId = openGroupMessageServerId.map { UInt64($0) }
// Validate // Validate

@ -339,11 +339,17 @@ public final class MessageSender {
.joined(separator: ".") .joined(separator: ".")
} }
// Note: It's possible to send a message and then delete the open group you sent the message to
// which would go into this case, so rather than handling it as an invalid state we just want to
// error in a non-retryable way
guard guard
let openGroup: OpenGroup = try? OpenGroup.fetchOne(db, id: threadId), let openGroup: OpenGroup = try? OpenGroup.fetchOne(db, id: threadId),
let userEdKeyPair: Box.KeyPair = Identity.fetchUserEd25519KeyPair(db), let userEdKeyPair: Box.KeyPair = Identity.fetchUserEd25519KeyPair(db),
case .openGroup(let roomToken, let server, let whisperTo, let whisperMods, let fileIds) = destination case .openGroup(let roomToken, let server, let whisperTo, let whisperMods, let fileIds) = destination
else { preconditionFailure() } else {
seal.reject(MessageSenderError.invalidMessage)
return promise
}
message.sender = { message.sender = {
let capabilities: [Capability.Variant] = (try? Capability let capabilities: [Capability.Variant] = (try? Capability

@ -145,15 +145,15 @@ public struct ProfileManager {
// Download already in flight; ignore // Download already in flight; ignore
return return
} }
guard guard let profileUrlStringAtStart: String = profile.profilePictureUrl else {
let profileUrlStringAtStart: String = profile.profilePictureUrl,
let profileUrlAtStart: URL = URL(string: profileUrlStringAtStart)
else {
SNLog("Skipping downloading avatar for \(profile.id) because url is not set") SNLog("Skipping downloading avatar for \(profile.id) because url is not set")
return return
} }
guard guard
let fileId: Int64 = Int64(profileUrlAtStart.lastPathComponent), let fileId: String = profileUrlStringAtStart
.split(separator: "/")
.last
.map({ String($0) }),
let profileKeyAtStart: OWSAES256Key = profile.profileEncryptionKey, let profileKeyAtStart: OWSAES256Key = profile.profileEncryptionKey,
profileKeyAtStart.keyData.count > 0 profileKeyAtStart.keyData.count > 0
else { else {

@ -14,8 +14,11 @@ public enum OnionRequestAPIError: LocalizedError {
public var errorDescription: String? { public var errorDescription: String? {
switch self { switch self {
case .httpRequestFailedAtDestination(let statusCode, _, let destination): case .httpRequestFailedAtDestination(let statusCode, let data, let destination):
if statusCode == 429 { return "Rate limited." } if statusCode == 429 { return "Rate limited." }
if let errorResponse: String = String(data: data, encoding: .utf8) {
return "HTTP request failed at destination (\(destination)) with status code: \(statusCode), error body: \(errorResponse)."
}
return "HTTP request failed at destination (\(destination)) with status code: \(statusCode)." return "HTTP request failed at destination (\(destination)) with status code: \(statusCode)."

@ -10,4 +10,5 @@ public enum SnodeAPIEndpoint: String {
case oxenDaemonRPCCall = "oxend_request" case oxenDaemonRPCCall = "oxend_request"
case getInfo = "info" case getInfo = "info"
case clearAllData = "delete_all" case clearAllData = "delete_all"
case expire = "expire"
} }

@ -650,8 +650,11 @@ public enum OnionRequestAPI: OnionRequestAPIType {
} }
if let bodyAsString = json["body"] as? String { if let bodyAsString = json["body"] as? String {
guard let bodyAsData = bodyAsString.data(using: .utf8), let body = try JSONSerialization.jsonObject(with: bodyAsData, options: [ .fragmentsAllowed ]) as? JSON else { guard let bodyAsData = bodyAsString.data(using: .utf8) else {
return seal.reject(HTTP.Error.invalidJSON) return seal.reject(HTTP.Error.invalidResponse)
}
guard let body = try? JSONSerialization.jsonObject(with: bodyAsData, options: [ .fragmentsAllowed ]) as? JSON else {
return seal.reject(OnionRequestAPIError.httpRequestFailedAtDestination(statusCode: UInt(statusCode), data: bodyAsData, destination: destination))
} }
if let timestamp = body["t"] as? Int64 { if let timestamp = body["t"] as? Int64 {

@ -719,6 +719,99 @@ public final class SnodeAPI {
return promise return promise
} }
// MARK: Edit
public static func updateExpiry(
publicKey: String,
edKeyPair: Box.KeyPair,
updatedExpiryMs: UInt64,
serverHashes: [String]
) -> Promise<[String: (hashes: [String], expiry: UInt64)]> {
let publicKey = (Features.useTestnet ? publicKey.removingIdPrefixIfNeeded() : publicKey)
return attempt(maxRetryCount: maxRetryCount, recoveringOn: Threading.workQueue) {
getSwarm(for: publicKey)
.then2 { swarm -> Promise<[String: (hashes: [String], expiry: UInt64)]> in
// "expire" || expiry || messages[0] || ... || messages[N]
let verificationBytes = SnodeAPIEndpoint.expire.rawValue.bytes
.appending(contentsOf: "\(updatedExpiryMs)".data(using: .ascii)?.bytes)
.appending(contentsOf: serverHashes.joined().bytes)
guard
let snode = swarm.randomElement(),
let signature = sodium.sign.signature(
message: verificationBytes,
secretKey: edKeyPair.secretKey
)
else {
throw SnodeAPIError.signingFailed
}
let parameters: JSON = [
"pubkey" : publicKey,
"pubkey_ed25519" : edKeyPair.publicKey.toHexString(),
"expiry": updatedExpiryMs,
"messages": serverHashes,
"signature": signature.toBase64()
]
return attempt(maxRetryCount: maxRetryCount, recoveringOn: Threading.workQueue) {
invoke(.expire, on: snode, associatedWith: publicKey, parameters: parameters)
.map2 { responseData -> [String: (hashes: [String], expiry: UInt64)] in
guard let responseJson: JSON = try? JSONSerialization.jsonObject(with: responseData, options: [ .fragmentsAllowed ]) as? JSON else {
throw HTTP.Error.invalidJSON
}
guard let swarm = responseJson["swarm"] as? JSON else { throw HTTP.Error.invalidJSON }
var result: [String: (hashes: [String], expiry: UInt64)] = [:]
for (snodePublicKey, rawJSON) in swarm {
guard let json = rawJSON as? JSON else { throw HTTP.Error.invalidJSON }
guard (json["failed"] as? Bool ?? false) == false else {
if let reason = json["reason"] as? String, let statusCode = json["code"] as? String {
SNLog("Couldn't delete data from: \(snodePublicKey) due to error: \(reason) (\(statusCode)).")
}
else {
SNLog("Couldn't delete data from: \(snodePublicKey).")
}
result[snodePublicKey] = ([], 0)
continue
}
guard
let hashes: [String] = json["updated"] as? [String],
let expiryApplied: UInt64 = json["expiry"] as? UInt64,
let signature: String = json["signature"] as? String
else {
throw HTTP.Error.invalidJSON
}
// The signature format is ( PUBKEY_HEX || EXPIRY || RMSG[0] || ... || RMSG[N] || UMSG[0] || ... || UMSG[M] )
let verificationBytes = publicKey.bytes
.appending(contentsOf: "\(expiryApplied)".data(using: .ascii)?.bytes)
.appending(contentsOf: serverHashes.joined().bytes)
.appending(contentsOf: hashes.joined().bytes)
let isValid = sodium.sign.verify(
message: verificationBytes,
publicKey: Bytes(Data(hex: snodePublicKey)),
signature: Bytes(Data(base64Encoded: signature)!)
)
// Ensure the signature is valid
guard isValid else {
throw SnodeAPIError.signatureVerificationFailed
}
result[snodePublicKey] = (hashes, expiryApplied)
}
return result
}
}
}
}
}
// MARK: Delete // MARK: Delete
public static func deleteMessage(publicKey: String, serverHashes: [String]) -> Promise<[String: Bool]> { public static func deleteMessage(publicKey: String, serverHashes: [String]) -> Promise<[String: Bool]> {
@ -732,10 +825,16 @@ public final class SnodeAPI {
return attempt(maxRetryCount: maxRetryCount, recoveringOn: Threading.workQueue) { return attempt(maxRetryCount: maxRetryCount, recoveringOn: Threading.workQueue) {
getSwarm(for: publicKey) getSwarm(for: publicKey)
.then2 { swarm -> Promise<[String: Bool]> in .then2 { swarm -> Promise<[String: Bool]> in
// "delete" || messages...
let verificationBytes = SnodeAPIEndpoint.deleteMessage.rawValue.bytes
.appending(contentsOf: serverHashes.joined().bytes)
guard guard
let snode = swarm.randomElement(), let snode = swarm.randomElement(),
let verificationData = (SnodeAPIEndpoint.deleteMessage.rawValue + serverHashes.joined()).data(using: String.Encoding.utf8), let signature = sodium.sign.signature(
let signature = sodium.sign.signature(message: Bytes(verificationData), secretKey: userED25519KeyPair.secretKey) message: verificationBytes,
secretKey: userED25519KeyPair.secretKey
)
else { else {
throw SnodeAPIError.signingFailed throw SnodeAPIError.signingFailed
} }
@ -771,15 +870,11 @@ public final class SnodeAPI {
} }
// The signature format is ( PUBKEY_HEX || RMSG[0] || ... || RMSG[N] || DMSG[0] || ... || DMSG[M] ) // The signature format is ( PUBKEY_HEX || RMSG[0] || ... || RMSG[N] || DMSG[0] || ... || DMSG[M] )
let verificationData = [ let verificationBytes = userX25519PublicKey.bytes
userX25519PublicKey, .appending(contentsOf: serverHashes.joined().bytes)
serverHashes.joined(), .appending(contentsOf: hashes.joined().bytes)
hashes.joined()
]
.joined()
.data(using: String.Encoding.utf8)!
let isValid = sodium.sign.verify( let isValid = sodium.sign.verify(
message: Bytes(verificationData), message: verificationBytes,
publicKey: Bytes(Data(hex: snodePublicKey)), publicKey: Bytes(Data(hex: snodePublicKey)),
signature: Bytes(Data(base64Encoded: signature)!) signature: Bytes(Data(base64Encoded: signature)!)
) )

@ -411,7 +411,7 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
guard targetIndex > halfPageSize else { return 0 } guard targetIndex > halfPageSize else { return 0 }
guard targetIndex < (totalCount - halfPageSize) else { guard targetIndex < (totalCount - halfPageSize) else {
return (totalCount - currentPageInfo.pageSize) return max(0, (totalCount - currentPageInfo.pageSize))
} }
return (targetIndex - halfPageSize) return (targetIndex - halfPageSize)

@ -7,8 +7,8 @@ import Curve25519Kit
public struct SessionId { public struct SessionId {
public enum Prefix: String, CaseIterable { public enum Prefix: String, CaseIterable {
case standard = "05" // Used for identified users, open groups, etc. case standard = "05" // Used for identified users, open groups, etc.
case blinded = "15" // Used for participants in open groups with blinding enabled case blinded = "15" // Used for authentication and participants in open groups with blinding enabled
case unblinded = "00" // Used for participants in open groups with blinding disabled case unblinded = "00" // Used for authentication in open groups with blinding disabled
public init?(from stringValue: String?) { public init?(from stringValue: String?) {
guard let stringValue: String = stringValue else { return nil } guard let stringValue: String = stringValue else { return nil }

Loading…
Cancel
Save