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

feat(sdk-lib-mpc): add implementation of paillier-blum #3940

Closed
wants to merge 20 commits into from

Conversation

zahin-mohammad
Copy link
Contributor

@zahin-mohammad zahin-mohammad commented Sep 27, 2023

TICKET: WP-132

@socket-security
Copy link

socket-security bot commented Sep 27, 2023

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎


describe('prove and verify', function () {
// This test takes quite some time ~ 18 seconds
// Test takes > 30s in CI
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will have to address this in a follow up on why this takes too long. Might have to make the paillier key a mocked key.

@@ -2,7 +2,7 @@

module.exports = {
require: 'ts-node/register',
timeout: '20000',
timeout: '40000',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bitgo/sdk-lib-mpc:          creating paillier blum proof should not throw an error, test case 0:
@bitgo/sdk-lib-mpc:      Error: Timeout of 20000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/BitGoJS/BitGoJS/modules/sdk-lib-mpc/test/unit/tss/ecdsa/paillierBlumProof.ts)

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 will need to be optimized in a follow up.

);
h = (h * h) % N;
if (gcd(h, N) === BigInt(1)) {
return h;
Copy link
Contributor

@zhongxishen zhongxishen Oct 3, 2023

Choose a reason for hiding this comment

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

The sampling of y here deviates from https://eprint.iacr.org/2020/492.pdf section 4.3 and the non-interactive version at https://www.zkdocs.com/docs/zkdocs/zero-knowledge-protocols/product-primes/paillier_blum_modulus/#non-interactive-protocol

Here it seems to be sampling y so that it will be the same as y' by implicitly making a = 0, b = 0 (and thus not including a, b in the proof). This will make only certain co-primes of N be picked as the challenge y, violating generateY() being a random oracle.

Copy link
Contributor

@zhongxishen zhongxishen Oct 3, 2023

Choose a reason for hiding this comment

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

Also, for y' (and therefore y here) to be suitable so that its 4th root is well defined, it needs to be a quadratic residue modulo N. Here by making y = h^2 and y (mod N) = 1, y is indeed a quadratic residue N but only for the cases where remainder is 1. Other quadratic residues are missed where x^2 === y (mod N) is other values (e.g. 4, 9, etc.). This further limits the sampling domain of the challenge value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could refer to Fireblocks' implementation https://github.com/fireblocks/mpc-lib/blob/main/src/common/crypto/paillier/paillier_zkp.c#L648

Basically, due to p, q === 3 mod 4 and jacobi(w, N) === -1, it's guaranteed one of (0, 0), (0, 1), (1, 0), (1,1) for (a, b) will make y' a quadratic residue modulo N.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

const beta = randBetween(sqrtN0 << (ELL + EPSILON), -sqrtN0 << (ELL + EPSILON));
const rho = randBetween((nHat * n0) << ELL, -(nHat * n0) << ELL);
// Commit to p.
const mu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think mu should be
randBetween(BigInt(nHat) << ELL, BigInt(-nHat) << ELL) according to https://eprint.iacr.org/2020/492.pdf B.4

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

const mu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL);
const P = (modPow(s, p, nHat) * modPow(t, mu, nHat)) % nHat;
// Commit to q.
const nu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as mu

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

const nu = randBetween(BigInt(1) << ELL, BigInt(-1) << ELL);
const Q = (modPow(s, q, nHat) * modPow(t, nu, nHat)) % nHat;
// Commit to alpha.
const x = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON));
Copy link
Contributor

Choose a reason for hiding this comment

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

1 and -1 should be nHat and -nHat respectively.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

const x = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON));
const A = (modPow(s, alpha, nHat) * modPow(t, x, nHat)) % nHat;
// Commit to beta.
const y = randBetween(BigInt(1) << (ELL + EPSILON), BigInt(-1) << (ELL + EPSILON));
Copy link
Contributor

Choose a reason for hiding this comment

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

1 and -1 should be nHat and -nHat respectively.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

.update('$')
.digest()
);
if (y > BigInt(0) && y < N) {
Copy link
Contributor

Choose a reason for hiding this comment

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

y should also be coprime of N

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

}
if (
modPow(t, (p - BigInt(1)) / BigInt(2), p) === BigInt(1) &&
modPow(t, (q - BigInt(1)) / BigInt(2), q) === BigInt(1)
Copy link
Contributor

@zhongxishen zhongxishen Oct 11, 2023

Choose a reason for hiding this comment

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

This can also be done by checking if y_i and w are quadratic residue of p or q first by calculating
modPow(y_i, (p - 1) / 2, p), modPow(t, (q - 1) / 2, q), modPow(w, (p -1)/2, p) and modPow(w, (q -1)/2, q) as above, the result can be used to determine the value of a_i and b_i. This does need 2 more calculations, so on average it evens out with the try-and-error approach.

Since p, q === 3 mod 4 and are primes, "If p ≡ 3 (mod 4) the negative of a residue modulo p is a nonresidue and the negative of a nonresidue is a residue." according to https://en.wikipedia.org/wiki/Quadratic_residue. Jacobi(w, N) = -1 means w is quadratic residue to one of p, q and nonresidue to the other.

For example, if (y_i/p) = 1 (residue), (y_i/q) = -1 (nonresidue), (w/p) = 1, (w/q) = -1, then a_i = 0, b_i = 1.

let t;
while (true) {
t = y_i;
ab[i] = (await randBits(2))[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there're only 4 cases, I wonder if it's sufficient to just enumerate them, just in case random generation gives duplicate values.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@alebusse alebusse left a comment

Choose a reason for hiding this comment

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

Flushing from the review queue, let me know when its ready to review

@zhongxishen
Copy link
Contributor

github's still having issue syncing this PR to the latest commits, but they've addressed comments on the paillier blum proof.

zhongxishen
zhongxishen previously approved these changes Oct 13, 2023
Copy link
Contributor

@zhongxishen zhongxishen left a comment

Choose a reason for hiding this comment

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

LGTM

islamaminBitGo
islamaminBitGo previously approved these changes Oct 13, 2023
@johnoliverdriscoll
Copy link
Contributor

@zahin-mohammad I don't know how to resolve the issue this "prettier" issue with mockNoSmallFactorsProof.json and mockPaillierBlumProof.json. Can you please help resolve?

@alebusse alebusse dismissed stale reviews from islamaminBitGo and zhongxishen via 15c7b2b October 16, 2023 19:03
@alebusse alebusse force-pushed the WP-132/paillier-blum-proof branch from 15c7b2b to c53c2f3 Compare October 16, 2023 19:14
@alebusse alebusse force-pushed the WP-132/paillier-blum-proof branch 2 times, most recently from 66774d5 to 4df6424 Compare October 17, 2023 16:10
johnoliverdriscoll and others added 20 commits October 18, 2023 11:08
@zahin-mohammad zahin-mohammad force-pushed the WP-132/paillier-blum-proof branch from 4df6424 to 519e200 Compare October 18, 2023 15:09
@zahin-mohammad zahin-mohammad marked this pull request as draft October 18, 2023 15:09
@zahin-mohammad
Copy link
Contributor Author

Rebased

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.

5 participants