[INDY-1389] Support Proof of Possession for BLS keys #867

Toktar · 2018-08-10T14:35:15Z

No description provided.

INDY-1112: change primeries election procedure for backup instances. …

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

…nto task-1389-bls-proof

Changes: - added test_add_node_with_invalid_key_proof - added test_fail_node_bls_key_validation Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

jovfer · 2018-08-13T17:21:28Z

@mikelodder7 would you like to review usage BLS PoP from IndyCrypo in this PR?

mikelodder7

Are we storing an aggregated public key that includes the proof of possession?
Its hard to tell if that's the case with these changes.

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

…om/Toktar/indy-plenum into task-1389-bls-proof

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Toktar · 2018-08-14T06:52:01Z

@mikelodder7 thank you for your review. No, we are not store the proof of possession and public, privat keys are storing like before. The proof of possession generates in every cases when it need. In the fact, it happens only with generating keys in sending transaction for creating new Node or change a current bls key.

ashcherbakov · 2018-08-14T06:59:57Z

crypto/test/bls/indy_crypto/test_bls_crypto_indy_crypto.py



-@pytest.yield_fixture(scope="function", params=['48', '32', '31', '33'])
+@pytest.yield_fixture(scope="function", params=['30', '32', '31', '22'])


Why did we change the params?

ashcherbakov · 2018-08-14T07:12:12Z

crypto/test/bls/indy_crypto/test_bls_crypto_indy_crypto.py

    assert sk != pk
+    assert bls_verifier.verify_key_proof_of_possession(key_proof, pk)


Please add more tests for verification of PoP. Please add tests for negative cases (where verification fails).

ashcherbakov · 2018-08-14T07:21:35Z

plenum/server/pool_req_handler.py

-        pass
+        blskey = request.operation.get(DATA).get(BLS_KEY, None)
+        blskey_proof = request.operation.get(DATA).get(BLS_KEY_PROOF, None)
+        if request.txn_type == NODE and blskey is not None and (


Please split the condition into a more simple form:

if request.txn_type != NODE: return blskey = request.operation.get(DATA).get(BLS_KEY, None) if blskey is None: return blskey_proof = request.operation.get(DATA).get(BLS_KEY_PROOF, None) if blskey_proof is None: raise InvalidClientRequest(request.identifier, request.reqId, "A Proof of possession must be provided with BLS key) if not self._verify_bls_key_proof_of_possession(blskey_proof, blskey)): raise InvalidClientRequest(request.identifier, request.reqId, "Proof of possession {} is incorrect for BLS key {}". format(blskey_proof, blskey))

ashcherbakov · 2018-08-14T07:25:08Z

plenum/test/bls/test_bls_crypto_factory.py

@@ -28,30 +28,30 @@ def bls_crypto_factory2(tdir_for_func):


 def test_create_and_store_bls_keys(bls_crypto_factory):
-    pk = bls_crypto_factory.generate_and_store_bls_keys()
+    pk, key_proof = bls_crypto_factory.generate_and_store_bls_keys()
    assert pk
    assert isinstance(pk, str)


Please add assert for key_proof as well

ashcherbakov · 2018-08-14T07:25:15Z

plenum/test/bls/test_bls_crypto_factory.py

    assert pk
    assert isinstance(pk, str)


 def test_create_bls_keys(bls_crypto_factory):
-    sk, pk = bls_crypto_factory.generate_bls_keys()
+    sk, pk, key_proof = bls_crypto_factory.generate_bls_keys()
    assert pk
    assert sk
    assert isinstance(sk, str)


Please add assert for key_proof as well

ashcherbakov · 2018-08-14T07:25:31Z

plenum/test/bls/test_bls_crypto_factory.py

-    pk3 = bls_crypto_factory.generate_and_store_bls_keys()
+    pk1, key_proof1 = bls_crypto_factory.generate_and_store_bls_keys()
+    pk2, key_proof2 = bls_crypto_factory.generate_and_store_bls_keys()
+    pk3, key_proof3 = bls_crypto_factory.generate_and_store_bls_keys()
    assert pk1 != pk2 != pk3


Please add assert that proofs are different

ashcherbakov · 2018-08-14T07:27:16Z

plenum/test/pool_transactions/test_add _node_with_invalid_key_proof.py

@@ -0,0 +1,53 @@
+import pytest


Please remove the space from file name

ashcherbakov · 2018-08-14T07:28:42Z

plenum/server/pool_req_handler.py

        super().__init__(ledger, state)
        self.domainState = domainState
        self.stateSerializer = pool_state_serializer
+        self.bls_crypto_verifier = bls_crypto_verifier

    def doStaticValidation(self, request: Request):


Please add unit tests for this method

ashcherbakov · 2018-08-14T07:30:52Z

plenum/test/pool_transactions/test_add _node_with_invalid_key_proof.py

+    assert "BLS key proof {} incorrect for key {}".format(key_proof, bls_key) \
+           in e._excinfo[1].args[0]
+
+    return create_and_start_new_node(looper, new_node_name, tdir, sigseed,


Why do we need this line?

ashcherbakov · 2018-08-14T07:33:47Z

plenum/server/pool_req_handler.py

@@ -21,13 +22,22 @@ class PoolRequestHandler(LedgerRequestHandler):
    write_types = {NODE, }



We also need checks that a Node can participate in BLS signing only if it has a valid PoP for its key.
We can not enable this right now, since nodes in live pools need to send PoP first.

ashcherbakov · 2018-08-14T10:58:22Z

plenum/test/bls/test_bls_crypto_factory.py

@@ -31,6 +31,8 @@ def test_create_and_store_bls_keys(bls_crypto_factory):
    pk, key_proof = bls_crypto_factory.generate_and_store_bls_keys()
    assert pk
    assert isinstance(pk, str)
+    assert not bls_crypto_factory.bls_verifier\


Why verify should return False here?

Toktar · 2018-08-14T11:30:54Z

test this please

mikelodder7

Looks good. I believe it’s validating the proof of possession when registering a new key

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Toktar · 2018-08-15T11:23:58Z

test this please

lampkin-diet · 2018-08-17T07:03:39Z

test this please

Toktar and others added 10 commits March 5, 2018 10:40

Merge pull request #1 from hyperledger/master

e85917f

INDY-1112: change primeries election procedure for backup instances. …

INDY-1389: add generating bls key proof of possession

b6f78fa

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: update tests

0f31b00

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: add static validation for bls keys and update tests

3d70e38

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Merge branch 'master' of https://github.com/hyperledger/indy-plenum i…

8898cc1

…nto task-1389-bls-proof

INDY-1389: add tests for key proof of possession validation

435dc26

Changes: - added test_add_node_with_invalid_key_proof - added test_fail_node_bls_key_validation Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: change crypto version to stable 0.4.3

31f6b02

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: add stable repo for sdk

3ec1e47

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: fix key proof in test_network_setup.py

447b748

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: update tests work with bls

6c0ddcc

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Toktar changed the title ~~[WIP][INDY-1389] Support Proof of Possession for BLS keys~~ INDY-1389: Support Proof of Possession for BLS keys Aug 13, 2018

mikelodder7 reviewed Aug 13, 2018

View reviewed changes

Toktar added 4 commits August 14, 2018 02:54

INDY-1389: update tests with bls and pool handler refactoring

decd125

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Merge branches 'master' and 'task-1389-bls-proof' of https://github.c…

4e9ed8e

…om/Toktar/indy-plenum into task-1389-bls-proof

INDY-1389: refactoring test_pool_req_handler_static_validation.py

636bca5

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: refactoring tests with bls

4c0d047

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

ashcherbakov reviewed Aug 14, 2018

View reviewed changes

mikelodder7 previously approved these changes Aug 14, 2018

View reviewed changes

Toktar dismissed mikelodder7’s stale review via 60bcf20 August 14, 2018 13:04

Toktar force-pushed the task-1389-bls-proof branch from 60bcf20 to 4c0d047 Compare August 14, 2018 13:21

INDY-1389: fix test_pool_req_handler_static_validation imports

3b89630

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Toktar changed the title ~~INDY-1389: Support Proof of Possession for BLS keys~~ [WIP]INDY-1389: Support Proof of Possession for BLS keys Aug 15, 2018

Toktar changed the title ~~[WIP]INDY-1389: Support Proof of Possession for BLS keys~~ [WIP][INDY-1389] Support Proof of Possession for BLS keys Aug 15, 2018

Toktar added 2 commits August 15, 2018 12:39

INDY-1389: fix tests in test_bls_crypto_factory.py

b3ba8aa

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

INDY-1389: remove sdk repo from dockerfile

26da913

Signed-off-by: toktar <renata.toktar@dsr-corporation.com>

Toktar changed the title ~~[WIP][INDY-1389] Support Proof of Possession for BLS keys~~ [INDY-1389] Support Proof of Possession for BLS keys Aug 17, 2018

ashcherbakov approved these changes Aug 17, 2018

View reviewed changes

ashcherbakov merged commit fcf9ce3 into hyperledger:master Aug 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INDY-1389] Support Proof of Possession for BLS keys #867

[INDY-1389] Support Proof of Possession for BLS keys #867

Toktar commented Aug 10, 2018

jovfer commented Aug 13, 2018

mikelodder7 left a comment

Toktar commented Aug 14, 2018 •

edited

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

ashcherbakov Aug 14, 2018

Toktar commented Aug 14, 2018

mikelodder7 left a comment

Toktar commented Aug 15, 2018

lampkin-diet commented Aug 17, 2018



		@pytest.yield_fixture(scope="function", params=['48', '32', '31', '33'])
		@pytest.yield_fixture(scope="function", params=['30', '32', '31', '22'])

		assert sk != pk
		assert bls_verifier.verify_key_proof_of_possession(key_proof, pk)

		@@ -21,13 +22,22 @@ class PoolRequestHandler(LedgerRequestHandler):
		write_types = {NODE, }

[INDY-1389] Support Proof of Possession for BLS keys #867

[INDY-1389] Support Proof of Possession for BLS keys #867

Conversation

Toktar commented Aug 10, 2018

jovfer commented Aug 13, 2018

mikelodder7 left a comment

Choose a reason for hiding this comment

Toktar commented Aug 14, 2018 • edited

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

Toktar commented Aug 14, 2018

mikelodder7 left a comment

Choose a reason for hiding this comment

Toktar commented Aug 15, 2018

lampkin-diet commented Aug 17, 2018

Toktar commented Aug 14, 2018 •

edited