Page MenuHomePhabricator
Differential D7004

[native][AES] Implement generateKey function
ClosedPublic
Actions

Authored by bartek on Mar 8 2023, 9:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 20, 2:15 PM
Unknown Object (File)
Sun, Apr 20, 1:40 PM
Unknown Object (File)
Sun, Apr 20, 10:05 AM
Unknown Object (File)
Sun, Apr 20, 9:47 AM
Unknown Object (File)
Sun, Apr 20, 7:34 AM
Unknown Object (File)
Sat, Apr 19, 2:39 PM
Unknown Object (File)
Thu, Apr 17, 1:19 PM
Unknown Object (File)
Fri, Apr 11, 5:17 AM
View All Files
Subscribers
ashoat
tomek

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
Lint Not Applicable
Unit
Tests Not Applicable

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
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 ↗(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

bartek updated this revision to Diff 23669.Mar 13 2023, 4:54 AM

Address code review feedback

Harbormaster completed remote builds in B17280: Diff 23669.Mar 13 2023, 5:10 AM
atul accepted this revision.Mar 13 2023, 4:53 PM

Sweet, thanks for addressing those comments

This revision is now accepted and ready to land.Mar 13 2023, 4:53 PM
bartek updated this revision to Diff 23840.Mar 20 2023, 6:55 AM

Rebase before landing

Harbormaster completed remote builds in B17398: Diff 23840.Mar 20 2023, 7:11 AM
Closed by commit rCOMM46037bfe9b83: [native][AES] Implement generateKey function (authored by bartek). · Explain WhyMar 20 2023, 7:15 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM46037bfe9b83: [native][AES] Implement generateKey function.

Revision Contents
Changeset List

PathSize
native/
expo-modules/
aes-crypto/
android/
src/
main/
java/
app/
comm/
android/
aescrypto/
39 lines
ios/
36 lines
utils/
10 lines

Diff 23853

View Options

native/expo-modules/aes-crypto/android/src/main/java/app/comm/android/aescrypto/AESCryptoModule.kt

import ExpoModulesCore import ExpoModulesCore
import CryptoKit
public class AESCryptoModule: Module { public class AESCryptoModule: Module {
public func definition() -> ModuleDefinition { public func definition() -> ModuleDefinition {
Name("AESCrypto") Name("AESCrypto")
Function("hello") { () -> String in Function("generateKey", generateKey)
return "Hello world! 👋"
} }
} }
// MARK: - Function implementations
private func generateKey(destination: Uint8Array) throws {
let key = SymmetricKey(size: .bits256)
guard destination.byteLength * 8 == key.bitCount else {
throw InvalidKeyLengthException()
}
atulUnsubmitted
Not Done
Inline Actions

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.

atul: Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength`…
bartekAuthorUnsubmitted
Done
Inline Actions

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

bartek: The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount`…
key.withUnsafeBytes { bytes in
let _ = bytes.copyBytes(to: destination.rawBufferPtr())
}
}
// MARK: - Utilities
extension TypedArray {
func rawBufferPtr() -> UnsafeMutableRawBufferPointer {
UnsafeMutableRawBufferPointer(start: self.rawPointer,
count: self.byteLength)
}
}
// MARK: - Exception definitions
private class InvalidKeyLengthException: Exception {
override var reason: String {
"The AES key has invalid length"
}
} }
View Options

native/expo-modules/aes-crypto/ios/AESCryptoModule.swift

import ExpoModulesCore import ExpoModulesCore
import CryptoKit
public class AESCryptoModule: Module { public class AESCryptoModule: Module {
public func definition() -> ModuleDefinition { public func definition() -> ModuleDefinition {
Name("AESCrypto") Name("AESCrypto")
Function("hello") { () -> String in Function("generateKey", generateKey)
return "Hello world! 👋"
} }
} }
// MARK: - Function implementations
private func generateKey(destination: Uint8Array) throws {
let key = SymmetricKey(size: .bits256)
guard destination.byteLength * 8 == key.bitCount else {
throw InvalidKeyLengthException()
}
atulUnsubmitted
Not Done
Inline Actions

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.

atul: Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength`…
bartekAuthorUnsubmitted
Done
Inline Actions

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

bartek: The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount`…
key.withUnsafeBytes { bytes in
let _ = bytes.copyBytes(to: destination.rawBufferPtr())
}
}
// MARK: - Utilities
extension TypedArray {
func rawBufferPtr() -> UnsafeMutableRawBufferPointer {
UnsafeMutableRawBufferPointer(start: self.rawPointer,
count: self.byteLength)
}
}
// MARK: - Exception definitions
private class InvalidKeyLengthException: Exception {
override var reason: String {
"The AES key has invalid length"
}
} }
View Options

native/utils/aes-crypto-module.js

import ExpoModulesCore import ExpoModulesCore
import CryptoKit
public class AESCryptoModule: Module { public class AESCryptoModule: Module {
public func definition() -> ModuleDefinition { public func definition() -> ModuleDefinition {
Name("AESCrypto") Name("AESCrypto")
Function("hello") { () -> String in Function("generateKey", generateKey)
return "Hello world! 👋"
} }
} }
// MARK: - Function implementations
private func generateKey(destination: Uint8Array) throws {
let key = SymmetricKey(size: .bits256)
guard destination.byteLength * 8 == key.bitCount else {
throw InvalidKeyLengthException()
}
atulUnsubmitted
Not Done
Inline Actions

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.

atul: Before we even generate `key` via `SymmetricKey`, should we check that `destination.byteLength`…
bartekAuthorUnsubmitted
Done
Inline Actions

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

bartek: The only reason the guard is below `SymmetricKey` is that I wanted to use its `bitCount`…
key.withUnsafeBytes { bytes in
let _ = bytes.copyBytes(to: destination.rawBufferPtr())
}
}
// MARK: - Utilities
extension TypedArray {
func rawBufferPtr() -> UnsafeMutableRawBufferPointer {
UnsafeMutableRawBufferPointer(start: self.rawPointer,
count: self.byteLength)
}
}
// MARK: - Exception definitions
private class InvalidKeyLengthException: Exception {
override var reason: String {
"The AES key has invalid length"
}
} }