Changeset View
Standalone View
native/expo-modules/aes-crypto/ios/AESCryptoModule.swift
import ExpoModulesCore | import ExpoModulesCore | ||||
import CryptoKit | import CryptoKit | ||||
private let IV_LENGTH = 12 // bytes | |||||
atul: Maybe a comment above writing out `INITIALIZATION_VECTOR`?
"IV" might be a standard acronym in… | |||||
private let TAG_LENGTH = 16 // bytes | |||||
public class AESCryptoModule: Module { | public class AESCryptoModule: Module { | ||||
public func definition() -> ModuleDefinition { | public func definition() -> ModuleDefinition { | ||||
Name("AESCrypto") | Name("AESCrypto") | ||||
Function("generateKey", generateKey) | Function("generateKey", generateKey) | ||||
Function("encrypt", encrypt) | |||||
} | } | ||||
} | } | ||||
// MARK: - Function implementations | // MARK: - Function implementations | ||||
private func generateKey(destination: Uint8Array) throws { | private func generateKey(destination: Uint8Array) throws { | ||||
let key = SymmetricKey(size: .bits256) | let key = SymmetricKey(size: .bits256) | ||||
guard destination.byteLength * 8 == key.bitCount else { | guard destination.byteLength * 8 == key.bitCount else { | ||||
throw InvalidKeyLengthException() | throw InvalidKeyLengthException() | ||||
} | } | ||||
key.withUnsafeBytes { bytes in | key.withUnsafeBytes { bytes in | ||||
let _ = bytes.copyBytes(to: destination.rawBufferPtr()) | let _ = bytes.copyBytes(to: destination.rawBufferPtr()) | ||||
} | } | ||||
} | } | ||||
private func encrypt(rawKey: Uint8Array, | |||||
plaintext: Uint8Array, | |||||
destination: Uint8Array) throws { | |||||
guard destination.byteLength >= plaintext.byteLength + IV_LENGTH + TAG_LENGTH | |||||
atulUnsubmitted Not Done Inline ActionsSince we're initializing the destination buffer via: new Uint8Array(data.length + IV_LENGTH + TAG_LENGTH); Should we just do == instead of >=? Let me know if there's something I'm missing here. atul: Since we're initializing the `destination` buffer via:
```lang=javascript
new Uint8Array(data. | |||||
bartekAuthorUnsubmitted Done Inline ActionsTechnically it is important for this array to be just larger to avoid memory errors, but you're right, the == can be used for extra safety. bartek: Technically it is important for this array to be just larger to avoid memory errors, but you're… | |||||
else { | |||||
throw InvalidDataLengthException() | |||||
} | |||||
let key = SymmetricKey(data: rawKey.data()) | |||||
let iv = AES.GCM.Nonce() | |||||
let encryptionResult = try AES.GCM.seal(plaintext.data(), | |||||
using: key, | |||||
nonce: iv) | |||||
// 'combined' returns concatenated: iv || ciphertext || tag | |||||
guard let ciphertext = encryptionResult.combined else { | |||||
// this happens only if Nonce/IV != 12 bytes long | |||||
throw EncryptionFailedException("Incorrect AES configuration") | |||||
} | |||||
ciphertext.copyBytes(to: destination.rawBufferPtr()) | |||||
atulUnsubmitted Not Done Inline ActionsIt's a little confusing that we're concatenating iv and tag to ciphertext and calling the destination buffer ciphertext on the JS side. Should we maybe use like seal(...) and sealedData naming instead of encrypt(...) and ciphertext so it's a little less ambiguous? Maybe a nit/pedantic, but figure we should be as precise as possible with anything cryptography related? atul: It's a little confusing that we're concatenating `iv` and `tag` to `ciphertext` and calling the… | |||||
bartekAuthorUnsubmitted Done Inline ActionsI totally agree that all names related to ciphertext are confusing.
I admit I started mixing names because I have no idea how to name them without having extra long/verbose variable names like ciphertextWithTag bartek: I totally agree that all names related to `ciphertext` are confusing.
We have three variations… | |||||
atulUnsubmitted Not Done Inline ActionsPersonally would totally err on the side of verbosity for anything having to do with cryptography since any confusion or lack of precision could be "devastating" and a couple extra characters doesn't hurt anyone. Personally think or something like that might make sense? Could add comments above like sealedData where it's defined to make it super clear what it contains? It's kind of difficult because we're just dealing with a "bunch of bytes" instead of like structs with fields in them. atul: Personally would totally err on the side of verbosity for anything having to do with… | |||||
} | |||||
// MARK: - Utilities | // MARK: - Utilities | ||||
extension TypedArray { | extension TypedArray { | ||||
func data() -> Data { | |||||
Data(bytes: self.rawPointer, count: self.byteLength) | |||||
} | |||||
func rawBufferPtr() -> UnsafeMutableRawBufferPointer { | func rawBufferPtr() -> UnsafeMutableRawBufferPointer { | ||||
UnsafeMutableRawBufferPointer(start: self.rawPointer, | UnsafeMutableRawBufferPointer(start: self.rawPointer, | ||||
count: self.byteLength) | count: self.byteLength) | ||||
} | } | ||||
} | } | ||||
// MARK: - Exception definitions | // MARK: - Exception definitions | ||||
private class InvalidKeyLengthException: Exception { | private class InvalidKeyLengthException: Exception { | ||||
override var reason: String { | override var reason: String { | ||||
"The AES key has invalid length" | "The AES key has invalid length" | ||||
} | } | ||||
} | } | ||||
private class InvalidDataLengthException: Exception { | |||||
override var reason: String { | |||||
"Source or destination array has invalid length" | |||||
} | |||||
} | |||||
private class EncryptionFailedException: GenericException<String> { | |||||
override var reason: String { | |||||
"Failed to encrypt data: \(param)" | |||||
} | |||||
} |
Maybe a comment above writing out INITIALIZATION_VECTOR?
"IV" might be a standard acronym in cryptography (eg HMAC), but I wasn't previously familiar with it.