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

Allow passing cipher to Hive store #62

Closed
aramperes opened this issue Apr 19, 2022 · 3 comments · Fixed by #65
Closed

Allow passing cipher to Hive store #62

aramperes opened this issue Apr 19, 2022 · 3 comments · Fixed by #65
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@aramperes
Copy link
Contributor

Follow-up to #1.

The current implementation uses newHiveDefaultVaultStore without passing the encryptionCipher parameter; as far as I can tell this simply stores the cookies in plain text under /tmp/cookievault.hive:

$ strings tmp/cookievault.hive

Hcookies-568e0fc623584e64ca46a227e484848febbadcd1459b0c8d708a9fd4234b0aa2
cookies-568e0fc623584e64ca46a227e484848febbadcd1459b0c8d708a9fd4234b0aa2
creationTime
2022-04-18T20:21:16.354189
accessTime
2022-04-18T20:21:16.354189
updateTime
2022-04-18T20:21:16.354189
value
     "Secret-Token": "<snip>"

Currently the Hive store is static final and cannot be changed by the user:

// Creates a store
static final store = newHiveDefaultVaultStore(path: path);
// Creates a vault from the previously created store
static final vault = store.vault<String>(
name: 'cookieVault',
eventListenerMode: EventListenerMode.synchronous,
);


Since the recent move has been to detach from flutter, I think the glue for storing and providing the encryption key for HiveAesCipher should be left to the user, for example by storing the AES key in flutter_secure_storage. However, for this to be feasible, there should be a way for the user to customize the store before initialization.

@sehnryr sehnryr added enhancement New feature or request good first issue Good for newcomers labels Apr 19, 2022
@aramperes
Copy link
Contributor Author

aramperes commented Apr 19, 2022

Since the codebase seems to be mostly static, my suggested implementation would be to make store non-final, and to expose a Requests.setCookieStore() function that defines store if it doesn't already exist.

Or, for better UX, maybe make it Requests.setEncryptionKey() and use the AES cipher internally.

Then, for backwards compatibility, the vault is only initialized when accessed for the first time. If store hasn't been defined, it is also initialized with the default/plain-text parameters.

I would also add a big warning in the README that the store is not encrypted by default, and an example on how to use the AES cipher with flutter_secure_storage.

(Note: I'm not familiar with this codebase, or Dart in general really, so let me know if you have something else in mind)

@sehnryr
Copy link
Collaborator

sehnryr commented Apr 19, 2022

Your proposition seems really good, and I really think it should be implemented that way, but as I was tweaking with the .hive file earlier I realized that there is no way for hive or stash to read a save file with a different encryption key and as such the code will throw this exception:

HiveError: Wrong checksum in hive file. Box may be corrupted.
#0      StorageBackendVm.initialize
package:hive/…/vm/storage_backend_vm.dart:98
<asynchronous suspension>
#1      HiveImpl._openBox
package:hive/src/hive_impl.dart:110
<asynchronous suspension>
#2      HiveImpl.openBox
package:hive/src/hive_impl.dart:140
<asynchronous suspension>

This exception will concern anyone who will have several scripts using requests with a unique encryption key since the store name will be the same for every instance.

I'm currently trying finding a way to make the save file unique to each instance of requests but also thinking that storing the cookies in memory could be a better alternative, encrypted or not.

@aramperes
Copy link
Contributor Author

This exception will concern anyone who will have several scripts using requests with a unique encryption key since the store name will be the same for every instance.

Presumably, if the scripts are using the same cookie file they should be using the same encryption key when using setCookieStore. They would also be able to pass distinct path parameters instead of using the default file in order to have separate encryption keys. This assumes that they are running in separate processes since the library is a singleton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
2 participants