[native][AES] Implement generateKey function
ClosedPublic
Actions

Authored by bartek on Mar 8 2023, 9:56 AM.

Details

Reviewers

marcin
atul
varun

Commits

rCOMM46037bfe9b83: [native][AES] Implement generateKey function

Summary

This diff implements a function, that generates AES-256 key using platforms' native APIs.

iOS docs: https://developer.apple.com/documentation/cryptokit/aes/gcm/nonce
Android docs: https://developer.android.com/reference/kotlin/javax/crypto/KeyGenerator

Depends on D7002

Test Plan

Ensured the function returns random numbers on both platforms:


import { generateKey } from './utils/aes-crypto-module.js';

console.log(generateKey());
// should display sth like: [82, 226, ..., 215, 19, 13] // 32 values

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Mar 8 2023, 9:56 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 8 2023, 9:56 AM

Harbormaster failed remote builds in B17186: Diff 23537!Mar 8 2023, 10:04 AM

bartek added a child revision: D7005: [native][AES] Implement encrypt function on iOS.Mar 8 2023, 10:06 AM

bartek published this revision for review.Mar 8 2023, 10:31 AM

bartek added inline comments.

native/expo-modules/aes-crypto/android/src/main/java/app/comm/android/aescrypto/AESCryptoModule.kt
29 ↗	(On Diff #23537)	I pass the result by argument because returning `TypedArray` doesn't work correctly. TypedArrays support is still experimental/undocumented feature. The same is done in latest SDK-48 `expo-crypto` module: https://github.com/expo/expo/blob/6bb367d3973931b1379ac836bdf875ab9d6850d6/packages/expo-crypto/android/src/main/java/expo/modules/crypto/CryptoModule.kt#L59

bartek added a reviewer: varun.Mar 9 2023, 2:42 AM

Harbormaster completed remote builds in B17186: Diff 23537.Mar 9 2023, 11:55 AM

Left a note about the error-handling on the Swift side, definitely let me know if there's something I'm missing. I think we should just be able to match what we're doing on the Android/Kotlin side?

iOS docs: https://developer.apple.com/documentation/cryptokit/aes/gcm/nonce

(Think you might've meant to link to https://developer.apple.com/documentation/cryptokit/symmetrickey/init(size:))

native/expo-modules/aes-crypto/ios/AESCryptoModule.swift
16–18 ↗	(On Diff #23537)	Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength` is `32` and throw `InvalidDestinationSizeException()` if not? It seems like `destination` is the input that we want to validate, so we should just do that directly? It's not really so much that `SymmetricKey` is giving us incorrect size `key` since it's (I'm assuming) always going to give us exactly the size we're asking for? Feel free to re-request review if there's something I'm missing here.
native/utils/aes-crypto-module.js
12 ↗	(On Diff #23537)	Kind of confusing naming that we're passing `key` to a function named `generateKey`. Maybe `buffer` or `keyBuffer` or `keyDestination` or something along those lines?

This revision now requires changes to proceed.Mar 9 2023, 12:02 PM

bartek marked an inline comment as not done.Mar 10 2023, 12:51 AM

bartek added inline comments.

native/expo-modules/aes-crypto/ios/AESCryptoModule.swift
16–18 ↗	(On Diff #23537)	The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount` property to get the desired destination length, but you're right that it can be done this way: guard destination.byteLength == KEY_SIZE else { throw InvalidKeyLengthException() } let key = SymmetricKey(size: .bits256) I'll refactor this as it is more clear solution
native/utils/aes-crypto-module.js
12 ↗	(On Diff #23537)	Good idea, `keyBuffer` sounds better