-
Notifications
You must be signed in to change notification settings - Fork 440
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.
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
feat(EIP712): parsing of TypedData payload; encoding + hashing; #833
Changes from all commits
354589f
0342f91
a9b23fb
ce459b6
89595ad
27d5b61
07965a1
5493dd0
4669bfe
b8e55b2
c01f6a1
41828fb
c6da90f
e40c3f8
4990084
d69d594
40a9a3d
cfd8ee9
0b0b3fa
68d8457
111d33e
e77af0a
bd1544d
6cc06db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,16 +7,41 @@ import Foundation | |
|
||
extension ABI { | ||
|
||
public enum ParsingError: Swift.Error { | ||
public enum ParsingError: LocalizedError { | ||
case invalidJsonFile | ||
case elementTypeInvalid | ||
case elementTypeInvalid(_ desc: String? = nil) | ||
case elementNameInvalid | ||
case functionInputInvalid | ||
case functionOutputInvalid | ||
case eventInputInvalid | ||
case parameterTypeInvalid | ||
case parameterTypeNotFound | ||
case abiInvalid | ||
|
||
public var errorDescription: String? { | ||
var errorMessage: [String?] | ||
switch self { | ||
case .invalidJsonFile: | ||
errorMessage = ["invalidJsonFile"] | ||
case .elementTypeInvalid(let desc): | ||
errorMessage = ["elementTypeInvalid", desc] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this line is a cause that we're messing here with an array of Strings rather a plain String? |
||
case .elementNameInvalid: | ||
errorMessage = ["elementNameInvalid"] | ||
case .functionInputInvalid: | ||
errorMessage = ["functionInputInvalid"] | ||
case .functionOutputInvalid: | ||
errorMessage = ["functionOutputInvalid"] | ||
case .eventInputInvalid: | ||
errorMessage = ["eventInputInvalid"] | ||
case .parameterTypeInvalid: | ||
errorMessage = ["parameterTypeInvalid"] | ||
case .parameterTypeNotFound: | ||
errorMessage = ["parameterTypeNotFound"] | ||
case .abiInvalid: | ||
errorMessage = ["abiInvalid"] | ||
} | ||
return errorMessage.compactMap { $0 }.joined(separator: " ") | ||
} | ||
} | ||
|
||
enum TypeParsingExpressions { | ||
|
@@ -39,7 +64,7 @@ extension ABI.Record { | |
public func parse() throws -> ABI.Element { | ||
let typeString = self.type ?? "function" | ||
guard let type = ABI.ElementType(rawValue: typeString) else { | ||
throw ABI.ParsingError.elementTypeInvalid | ||
throw ABI.ParsingError.elementTypeInvalid("Invalid ABI type \(typeString).") | ||
} | ||
return try parseToElement(from: self, type: type) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,11 +11,30 @@ public protocol AbstractKeystore { | |
func UNSAFE_getPrivateKeyData(password: String, account: EthereumAddress) throws -> Data | ||
} | ||
|
||
public enum AbstractKeystoreError: Error { | ||
case noEntropyError | ||
case keyDerivationError | ||
case aesError | ||
case invalidAccountError | ||
public enum AbstractKeystoreError: LocalizedError { | ||
case noEntropyError(_ additionalDescription: String? = nil) | ||
case keyDerivationError(_ additionalDescription: String? = nil) | ||
case aesError(_ additionalDescription: String? = nil) | ||
case invalidAccountError(_ additionalDescription: String? = nil) | ||
case invalidPasswordError | ||
case encryptionError(String) | ||
case encryptionError(_ additionalDescription: String? = nil) | ||
|
||
public var errorDescription: String? { | ||
var errorMessage: [String?] | ||
switch self { | ||
case .noEntropyError(let additionalDescription): | ||
errorMessage = ["Entropy error (e.g. failed to generate a random array of bytes).", additionalDescription] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this line is a cause that we're messing here with an array of Strings rather a plain String? Here it looks quite more necessary than in the previous alike code snippet, yet I wonder if it's a best way in terms of code intention clarity to implement such behaviour? |
||
case .keyDerivationError(let additionalDescription): | ||
errorMessage = ["Key derivation error.", additionalDescription] | ||
case .aesError(let additionalDescription): | ||
errorMessage = ["AES error.", additionalDescription] | ||
case .invalidAccountError(let additionalDescription): | ||
errorMessage = ["Invalid account error.", additionalDescription] | ||
case .invalidPasswordError: | ||
errorMessage = ["Invalid password error."] | ||
case .encryptionError(let additionalDescription): | ||
errorMessage = ["Encryption error.", additionalDescription] | ||
} | ||
return errorMessage.compactMap { $0 }.joined(separator: " ") | ||
} | ||
} |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,11 +95,13 @@ public class BIP39 { | |
} | ||
|
||
private static func entropyOf(size: Int) throws -> Data { | ||
let isCorrectSize = size >= 128 && size <= 256 && size.isMultiple(of: 32) | ||
let randomBytesCount = size / 8 | ||
guard | ||
size >= 128 && size <= 256 && size.isMultiple(of: 32), | ||
let entropy = Data.randomBytes(length: size/8) | ||
isCorrectSize, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although it got better, it still doesn't fit with our convention of |
||
let entropy = Data.randomBytes(length: randomBytesCount) | ||
else { | ||
throw AbstractKeystoreError.noEntropyError | ||
throw AbstractKeystoreError.noEntropyError("BIP39. \(!isCorrectSize ? "Requested entropy of wrong bits size: \(size). Expected: 128 <= size <= 256, size % 32 == 0." : "Failed to generate \(randomBytesCount) of random bytes.")") | ||
} | ||
return entropy | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,13 +23,13 @@ public class EthereumKeystoreV3: AbstractKeystore { | |
} | ||
|
||
public func UNSAFE_getPrivateKeyData(password: String, account: EthereumAddress) throws -> Data { | ||
if self.addresses?.count == 1 && account == self.addresses?.last { | ||
guard let privateKey = try? self.getKeyData(password) else { | ||
if account == addresses?.last { | ||
guard let privateKey = try? getKeyData(password) else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can make this bit a bit more straightforward? Like changing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Upd: oh, I see that the execution flow continues after that line:32 throw statement. Maybe we should add at least |
||
throw AbstractKeystoreError.invalidPasswordError | ||
} | ||
return privateKey | ||
} | ||
throw AbstractKeystoreError.invalidAccountError | ||
throw AbstractKeystoreError.invalidAccountError("EthereumKeystoreV3. Cannot get private key: keystore doesn't contain information about given address \(account.address).") | ||
} | ||
|
||
// Class | ||
|
@@ -77,7 +77,7 @@ public class EthereumKeystoreV3: AbstractKeystore { | |
defer { | ||
Data.zero(&newPrivateKey) | ||
} | ||
try encryptDataToStorage(password, keyData: newPrivateKey, aesMode: aesMode) | ||
try encryptDataToStorage(password, privateKey: newPrivateKey, aesMode: aesMode) | ||
} | ||
|
||
public init?(privateKey: Data, password: String, aesMode: String = "aes-128-cbc") throws { | ||
|
@@ -87,68 +87,60 @@ public class EthereumKeystoreV3: AbstractKeystore { | |
guard SECP256K1.verifyPrivateKey(privateKey: privateKey) else { | ||
return nil | ||
} | ||
try encryptDataToStorage(password, keyData: privateKey, aesMode: aesMode) | ||
try encryptDataToStorage(password, privateKey: privateKey, aesMode: aesMode) | ||
} | ||
|
||
fileprivate func encryptDataToStorage(_ password: String, keyData: Data?, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1, aesMode: String = "aes-128-cbc") throws { | ||
if keyData == nil { | ||
throw AbstractKeystoreError.encryptionError("Encryption without key data") | ||
fileprivate func encryptDataToStorage(_ password: String, privateKey: Data, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1, aesMode: String = "aes-128-cbc") throws { | ||
if privateKey.count != 32 { | ||
throw AbstractKeystoreError.encryptionError("EthereumKeystoreV3. Attempted encryption with private key of length != 32. Given private key length is \(privateKey.count).") | ||
} | ||
let saltLen = 32 | ||
guard let saltData = Data.randomBytes(length: saltLen) else { | ||
throw AbstractKeystoreError.noEntropyError | ||
throw AbstractKeystoreError.noEntropyError("EthereumKeystoreV3. Failed to generate random bytes: `Data.randomBytes(length: \(saltLen))`.") | ||
} | ||
guard let derivedKey = scrypt(password: password, salt: saltData, length: dkLen, N: N, R: R, P: P) else { | ||
throw AbstractKeystoreError.keyDerivationError | ||
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Scrypt function failed.") | ||
} | ||
let last16bytes = Data(derivedKey[(derivedKey.count - 16)...(derivedKey.count - 1)]) | ||
let encryptionKey = Data(derivedKey[0...15]) | ||
guard let IV = Data.randomBytes(length: 16) else { | ||
throw AbstractKeystoreError.noEntropyError | ||
throw AbstractKeystoreError.noEntropyError("EthereumKeystoreV3. Failed to generate random bytes: `Data.randomBytes(length: 16)`.") | ||
} | ||
var aesCipher: AES? | ||
switch aesMode { | ||
var aesCipher: AES | ||
switch aesMode.lowercased() { | ||
case "aes-128-cbc": | ||
aesCipher = try? AES(key: encryptionKey.bytes, blockMode: CBC(iv: IV.bytes), padding: .noPadding) | ||
aesCipher = try AES(key: encryptionKey.bytes, blockMode: CBC(iv: IV.bytes), padding: .noPadding) | ||
case "aes-128-ctr": | ||
aesCipher = try? AES(key: encryptionKey.bytes, blockMode: CTR(iv: IV.bytes), padding: .noPadding) | ||
aesCipher = try AES(key: encryptionKey.bytes, blockMode: CTR(iv: IV.bytes), padding: .noPadding) | ||
default: | ||
aesCipher = nil | ||
throw AbstractKeystoreError.aesError("EthereumKeystoreV3. AES error: given AES mode can be one of 'aes-128-cbc' or 'aes-128-ctr'. Instead '\(aesMode)' was given.") | ||
} | ||
if aesCipher == nil { | ||
throw AbstractKeystoreError.aesError | ||
} | ||
guard let encryptedKey = try aesCipher?.encrypt(keyData!.bytes) else { | ||
throw AbstractKeystoreError.aesError | ||
} | ||
let encryptedKeyData = Data(encryptedKey) | ||
var dataForMAC = Data() | ||
dataForMAC.append(last16bytes) | ||
dataForMAC.append(encryptedKeyData) | ||
|
||
let encryptedKeyData = Data(try aesCipher.encrypt(privateKey.bytes)) | ||
let dataForMAC = last16bytes + encryptedKeyData | ||
let mac = dataForMAC.sha3(.keccak256) | ||
let kdfparams = KdfParamsV3(salt: saltData.toHexString(), dklen: dkLen, n: N, p: P, r: R, c: nil, prf: nil) | ||
let cipherparams = CipherParamsV3(iv: IV.toHexString()) | ||
let crypto = CryptoParamsV3(ciphertext: encryptedKeyData.toHexString(), cipher: aesMode, cipherparams: cipherparams, kdf: "scrypt", kdfparams: kdfparams, mac: mac.toHexString(), version: nil) | ||
guard let pubKey = Utilities.privateToPublic(keyData!) else { | ||
throw AbstractKeystoreError.keyDerivationError | ||
guard let publicKey = Utilities.privateToPublic(privateKey) else { | ||
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Failed to derive public key from given private key. `Utilities.privateToPublic(privateKey)` returned `nil`.") | ||
} | ||
guard let addr = Utilities.publicToAddress(pubKey) else { | ||
throw AbstractKeystoreError.keyDerivationError | ||
guard let addr = Utilities.publicToAddress(publicKey) else { | ||
throw AbstractKeystoreError.keyDerivationError("EthereumKeystoreV3. Failed to derive address from derived public key. `Utilities.publicToAddress(publicKey)` returned `nil`.") | ||
} | ||
self.address = addr | ||
let keystoreparams = KeystoreParamsV3(address: addr.address.lowercased(), crypto: crypto, id: UUID().uuidString.lowercased(), version: 3) | ||
self.keystoreParams = keystoreparams | ||
} | ||
|
||
public func regenerate(oldPassword: String, newPassword: String, dkLen: Int = 32, N: Int = 4096, R: Int = 6, P: Int = 1) throws { | ||
var keyData = try self.getKeyData(oldPassword) | ||
if keyData == nil { | ||
throw AbstractKeystoreError.encryptionError("Failed to decrypt a keystore") | ||
guard var privateKey = try getKeyData(oldPassword) else { | ||
throw AbstractKeystoreError.encryptionError("EthereumKeystoreV3. Failed to decrypt a keystore") | ||
} | ||
defer { | ||
Data.zero(&keyData!) | ||
Data.zero(&privateKey) | ||
} | ||
try self.encryptDataToStorage(newPassword, keyData: keyData!, aesMode: self.keystoreParams!.crypto.cipher) | ||
try self.encryptDataToStorage(newPassword, privateKey: privateKey, aesMode: self.keystoreParams!.crypto.cipher) | ||
} | ||
|
||
fileprivate func getKeyData(_ password: String) throws -> Data? { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,12 @@ public class EIP712 { | |
public typealias Bytes = Data | ||
} | ||
|
||
// FIXME: this type is wrong - The minimum number of optional fields is 5, and those are | ||
// string name the user readable name of signing domain, i.e. the name of the DApp or the protocol. | ||
// string version the current major version of the signing domain. Signatures from different versions are not compatible. | ||
// uint256 chainId the EIP-155 chain id. The user-agent should refuse signing if it does not match the currently active chain. | ||
// address verifyingContract the address of the contract that will verify the signature. The user-agent may do contract specific phishing prevention. | ||
// bytes32 salt an disambiguating salt for the protocol. This can be used as a domain separator of last resort. | ||
public struct EIP712Domain: EIP712Hashable { | ||
Comment on lines
+19
to
25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something to deal with later. |
||
public let chainId: EIP712.UInt256? | ||
public let verifyingContract: EIP712.Address | ||
|
@@ -54,6 +60,8 @@ public extension EIP712Hashable { | |
result = ABIEncoder.encodeSingleType(type: .uint(bits: 256), value: field)! | ||
case is EIP712.Address: | ||
result = ABIEncoder.encodeSingleType(type: .address, value: field)! | ||
case let boolean as Bool: | ||
result = ABIEncoder.encodeSingleType(type: .uint(bits: 8), value: boolean ? 1 : 0)! | ||
case let hashable as EIP712Hashable: | ||
result = try hashable.hash() | ||
default: | ||
|
@@ -64,16 +72,19 @@ public extension EIP712Hashable { | |
preconditionFailure("Not solidity type") | ||
} | ||
} | ||
guard result.count == 32 else { preconditionFailure("ABI encode error") } | ||
guard result.count % 32 == 0 else { preconditionFailure("ABI encode error") } | ||
parameters.append(result) | ||
} | ||
return Data(parameters.flatMap { $0.bytes }).sha3(.keccak256) | ||
} | ||
} | ||
|
||
public func eip712encode(domainSeparator: EIP712Hashable, message: EIP712Hashable) throws -> Data { | ||
let data = try Data([UInt8(0x19), UInt8(0x01)]) + domainSeparator.hash() + message.hash() | ||
return data.sha3(.keccak256) | ||
public func eip712hash(domainSeparator: EIP712Hashable, message: EIP712Hashable) throws -> Data { | ||
try eip712hash(domainSeparatorHash: domainSeparator.hash(), messageHash: message.hash()) | ||
} | ||
|
||
public func eip712hash(domainSeparatorHash: Data, messageHash: Data) -> Data { | ||
(Data([UInt8(0x19), UInt8(0x01)]) + domainSeparatorHash + messageHash).sha3(.keccak256) | ||
} | ||
|
||
// MARK: - Additional private and public extensions with support members | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've decided not to add
(_ desc: String? = nil)
to all error types as it would trigger more changes across the library.It's better to do that updated in a separate PR.