Skip to content

Conversation

@PeterSurda
Copy link
Member

@PeterSurda PeterSurda added enhancement New feature developers The issue is relevant mainly for developers rather than users test labels Aug 26, 2019
@PeterSurda PeterSurda added this to the Undefined milestone Aug 26, 2019
@PeterSurda PeterSurda requested a review from g1itch August 26, 2019 15:50
@PeterSurda
Copy link
Member Author

I'll fix the pylint complaints.

@PeterSurda PeterSurda force-pushed the blindsig1 branch 2 times, most recently from 14e56bf to 1c50db5 Compare August 26, 2019 16:50
Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

Honestly, I don't understand what exactly is going on here, but your test case may be better.

@@ -0,0 +1,27 @@
#!/usr/bin/env python
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shebang is not needed here.

import os
import unittest

from src.pyelliptic.eccblind import ECCBlind
Copy link
Collaborator

Choose a reason for hiding this comment

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

from pybitmessage.pyelliptic.eccblind import ECCBlind

Copy link
Member Author

Choose a reason for hiding this comment

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

This breaks the test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It depends on how you run it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Travis CI should be OK, because I used such imports before: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/tests/test_config.py#L8

blind_sig.create_signing_request(msg)
blind_sig.blind_sign()
blind_sig.unblind()
assert blind_sig.verify()
Copy link
Collaborator

Choose a reason for hiding this comment

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

self.assertTrue(blind_sig.verify())

@staticmethod
def test_blind_sig():
"""
Perform a test of full sequence using a random certifier's key and a
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you write docstring in one line it will be shown in the test results. e.g.

    def test_blind_sig(self):
        """Test full sequence using a random certifier key and a random msg"""

@PeterSurda
Copy link
Member Author

@g1itch I'll make the changes you requested.

The low level math I also don't understand, but it looks like it does what the paper says. On a high level, the paper mentions 5 steps (in Section 4), these are roughly mapped to 6 high level methods in the ECCBlind class, with some reshuffling because some are executed by the signer and some by the requester. It needs to be refactored, because the signer and requester need to communicate and pass data, so there will be one object at the signer's side and one at the requester's side, but here I just reuse a single object. Also in __init__, it now generates a new pubkey, wereas for actual deployment the pubkey will be loaded from the disk (like a CA for traditional PKI). There still are open questions about how to serialise the data. This all will be solved in the subsequent PRs.

@g1itch
Copy link
Collaborator

g1itch commented Aug 27, 2019

I also expected to find the blinded message and its signature and check the signature's validity.

@PeterSurda
Copy link
Member Author

PeterSurda commented Aug 27, 2019

@g1itch The message is only blinded internally and such blinded message is only available to the requester, it's not made public. Similarly with the blinded signature, that's calculated by the signer and then transmitted to the requester and unblinded by him.

What you see publicly is the message (in the case of PyBitmessage's wire protocol, this will be the object data), the signature (which has two components, s and F), and pubkey of the signer (Q). There is also r, but that's derived from F. F is a point on a finite field and r is its x coordinate modulo n, although it looks to me like the current code skips the modulo n, the way I understand the code is that n is selected from the elliptic curve used, so x can't higher than that. But I don't really understand ECC so I may be wrong.

Then during the verification, you take the signature, the signer's pubkey and the message, and if verification succeeds, it means the singer signed the message, just like with traditional PKI, except here the signer doesn't know which of the requests he signed correspond to which message..

"""
Generate an ECC keypair
"""
d = ECCBlind.ec_get_random(group, ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

d = self.ec_get_random(group, ctx) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

pylint/flake8 complained.

Copy link
Collaborator

Choose a reason for hiding this comment

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

because of @staticmethod. well, maybe this really should be a static method

@g1itch
Copy link
Collaborator

g1itch commented Aug 27, 2019

I am trying to split signer and requester with no luck ):

    def test_blind_sig(self):
        """Test full sequence using a random certifier key and a random msg"""
        signer_obj = ECCBlind()
        signer_obj.signer_init()
        signer_obj.create_signing_request('dummy')

        requester_obj = ECCBlind()
        requester_obj.R = signer_obj.R
        requester_obj.keypair = (0, signer_obj.keypair[1])

        msg = os.urandom(64)
        requester_obj.create_signing_request(msg)

        signer_obj.m_ = requester_obj.m_
        signer_obj.blind_sign()

        requester_obj.s_ = signer_obj.s_
        requester_obj.unblind()

        signer_obj.m = requester_obj.m
        signer_obj.signature = requester_obj.signature
        signer_obj.F = signer_obj.signature[1]
        self.assertTrue(signer_obj.verify())

@PeterSurda
Copy link
Member Author

let me try...

- add blind signature functionality to pyelliptic as described in Bitmessage#1409
- add tests for blind signatures
- PEP8 fixes for pyelliptic
- some minor refactoring is necessary for further integration, this is just a
  minimal implementation to pass a test
@PeterSurda
Copy link
Member Author

@g1itch here you go.

requester_obj = ECCBlind(pubkey=signer_obj.pubkey)
# only 64 byte messages are planned to be used in Bitmessage
msg = os.urandom(64)
msg_blinded = requester_obj.create_signing_request(point_r, msg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe self.assertNotEqual(msg, msg_blinded)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well that will always succeed as msg is a string and msg_blinded is an openssl bignum, but I can convert it before comparison, and I can also compare blinded an unblinded signature (those should both be bignums already).

As I said before, there is still no serialisation for the wire protocol, as that has to be designed first, then there will be minor refactoring here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They will obviously differ. That's just a tests logic (as I understand it) to compare just in case and document it. Maybe that's unnecessary.

Serialization will be probably done in the network package, so it's not related to the blind signature implementation itself. The signatures seems to work well. So are you going to merge this now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add the tests and then merge.

- primitive serialisation (BN_bn2bin and ctypes) used in intermediary tests
@PeterSurda
Copy link
Member Author

@g1itch I added tests to compare signature with blinded signature, and message with blinded message, all in form which serialises data to bytes with OpenSSL.BN_bn2bin and ctypes.cast. I think this format is usable on wire but more tests are needed.

@PeterSurda PeterSurda merged commit 395fbcd into Bitmessage:v0.6 Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developers The issue is relevant mainly for developers rather than users enhancement New feature test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants