Skip to content
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

Passing encryption key via a callback #1636

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

nhachicha
Copy link
Collaborator

@nhachicha nhachicha commented Jan 18, 2024

This PR adds the ability to pass in the AES encryption key from a native memory region. After opening the Realm the caller can reset this native memory region to reduce the window where the key is available clear in memory.

  • Update Core when it's ready
  • Test will not pass on Android (need to add a helper method in JNI to allocate native memory for the openEncryptedRealmWithEncryptionKeyCallback test)

Closes #1705

…ry region that can be reset later to enhance security
- Adding jni lib for Android test
@nhachicha nhachicha marked this pull request as ready for review January 19, 2024 09:24
Copy link
Contributor

@cmelchior cmelchior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice...great description in the docs. I do have a few comments about the API docs + some missing tests, but it is minor

@@ -354,6 +378,52 @@ public interface Configuration {
public fun encryptionKey(encryptionKey: ByteArray): S =
apply { this.encryptionKey = validateEncryptionKey(encryptionKey) } as S

/**
* Similar to [encryptionKey] but instead this will read the encryption key from native memory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanation 💯

// These touches the notifier and writer lazy initialised Realms to open them with the provided configuration.
awaitAll(
async(notificationScheduler.dispatcher) {
notifier.realm.version().version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, would it make sense to have a helper method like openRealm() or something, just to make it more readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a TODO. There might be other feedbacks we can incorporate after the POC

packages/test-base/src/androidMain/cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@ExperimentalEncryptionCallbackApi
public interface EncryptionKeyCallback {
/**
* Provides the native memory address of the 64 byte array containing the key used to encrypt and decrypt the Realm file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also document that this can be called multiple times and that release is only called once, so they do not accidentially create a new Pointer for every call to this, but only release it once.

This was also why I thought it might make a better API if this was an symmetric API, i.e. called 3 times, and released 3 times, but I do agree that the current behavior is easier to implement for the sake of an POC.

Copy link
Contributor

@cmelchior cmelchior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. CI will be fixed by #1633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RLM_ERR_INVALID_DATABASE] Realm cannot be decrypted after a migration from java-sdk to kotlin-sdk
2 participants