fireproof icon indicating copy to clipboard operation
fireproof copied to clipboard

Deterministic iv

Open jchris opened this issue 1 year ago • 11 comments

the point of this is to make it so two clients generate the same car file for the same operations.

jchris avatar Aug 12 '24 20:08 jchris

this needs a test to make sure it does that

jchris avatar Aug 12 '24 20:08 jchris

The double hash intuition is that even outside of our cryptosystem, the single hash will correspond to the cleartext cid, and so if someone is dropping blocks in both clear and encrypted stores, this allows those blocks to be looked up from the iv. Double hashing the cid prevents the iv from becoming a list of things to search for in the clear world. To fully respect the crypto system maybe we want the hash(cid + hash(dbKey)) so we can't be searched via rainbow tables of hashed cids

Agree on all the other points

jchris avatar Aug 13 '24 15:08 jchris

we should start integration testing that different clients get deterministics car file names from this branch. There could be other sources of non-determinism.

jchris avatar Aug 13 '24 15:08 jchris

There is no difference in rainbow attacks by double hashing and if we cut to 96 bit basically everything is broken the only why to calculate the iv is by knowing the original text but to guess the text from the iv is due to the cut almost impossible

mabels avatar Aug 13 '24 16:08 mabels

To ensure the contract i would check the iv of a known value in the test.

What about the cut to 96 bit?

We could do that in algo but I would think we could do it here for the double hash we could also fold/XOR the remaining sha256 bit into one 96 bits .

About your integrationtest what you have in mind? I would love to automate them

mabels avatar Aug 13 '24 16:08 mabels

We could do that in algo but I would think we could do it here for the double hash we could also fold/XOR the remaining sha256 bit into one 96 bits .

This sounds like it would work. If we assume the clear CIDs are indexed somewhere, then we need something you can't prefix search with. In this case folding the hash to build a rainbow table is same as double hashing, so this addresses my concern.

jchris avatar Aug 13 '24 20:08 jchris

For integration testing, ideally show that a third party reading / following live a 2-party concurrent collaboration session sees fewer car files in total for a big enough workload after the change.

jchris avatar Aug 13 '24 20:08 jchris

New test is expected to fail it would be great to have help configuring the fireproof() call in the test so both instance A and B have the same key.

If this carlog test doesn't pass when we fix the configuration issue so that both databases are using the same key, then we should have one confirmation we are on the right track.

If it doesn't pass then, we need to look for other sources of non-determinism.

jchris avatar Aug 13 '24 22:08 jchris

The only part of above that isn't done is the make it configurable - allow on construction to pass a iv generator function and then of course I need help in the setup function (will note below) to make a two database with same key material.

Note a database that is connected while empty should adopt the remote key, so we will shortly be testing how to do this without special configuration via the connect API.

jchris avatar Aug 14 '24 01:08 jchris

you can preset the key to get deterministic results. the same are for the database it's the URL parameter storekey.

I'm joining you tomorrow:)

meno

On Wed, 14 Aug 2024, 03:13 Chris Anderson, @.***> wrote:

@.**** commented on this pull request.

In tests/fireproof/fireproof.test.ts https://github.com/fireproof-storage/fireproof/pull/166#discussion_r1716173690 :

  • });
  • beforeEach(async function () {
  • let ok: DocResponse;
  • await rt.SysContainer.start();
  • // todo this fails because the test setup doesn't properly configure both databases to use the same key
  • dbA = fireproof("test-dual-workload-a");
  • for (const doc of docs) {
  •  ok = await dbA.put(doc);
    
  •  expect(ok).toBeTruthy();
    
  •  expect(ok.id).toBeTruthy();
    
  • }
  • headA = dbA._crdt.clock.head.toString();
  • // todo this fails because the test setup doesn't properly configure both databases to use the same key
  • dbB = fireproof("test-dual-workload-b");

should use the same key as database A

— Reply to this email directly, view it on GitHub https://github.com/fireproof-storage/fireproof/pull/166#pullrequestreview-2236944511, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEWJQ3BJCVK7LCWI3TBTDZRKVKZAVCNFSM6AAAAABMM4DTHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMZWHE2DINJRGE . You are receiving this because your review was requested.Message ID: @.***>

mabels avatar Aug 14 '24 06:08 mabels

the only example I see in the tests of calling fireproof() with config is here

there are also plenty of config examples here, so I'll try that

btw we should take a look at the config in real app use cases and consider if we want to sugar the API at all

jchris avatar Aug 14 '24 13:08 jchris

it looks I might have to do something like this? we should make an api shortcut to just set various URL params on top of whatever baseURL the runtime chooses https://github.com/fireproof-storage/fireproof/blob/1baf2b253ea2ad9d855572e2ca67c96dc0a8c991/tests/blockstore/keyed-crypto.test.ts#L100

jchris avatar Aug 14 '24 14:08 jchris

oh that feature to use a base58 key I missed

I will add that tomorrow morning

back from the digital desert

On Wed, Aug 14, 2024 at 6:52 PM Chris Anderson @.***> wrote:

@.**** commented on this pull request.

In tests/fireproof/fireproof.test.ts https://github.com/fireproof-storage/fireproof/pull/166#discussion_r1717263483 :

@@ -602,3 +603,109 @@ describe("basic js verify", function () { await db.destroy(); }); });

+describe("same workload twice, same CID", function () {

  • let dbA: Database;
  • let dbB: Database;
  • let headA: string;
  • let headB: string;
  • // let configA: any;
  • // let configB: any;
  • const configA = {
  • store: {
  •  stores: {
    
  •    base: "file://./dist/databaseA?storekey=zTvTPEPQRWij8rfb3FrFqBm",
    

setting the key like this gives

Error: {"module":"CRDT","level":"error","error":{"module":"keyedCrypto","url":"file://./dist/databaseA?name=test-dual-workload-a&store=data&storekey=%40test-dual-workload-a%3Adata%40&version=v0.19-file","name":"test-dual-workload-a","level":"error","fp":"z4Xw4qTVndnS6ATUrFUEpdtmGzRgfgCbvfPjccC4QdtKG","keyId":"z2YcX9rjbvnPnqG33TDLoD1K5UC7ZSvkQMTM3217kWP6L","msg":"keyId mismatch"},"msg":"CRDT not ready"}

— Reply to this email directly, view it on GitHub https://github.com/fireproof-storage/fireproof/pull/166#pullrequestreview-2238719283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEWJVMHTGYEAFTIEAL3TLZRODL3AVCNFSM6AAAAABMM4DTHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMZYG4YTSMRYGM . You are receiving this because your review was requested.Message ID: @.***>

mabels avatar Aug 14 '24 18:08 mabels

looks good will merge

jchris avatar Aug 14 '24 20:08 jchris