Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolution of review comments #1409

Merged
merged 6 commits into from
Nov 17, 2024
Merged

Resolution of review comments #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6b9c46f
Resolution of review comments
LukasKorba Nov 14, 2024
9441a06
Fixes of reported blockers
LukasKorba Nov 14, 2024
5102828
Tweak of a code comment
LukasKorba Nov 14, 2024
385331c
Propagation of the correct version
LukasKorba Nov 14, 2024
c328687
Remote error handling and byte alignment resolved
LukasKorba Nov 15, 2024
0009cf1
Final cleanup and feedback resolution
LukasKorba Nov 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 59 additions & 77 deletions modules/Sources/Dependencies/AddressBookClient/AddressBookEncryption.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,35 @@ import CryptoKit
import WalletStorage

extension AddressBookClient {
static func serializeContacts(_ abContacts: AddressBookContacts) -> Data {
var dataForEncryption = Data()

// Serialize `address book version`
dataForEncryption.append(contentsOf: intToBytes(abContacts.version))

// Serialize `lastUpdated`
dataForEncryption.append(contentsOf: AddressBookClient.serializeDate(Date()))

// Serialize `contacts.count`
dataForEncryption.append(contentsOf: intToBytes(abContacts.contacts.count))

// Serialize `contacts`
abContacts.contacts.forEach { contact in
let serializedContact = serializeContact(contact)
dataForEncryption.append(serializedContact)
}

return dataForEncryption
}

/// Encrypts address book contacts. The structure:
/// [Unencrypted data] `encryption version`
/// [Unencrypted data] `salt`
/// [Encrypted data] `address book version`
/// [Encrypted data] `timestamp`
/// [Encrypted data] `contacts`
///
/// This method always produces the latest structure with he latest encryption version.
/// This method always produces the latest structure with the latest encryption version.
static func encryptContacts(_ abContacts: AddressBookContacts) throws -> Data {
@Dependency(\.walletStorage) var walletStorage

Expand All @@ -31,22 +52,7 @@ extension AddressBookClient {
var encryptionVersionData = Data()
encryptionVersionData.append(contentsOf: intToBytes(AddressBookEncryptionKeys.Constants.version))

var dataForEncryption = Data()

// Serialize `address book version`
dataForEncryption.append(contentsOf: intToBytes(abContacts.version))

// Serialize `lastUpdated`
dataForEncryption.append(contentsOf: AddressBookClient.serializeDate(Date()))

// Serialize `contacts.count`
dataForEncryption.append(contentsOf: intToBytes(abContacts.contacts.count))

// Serialize `contacts`
abContacts.contacts.forEach { contact in
let serializedContact = serializeContact(contact)
dataForEncryption.append(serializedContact)
}
let dataForEncryption = AddressBookClient.serializeContacts(abContacts)

// Generate a fresh one-time sub-key for encrypting the address book.
let salt = SymmetricKey(size: SymmetricKeySize.bits256)
Expand Down Expand Up @@ -84,15 +90,15 @@ extension AddressBookClient {
var offset = 0

// Deserialize `encryption version`
let encryptionVersionBytes = encryptedData.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size
let encryptionVersionBytes = try AddressBookClient.subdata(of: encryptedData, in: offset..<(offset + Constants.int64Size))
offset += Constants.int64Size

guard let encryptionVersion = AddressBookClient.bytesToInt(Array(encryptionVersionBytes)) else {
return .empty
}

if encryptionVersion == AddressBookEncryptionKeys.Constants.version {
let encryptedSubData = encryptedData.subdata(in: offset..<encryptedData.count)
let encryptedSubData = try AddressBookClient.subdata(of: encryptedData, in: offset..<encryptedData.count)

// Derive the sub-key for decrypting the address book.
let salt = encryptedSubData.prefix(upTo: 32)
Expand All @@ -101,42 +107,8 @@ extension AddressBookClient {
// Unseal the encrypted address book.
let sealed = try ChaChaPoly.SealedBox.init(combined: encryptedSubData.suffix(from: 32))
let data = try ChaChaPoly.open(sealed, using: subKey)
offset = 0

// Deserialize `address book version`
let abVersionCountBytes = data.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
guard let abVersion = AddressBookClient.bytesToInt(Array(abVersionCountBytes)) else {
return .empty
}
offset += MemoryLayout<Int>.size

// Deserialize `lastUpdated`
guard let lastUpdated = AddressBookClient.deserializeDate(from: data, at: &offset) else {
return .empty
}

// Deserialize `contactsCount`
let contactsCountBytes = data.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size

guard let contactsCount = AddressBookClient.bytesToInt(Array(contactsCountBytes)) else {
return .empty
}

var contacts: [Contact] = []
for _ in 0..<contactsCount {
if let contact = AddressBookClient.deserializeContact(from: data, at: &offset) {
contacts.append(contact)
}
}

let abContacts = AddressBookContacts(
lastUpdated: lastUpdated,
version: abVersion,
contacts: IdentifiedArrayOf(uniqueElements: contacts)
)

return abContacts
return try contactsFrom(plainData: data)
} else {
throw AddressBookClientError.encryptionVersionNotSupported
}
Expand All @@ -146,41 +118,41 @@ extension AddressBookClient {
/// [Unencrypted data] `address book version`
/// [Unencrypted data] `timestamp`
/// [Unencrypted data] `contacts`
static func contactsFrom(unencryptedData: Data) throws -> AddressBookContacts {
static func contactsFrom(plainData: Data) throws -> AddressBookContacts {
var offset = 0

// Deserialize `version`
let versionBytes = unencryptedData.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size
let versionBytes = try AddressBookClient.subdata(of: plainData, in: offset..<(offset + Constants.int64Size))
offset += Constants.int64Size

// Deserialize and check `address book version`
guard let version = AddressBookClient.bytesToInt(Array(versionBytes)), version == AddressBookContacts.Constants.version else {
return .empty
}

// Deserialize `lastUpdated`
guard let lastUpdated = AddressBookClient.deserializeDate(from: unencryptedData, at: &offset) else {
guard let lastUpdated = try AddressBookClient.deserializeDate(from: plainData, at: &offset) else {
return .empty
}

// Deserialize `contactsCount`
let contactsCountBytes = unencryptedData.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size
let contactsCountBytes = try AddressBookClient.subdata(of: plainData, in: offset..<(offset + Constants.int64Size))
offset += Constants.int64Size

guard let contactsCount = AddressBookClient.bytesToInt(Array(contactsCountBytes)) else {
return .empty
}

var contacts: [Contact] = []
for _ in 0..<contactsCount {
if let contact = AddressBookClient.deserializeContact(from: unencryptedData, at: &offset) {
if let contact = try AddressBookClient.deserializeContact(from: plainData, at: &offset) {
contacts.append(contact)
}
}
LukasKorba marked this conversation as resolved.
Show resolved Hide resolved

let abContacts = AddressBookContacts(
lastUpdated: lastUpdated,
version: AddressBookContacts.Constants.version,
version: version,
contacts: IdentifiedArrayOf(uniqueElements: contacts)
)

Expand Down Expand Up @@ -210,19 +182,19 @@ extension AddressBookClient {
return data
}

private static func deserializeContact(from data: Data, at offset: inout Int) -> Contact? {
private static func deserializeContact(from data: Data, at offset: inout Int) throws -> Contact? {
// Deserialize `lastUpdated`
guard let lastUpdated = AddressBookClient.deserializeDate(from: data, at: &offset) else {
guard let lastUpdated = try AddressBookClient.deserializeDate(from: data, at: &offset) else {
return nil
}

// Deserialize `address`
guard let address = readString(from: data, at: &offset) else {
guard let address = try readString(from: data, at: &offset) else {
return nil
}

// Deserialize `name`
guard let name = readString(from: data, at: &offset) else {
guard let name = try readString(from: data, at: &offset) else {
return nil
}

Expand All @@ -242,12 +214,12 @@ extension AddressBookClient {
}

private static func bytesToInt(_ bytes: [UInt8]) -> Int? {
guard bytes.count == MemoryLayout<Int>.size else {
guard bytes.count == Constants.int64Size else {
return nil
}

LukasKorba marked this conversation as resolved.
Show resolved Hide resolved
return bytes.withUnsafeBytes {
$0.load(as: Int.self).bigEndian
return bytes.withUnsafeBytes { ptr -> Int? in
Int.init(exactly: ptr.loadUnaligned(as: Int64.self).bigEndian)
}
}

Expand All @@ -259,10 +231,10 @@ extension AddressBookClient {
return AddressBookClient.intToBytes(timestamp)
}

private static func deserializeDate(from data: Data, at offset: inout Int) -> Date? {
private static func deserializeDate(from data: Data, at offset: inout Int) throws -> Date? {
// Extract the bytes for the timestamp (assume it's stored as an Int)
let timestampBytes = data.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size
let timestampBytes = try AddressBookClient.subdata(of: data, in: offset..<(offset + Constants.int64Size))
offset += Constants.int64Size

// Convert the bytes back to an Int
guard let timestamp = AddressBookClient.bytesToInt(Array(timestampBytes)) else { return nil }
Expand All @@ -272,15 +244,25 @@ extension AddressBookClient {
}

// Helper function to read a string from the data using a length prefix
private static func readString(from data: Data, at offset: inout Int) -> String? {
private static func readString(from data: Data, at offset: inout Int) throws -> String? {
// Read the length first (assumes the length is stored as an Int)
let lengthBytes = data.subdata(in: offset..<(offset + MemoryLayout<Int>.size))
offset += MemoryLayout<Int>.size
let lengthBytes = try AddressBookClient.subdata(of: data, in: offset..<(offset + Constants.int64Size))
offset += Constants.int64Size
guard let length = AddressBookClient.bytesToInt(Array(lengthBytes)), length > 0 else { return nil }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a string of length 0 be accepted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the question, there's a check

guard let length = AddressBookClient.bytesToInt(Array(lengthBytes)), length > 0 else { return nil }

so 0 length strings are dropped and nil is returned.

Copy link
Contributor

@daira daira Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was asking whether it's intended and correct to reject them.


// Read the string bytes
let stringBytes = data.subdata(in: offset..<(offset + length))
let stringBytes = try AddressBookClient.subdata(of: data, in: offset..<(offset + length))
offset += length
return AddressBookClient.bytesToString(Array(stringBytes))
}

private static func subdata(of data: Data, in range: Range<Data.Index>) throws -> Data {
guard data.count >= range.upperBound else {
throw AddressBookClient.AddressBookClientError.subdataRange
}

return data.subdata(in: range)
}
}


8 changes: 4 additions & 4 deletions modules/Sources/Dependencies/AddressBookClient/AddressBookInterface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ extension DependencyValues {

@DependencyClient
public struct AddressBookClient {
public let allLocalContacts: () throws -> AddressBookContacts
public let syncContacts: (AddressBookContacts?) async throws -> AddressBookContacts
public let storeContact: (Contact) throws -> AddressBookContacts
public let deleteContact: (Contact) throws -> AddressBookContacts
public let allLocalContacts: () throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult)
public let syncContacts: (AddressBookContacts?) async throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult)
public let storeContact: (Contact) throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult)
public let deleteContact: (Contact) throws -> (contacts: AddressBookContacts, remoteStoreResult: RemoteStoreResult)
}
Loading