Page MenuHomePhabricator

[native][AES] Implement generateKey function
ClosedPublic

Authored by bartek on Mar 8 2023, 9:56 AM.
Tags
None
Referenced Files
F3347632: D7004.diff
Fri, Nov 22, 12:49 PM
Unknown Object (File)
Sun, Nov 10, 5:54 PM
Unknown Object (File)
Mon, Nov 4, 6:45 PM
Unknown Object (File)
Sat, Nov 2, 12:28 AM
Unknown Object (File)
Sat, Nov 2, 12:28 AM
Unknown Object (File)
Sat, Nov 2, 12:28 AM
Unknown Object (File)
Sat, Nov 2, 12:28 AM
Unknown Object (File)
Sun, Oct 27, 8:14 PM
Subscribers

Details

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 held this revision as a draft.
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

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

atul requested changes to this revision.Mar 9 2023, 12:02 PM

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

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

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

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

Good idea, keyBuffer sounds better

Address code review feedback

Sweet, thanks for addressing those comments

This revision is now accepted and ready to land.Mar 13 2023, 4:53 PM