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

refactor: add @cloudflare/voprf-ts/facade and pass crypto as arg everywhere #49

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

sublimator
Copy link
Contributor

@sublimator sublimator commented Oct 10, 2023

Supercedes #44

Pass Around Crypto As Arg Everywhere

  1. Oprf.Group -> hash(...) doesn't belong here! and WebCrypto doesn't support shake256
  2. Oprf.Crypto -> circular dependencies!
  3. CryptoImpl.provider -> better, but ick, a global?
  4. Explicit optional provider passing (crypto?: CryptoProvider) -> tests failing, where??
  5. Required (crypto: CryptoProvider)-> fixed, but damn that's verbose and breaks api
  6. --> !!CryptoProvider via...Rest syntax -> nice, can configure Rest as [CryptoProvider] or [CryptoProvider?]!! <---
  7. But why? -> Can make sure passing around the provider everywhere
  8. But why? -> Oprf.withConfig({provider: CryptoNoble) and old Oprf work in same process
  9. So what? -> In any case ...

Toggle this setting here and try the npm run examples in both modes
Fails to build when it's required - added to ci

@cloudflare/voprf-ts/facade, backwards compat impl of #46

See examples

Named imports for examples

Uses tsx to execute examples/bench which supports runtime compilerOptions.paths to provide more readily modifiable examples.

Breaking changes

Minimum changes needed for: cloudflare/privacypass-ts#15
Also see: Port to facade

diff --git a/src/priv_verif_token.ts b/src/priv_verif_token.ts
index 08c95c9..8809078 100644
--- a/src/priv_verif_token.ts
+++ b/src/priv_verif_token.ts
@@ -9,7 +9,6 @@ import {
     VOPRFClient,
     VOPRFServer,
     generateKeyPair,
-    type DLEQParams,
     type Group,
     type SuiteID,
     type HashID,
@@ -31,7 +30,6 @@ export interface VOPRFExtraParams {
     Ns: number;
     Nk: number;
     hash: HashID;
-    dleqParams: DLEQParams;
 }
 
 const VOPRF_SUITE = Oprf.Suite.P384_SHA384;
@@ -44,12 +42,6 @@ const VOPRF_EXTRA_PARAMS: VOPRFExtraParams = {
     Ns: VOPRF_GROUP.scalarSize(),
     Nk: Oprf.getOprfSize(VOPRF_SUITE),
     hash: VOPRF_HASH,
-    dleqParams: {
-        gg: VOPRF_GROUP,
-        hashID: VOPRF_HASH,
-        hash: Oprf.Crypto.hash,
-        dst: '',
-    },
 } as const;
 
 // Token Type Entry Update:
@@ -82,6 +74,7 @@ export class TokenRequest2 {
     //   } TokenRequest;
 
     tokenType: number;
+
     constructor(
         public readonly truncatedTokenKeyId: number,
         public readonly blindedMsg: Uint8Array,
@@ -257,7 +250,7 @@ export class Client2 {
             throw new Error('no token request was created yet');
         }
 
-        const proof = DLEQProof.deserialize(VOPRF.dleqParams, tokRes.evaluateProof);
+        const proof = DLEQProof.deserialize(VOPRF.group.id, tokRes.evaluateProof);
         const evaluateMsg = VOPRF.group.desElt(tokRes.evaluateMsg);
         const evaluation = new Evaluation(Oprf.Mode.VOPRF, [evaluateMsg], proof);
         const [authenticator] = await this.finData.vClient.finalize(

TODO

  • Full document Security.md that summarizes the current status of both back-ends (@armfazh)

src/facade/types.ts Outdated Show resolved Hide resolved
@sublimator sublimator changed the title refactor: crypto provider arg optionality switch refactor: add @cloudflare/voprf-ts/facade and pass crypto as arg everywhere Oct 12, 2023
package.json Outdated
"examples": "tsx examples/index.ts",
"examples:facade": "tsx examples/facade/index.ts",
"lint": "eslint .",
"bench": "tsx bench/index.ts",
Copy link
Contributor Author

@sublimator sublimator Oct 12, 2023

Choose a reason for hiding this comment

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

using tsx for examples/bench - CI does build:others

],
"compilerOptions": {
"paths": {
"@cloudflare/voprf-ts": ["../src/index.js"],
Copy link
Contributor Author

@sublimator sublimator Oct 12, 2023

Choose a reason for hiding this comment

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

makes examples easily copy/pastable - execute via tsx to support the paths at runtime

return sjcl;
});
}
// if(typeof module !== 'undefined' && module.exports){
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this seems unneeded - was done to stop tsx complaining

@@ -98,6 +87,8 @@ export interface Group extends SerializationHelpers {
}

export interface GroupCons {
fromID(id: GroupID): Group
get(id: GroupID): Group
Copy link
Contributor Author

Choose a reason for hiding this comment

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

idempotent/get/cached semantics

@sublimator sublimator marked this pull request as ready for review October 12, 2023 02:49
src/facade/impl.ts Outdated Show resolved Hide resolved
@sublimator
Copy link
Contributor Author

sublimator commented Oct 13, 2023 via email

@sublimator sublimator closed this Oct 13, 2023
@sublimator sublimator reopened this Oct 14, 2023
// OPRF protocol only specifies encodings for these elements
export type OPRFElement = Elt | Scalar | DLEQProof

// So we only encode elements, and leave packet encoding to application layer
Copy link
Contributor Author

@sublimator sublimator Oct 14, 2023

Choose a reason for hiding this comment

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

Key point. Codec class and Server/Client wrappers wouldn't be needed if serialize() serialized objects to objects of same shape, where leaf nodes were Uint8Array(Uint8Array|DLEQProof|Elt|Scalar) | number (ModeID). This would be a more versatile format for a low level library where applications may include elements in their own packet formats.

Ideally it would be possible to only need the facade to implement the privacy pass protocol or similar, so any packets that are going on the wire, will just be an object of serialized elements.

FinalizeData keeps the "wet" EvaluationRequest, so it doesn't need to pointlessly dehydrate/decode serialized blinded Elts.

Copy link
Contributor

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

lgtm, could you please squash the commits

@sublimator sublimator force-pushed the nd-switch-flipping-2023-10-10 branch from c6f1f3f to 53881f1 Compare December 22, 2023 06:06
@sublimator
Copy link
Contributor Author

@armfazh

Nice! Beginning to feel a lot like xmas!

@sublimator
Copy link
Contributor Author

@thibmeu @armfazh

Bump? :)

@thibmeu
Copy link
Contributor

thibmeu commented Jan 15, 2024

Merging now. It should be part of a future release. Thanks for the change @sublimator, and @armfazh for the review!

@thibmeu thibmeu merged commit 492e8b7 into cloudflare:main Jan 15, 2024
5 checks passed
@sublimator
Copy link
Contributor Author

Awesome :)

@sublimator
Copy link
Contributor Author

And indeed, thank you @armfazh

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.

3 participants