[viff-devel] [viff-commits] viff: Generate_config_files:Added support NaCl implementation of...

[Janus Dam Nielsen]

> Some good old-fashioned code review coming up... :-)
Great!

>> /rev/736ad1d97024
>> changeset: 1361:736ad1d97024
>> user:      Janus Dam Nielsen <janus.nielsen at alexandra.dk>
>> date:      Wed Oct 28 14:53:51 2009 +0100
>> summary:   Generate_config_files:Added support NaCl implementation  
>> of Paillier.
>
> There's a space missing after the colon.
Ok.

>> diffstat:
>>
>> apps/generate-config-files.py |  22 +++++++++++++++++++---
>> viff/paillierutil.py          |  20 +++++++++++++++++++-
>> 2 files changed, 38 insertions(+), 4 deletions(-)
>>
>> diffs (90 lines):
>>
>> diff -r 3fe6e03541c1 -r 736ad1d97024 apps/generate-config-files.py
>> --- a/apps/generate-config-files.py	Wed Oct 28 14:53:49 2009 +0100
>> +++ b/apps/generate-config-files.py	Wed Oct 28 14:53:51 2009 +0100
>> @@ -55,7 +55,17 @@
>> from optparse import OptionParser
>>
>> from viff.config import generate_configs
>> -from viff.paillierutil import ViffPaillier
>> +from viff.paillierutil import ViffPaillier, NaClPaillier
>> +
>> +try:
>> +    import pypaillier
>> +except ImportError:
>> +    pypaillier = None
>
> Are we getting a module called 'pypaillier' alongside the old module
> called 'paillier'? I don't like that name very much. Perhaps we should
> make a module called nacl so that you could do
>
>  try:
>      from viff.nacl import paillier
>  except ImportError:
>      from viff import paillier
>
> and then make the interface identical for the two modules.
Agree, this is a goal to be pursued soonish, but I would like Marc to  
make a distribution of his work that can be accessed somewhere on the  
internet.
I believe the interfaces are identical

> Also, can we please have that code put into VIFF? I don't like it that
> we're getting more and more "secret" code floating around :-)  
> Especially
> not when we make changes to VIFF to accomodate this secret code -- it
> would be different if we had simple drop-in Python replacements for  
> it.
>
> (I know you've said that this code can be made public since it's  
> part of
> NaCL, so this is more for the record...)
The code will not be a part of VIFF, but a prerequisite like Zope  
interfaces or Twisted. I will issue warning on the mailling list when  
I submit the changeset that will require it.

>> +
>> +paillier_choices = ['viff']
>> +
>> +if pypaillier:
>> +    paillier_choices += ['nacl']
>
> The append method is better for that.
Ok.

>
>> +paillier = ViffPaillier(options.keysize)
>> +if "nacl" == options.paillier:
>> +    paillier = NaClPaillier(options.keysize)
>
> I think it's clearer if you write
>
>  if options.paillier == "nacl":
>      paillier = NaClPaillier(options.keysize)
>  else:
>      paillier = ViffPaillier(options.keysize)
I slightly disagree.
  if "nacl" == options.paillier:
      paillier = NaClPaillier(options.keysize)
  else:
      paillier = ViffPaillier(options.keysize)
Is more natural in the current case.


>> +try:
>> +    import pypaillier
>> +except ImportError:
>> +    pypaillier = None
>> +
>> +
>> class Paillier:
>>
>>     def __init__(self, keysize):
>> @@ -35,8 +41,20 @@
>>
>>     def generate_keys(self):
>>         return paillier.generate_keys(self.keysize)
>> +
>> +class NaClPaillier:
>> +
>> +    def __init__(self, keysize):
>> +        self.keysize = keysize
>> +        self.type = 'nacl'
>> +
>> +    def generate_keys(self):
>> +        return pypaillier.generate_keys(self.keysize)
>>
>>
>> def deserializer(paillier_type, str):
>> -    return tuple(map(long, str))
>> +    if paillier_type == "viff":
>> +        return tuple(map(long, str))
>> +    if paillier_type == "nacl":
>> +        return str.dict()
>
> I think that function would belong in the otherwise unnecessary  
> classes
> you define above?
> Or even better: make a function in two different
> modules like I suggest above.
This is a good question. I choose the current design because it leaves  
you with only one place you should change if you want to add another  
paillier implementation. I need to think some more before I comment  
further on this.

____________________________________________________

Janus Dam Nielsen

Research and Innovationspecialist, PhD.
CENTRE FOR IT-SECURITY

THE ALEXANDRA INSTITUTE LTD.

T +45 42 22 93 56
E janus.nielsen at alexandra.dk
W alexandra.dk
____________________________________________________

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.viff.dk/pipermail/viff-devel-viff.dk/attachments/20091029/c909ce03/attachment-0001.htm>