Blind index #118

coltfred · 2020-02-20T22:42:16Z

TODO

Tests
Clarify use of the words index and indexed terms

BobWall23 · 2020-02-21T16:54:27Z

src/search.rs

+///The required length of the salt.
+const REQUIRED_LEN: usize = 32;
+
+///The result of creating a new index as well as indexing on a particular phrase.


We return this structure when they generate the index tokens for a string?

BobWall23 · 2020-02-21T16:56:05Z

src/search.rs

+ pub sdk: IronSimpleSearch,
+}
+
+///Trait which gives the ability to create an index


Maybe the ability to create an index and the ability to initialize an existing index should be in separate traits. Everyone using the SDK needs initialize - the ability to create the index might not even be included in the normal import app.

BobWall23 · 2020-02-21T16:59:19Z

src/search.rs

+ ///If you need to index terms immediately, see `create_index_and_initialize` which will return
+ ///the IronSimpleSearch for reuse.
+ async fn create_index(&self, group_id: &GroupId) -> Result<DocumentEncryptUnmanagedResult>;
+ ///Create an index and also index the term.


Doesn't actually index anything, does it? Just returns the SDK "handle".
I'm not sure how useful it is to have these two separate. Maybe just have create_search_index and initialize_search_index?

BobWall23 · 2020-02-21T17:00:04Z

src/search.rs

+ ///Create an index and encrypt it to the provided group_id.
+ ///If you need to index terms immediately, see `create_index_and_initialize` which will return
+ ///the IronSimpleSearch for reuse.
+ async fn create_index(&self, group_id: &GroupId) -> Result<DocumentEncryptUnmanagedResult>;


Shouldn't the DocumentEncryptUnmanagedResult that this returns be the input initialize_search_index?

The DocumentEncryptUnmanagedResult has the people it was encrypted to, errors about the sharing, etc. Maybe I should change the structure to just include the relevant data.

Or maybe give it an alias that better describes its usage here?

src/search.rs

clintfred

Review not complete, still trying to wrap my head around some of the details here, but I have some initial feedback that I hope will generate discussion and be helpful on it's own.

src/search.rs

clintfred · 2020-02-28T17:03:57Z

src/search.rs

+#[async_trait]
+pub trait SimpleSeachInitialize {
+ ///Given the encrypted salt and the edeks, decrypt them and give back the IronSimpleSearch object.
+ async fn initialize_search_sdk(


naming suggestions: initialize_search

src/search.rs

clintfred · 2020-02-28T18:14:29Z

src/search.rs

+ ///Given the encrypted salt and the edeks, decrypt them and give back the IronSimpleSearch object.
+ async fn initialize_search_sdk(
+ &self,
+ encrypted_data: &[u8],


Should we just call this the encrypted_salt?

I changed it to encrypted_salt_bytes to hopefully avoid confusion between the entire "EncryptedSalt" which is the deks + bytes and just the Aes bytes.

clintfred · 2020-02-28T18:24:53Z

src/search.rs

+
+///The result of creating a new index as well as initializing an IronSimpleSdk.
+///If you only want to create the index, see create_index.
+pub struct CreatedIndexResult {


Suggest: CreateIndexResult or SearchIndexCreateResult or IndexCreateResult

Of the suggestions, I would choose IndexCreateResult

Shouldn't create_index return an Index?

Or maybe a BlindIndex?

What do you think about putting the encrypted_salt in the BlindIndexSearchSdk struct and getting rid of this altogether?

Maybe this would look like getting rid of create_index (or not) and changing create_index_and_initialize_search to also return Result<BlindIndexSearchSdk>

Discussed offline that this type of construction doesn't make sense as the existence of a BlindIndexSearch doesn't necessarily mean that an EncryptedBlindIndexSalt exists.

So I'm back to suggesting IndexCreateResult as a name here, unless someone has something better. I think what bothers me about the current name is that it starts with a verb.

clintfred · 2020-02-28T18:28:19Z

src/search.rs

+ ///Create an index and encrypt it to the provided group_id.
+ ///If you need to index terms immediately, see `create_index_and_initialize` which will return
+ ///the IronSimpleSearch for reuse.
+ async fn create_index(&self, group_id: &GroupId) -> Result<DocumentEncryptUnmanagedResult>;


Or maybe give it an alias that better describes its usage here?

clintfred · 2020-03-03T19:26:49Z

src/search.rs

+
+///The result of creating a new index as well as initializing an IronSimpleSdk.
+///If you only want to create the index, see create_index.
+pub struct CreatedIndexResult {


What do you think about putting the encrypted_salt in the BlindIndexSearchSdk struct and getting rid of this altogether?

Maybe this would look like getting rid of create_index (or not) and changing create_index_and_initialize_search to also return Result<BlindIndexSearchSdk>

src/search.rs

clintfred · 2020-03-03T19:35:41Z

src/search.rs

+
+#[async_trait]
+impl BlindIndexSearchInitialize for IronOxide {
+ async fn initialize_search(


consider rename to initialize_blind_index_search?

You said "ok", but didn't change it. Intentional?

Sorry. Pushed.

src/search.rs

clintfred · 2020-03-03T19:43:11Z

src/search.rs

+ }
+
+ ///Generate the list of tokens to use to find entries that match the search query, given the specified partition_id.
+ pub fn tokenize_query(&self, query: &str, partition_id: Option<&str>) -> Vec<u32> {


I don't understand what partition_id is. These two functions could use more descriptive docs, perhaps with an example.

Added some more text about it.

clintfred · 2020-03-03T22:16:06Z

src/search.rs

+
+ ///Generate the list of tokens to use to find entries that match the search query, given the specified partition_id.
+ /// query - The string you want to tokenize and hash
+ /// partition_id - An extra string you want to include in every hash, this allows 2 queries with different partition_ids to produce a different set of


I think you forgot to end your

I don't know what

tests/search_ops.rs

ernieturner · 2020-03-04T21:13:20Z

Cargo.toml

@@ -40,6 +40,7 @@ vec1 = "~1.4.0"
 # ironoxide requires rt-threaded at runtime, but not at compile time
 tokio = {version = "~0.2.0", features=["time", "rt-threaded"]}
 dashmap = "3.4.0"
+ironcore-search-helpers = { git = "https:/IronCoreLabs/search-helpers", rev = "38be96fe6160a739396cf505054a4e0dec6e0aa6", optional=true}


Is the plan to still add tags to this repo so we can point to a specific tag for versioning?

Turns out we have to publish it or we can't publish ironoxide. I'll do that.

ernieturner · 2020-03-04T21:25:56Z

src/search.rs

+
+ ///Generate the list of tokens to use to find entries that match the search query, given the specified partition_id.
+ /// query - The string you want to tokenize and hash
+ /// partition_id - An extra string you want to include in every hash, this allows 2 queries with different partition_ids to produce a different set of tokens for the same data


This comment looks like it was just copy/pasted from tokenize_query where I assume it serves a different purpose even though the arguments are the same type.

That's because they currently do the same thing. I'll change it when it gets updated to do something else.

That time has come

I changed the comment. Let me know if you think it could be improved.

ernieturner · 2020-03-06T16:32:11Z

src/search.rs

+pub struct BlindIndexCreateResult {
+ pub encrypted_salt: EncryptedBlindIndexSalt,
+ pub sdk: BlindIndexSearch,
+}


This looks to be unused now, which matches up with what we discussed in the meeting.

src/search.rs

Co-Authored-By: Craig Colegrove <[email protected]>

coltfred added 6 commits February 20, 2020 13:18

Initial version of the blind_index

2f69089

Move to search and add the beta flag

dee02f6

Tests

88e9c76

Cleanup imports and array initialization

762b984

Remove term & partition from create_index_and_initialize

8f24d1f

Fix comments

0688796

BobWall23 reviewed Feb 21, 2020

View reviewed changes

Address code review comments

349b2d7

clintfred suggested changes Feb 28, 2020

View reviewed changes

Code review and another round of naming

19be78c

clintfred suggested changes Mar 3, 2020

View reviewed changes

Address code review

1d2a882

clintfred reviewed Mar 3, 2020

View reviewed changes

tests/search_ops.rs Outdated Show resolved Hide resolved

coltfred added 2 commits March 3, 2020 15:42

Code review

a61c89c

Fix tests

3a782f7

ernieturner reviewed Mar 4, 2020

View reviewed changes

Renaming

f685978

ernieturner reviewed Mar 6, 2020

View reviewed changes

giarc3 reviewed Mar 6, 2020

View reviewed changes

src/search.rs Outdated Show resolved Hide resolved

src/search.rs Outdated Show resolved Hide resolved

src/search.rs Outdated Show resolved Hide resolved

giarc3 assigned coltfred Mar 6, 2020

coltfred and others added 7 commits March 6, 2020 10:27

Merge branch 'master' into blind-index

017b848

Merge branch 'master' into blind-index

bf52850

Apply suggestions from code review

92d827a

Co-Authored-By: Craig Colegrove <[email protected]>

Remove incorrect commend and unused struct

d871c1f

add create_blind_index to BlockingIronOxide

b14c4f5

add initialize_search_blocking()

219165a

add derive to EncryptedBlindIndexSalt

938e5b4

clintfred approved these changes Mar 12, 2020

View reviewed changes

Merge branch 'master' into blind-index

f45a862

coltfred added 2 commits March 12, 2020 12:57

Fix build

a6ef164

Updated comment

b034007

coltfred merged commit b88cdbc into master Mar 12, 2020

coltfred deleted the blind-index branch March 12, 2020 21:57

coltfred mentioned this pull request Mar 16, 2020

Consume changes to search-helpers #127

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blind index #118

Blind index #118

coltfred commented Feb 20, 2020 •

edited

Loading

BobWall23 Feb 21, 2020

BobWall23 Feb 21, 2020

BobWall23 Feb 21, 2020

BobWall23 Feb 21, 2020

coltfred Feb 21, 2020

clintfred Feb 28, 2020

clintfred left a comment

clintfred Feb 28, 2020

clintfred Feb 28, 2020

coltfred Mar 3, 2020

clintfred Feb 28, 2020

giarc3 Mar 3, 2020

BobWall23 Mar 3, 2020 •

edited

Loading

BobWall23 Mar 3, 2020

clintfred Mar 3, 2020

clintfred Mar 3, 2020

clintfred Feb 28, 2020

clintfred Mar 3, 2020

clintfred Mar 3, 2020

coltfred Mar 3, 2020

clintfred Mar 3, 2020

coltfred Mar 3, 2020

clintfred Mar 3, 2020

coltfred Mar 3, 2020

clintfred Mar 3, 2020

coltfred Mar 3, 2020

ernieturner Mar 4, 2020

coltfred Mar 10, 2020

ernieturner Mar 4, 2020

coltfred Mar 10, 2020

giarc3 Mar 12, 2020

coltfred Mar 12, 2020

ernieturner Mar 6, 2020

coltfred Mar 10, 2020

Blind index #118

Blind index #118

Conversation

coltfred commented Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintfred left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BobWall23 Mar 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coltfred commented Feb 20, 2020 •

edited

Loading

BobWall23 Mar 3, 2020 •

edited

Loading