Skip to content

feat: multiple plaintext authentication#91

Merged
lonerapier merged 9 commits into
mainfrom
feat/multiple-plaintext-authentication
Jan 22, 2025
Merged

feat: multiple plaintext authentication#91
lonerapier merged 9 commits into
mainfrom
feat/multiple-plaintext-authentication

Conversation

@lonerapier
Copy link
Copy Markdown
Contributor

@lonerapier lonerapier commented Jan 15, 2025

closes #89
closes #80

@lonerapier lonerapier changed the title Feat/multiple plaintext authentication feat: multiple plaintext authentication Jan 15, 2025
@lonerapier lonerapier marked this pull request as ready for review January 16, 2025 16:46
Comment thread package.json Outdated
@Autoparallel
Copy link
Copy Markdown
Contributor

Can you make an updated diagram for this version?

Copy link
Copy Markdown
Contributor

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

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

Looks good to me. Given tests are all passing and this is being integrated into web prover, we will know with certainty this all works.

Please see my comments, especially about versioning. Let me know if you want to meet!

Comment thread circuits/http/verification.circom Outdated
Comment thread circuits/chacha20/authentication.circom
Comment thread circuits/utils/hash.circom
@@ -0,0 +1,13 @@
pragma circom 2.1.9;

function log2Ceil(a) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this could be used in the JSON parser/machine. I have a comment about it in there actually.

Comment thread circuits/test/utils/hash.test.ts
@lonerapier lonerapier force-pushed the feat/multiple-plaintext-authentication branch from 5b1ac19 to 53d33e1 Compare January 17, 2025 14:12
Copy link
Copy Markdown
Contributor

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

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

Added more comments

Comment thread package.json Outdated
Comment thread circuits/chacha20/authentication.circom
}

step_out[0] <== step_in[0] + body_digest_hashed - accumulated_main_digests_hashed - data_digest_hashed; // TODO: data_digest is really plaintext_digest from before, consider changing names
step_out[0] <== step_in[0] + body_digest_hashed - accumulated_main_digests_hashed - pt_digest;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to hash pt_digest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, i had thought about that, but then how do we calculate pt_digest_hash in authentication circuit?

my reasoning to just use digest and not hash was because all others were hashes, so you'd still have to find preimage of those to cancel these?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, because of the different components...

Okay, I see.

Can you please make some issues on some of these potential security problems like this we're introducing? Then I'm good to sign off on this..

@devloper
Copy link
Copy Markdown
Contributor

high level review lgtm, I'll defer to colin for deeper comments.

@lonerapier lonerapier merged commit bc071c1 into main Jan 22, 2025
@lonerapier lonerapier deleted the feat/multiple-plaintext-authentication branch January 22, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support multiple ciphertext matching ci: release artifacts circuit assert

3 participants