Add constructor to allow Curve to the specified#16
Conversation
- added new method to permit backward compatibility - replicated the switch statement Note slight concern that some of func in the switch really should be in mathlib Signed-off-by: Matthew B White <whitemat@uk.ibm.com>
| // as per the idemixgen file determine which | ||
| // curve is being used, and proceed accordingly | ||
| switch curveID { | ||
| case math.FP256BN_AMCL: |
There was a problem hiding this comment.
Considered that it might be better to move some of this to the Mathlib library... i.e. be able to get from the mathlib library the 'translator' structure - save having to manually code it here.
There was a problem hiding this comment.
Need to add some more specific tests as well...
|
|
||
| // NewIdemixMspCurve creates a new instance of idemixmsp allowing the Elliptic Curve to be specified | ||
| // Must be one of the support curves from https://github.com/IBM/mathlib | ||
| func NewIdemixMspCurve(version MSPVersion, curveID math.CurveID) (MSP, error) { |
There was a problem hiding this comment.
The MSP is configured using protobuf, you can just expand the protobuf definition and specify the curve there.
Expanding the constructor gives you nothing because you anyway need to encode the curve in the channel config, which is done via encoding it in the protobuf message that configures the MSP.
There was a problem hiding this comment.
But the configuration is loaded from a folder and we don't have a place in the folder where we store the curve used by idemix.
An interesting alternative would be to recognise the curve used by looking at the issuer public key. The feasibility needs to be investigated though.
There was a problem hiding this comment.
But the configuration is loaded from a folder and we don't have a place in the folder where we store the curve used by idemix.
Right but we have the MSP Config YAML file where we already specify whether node OUs are used or not, can we not take a similar approach here?
An interesting alternative would be to recognise the curve used by looking at the issuer public key. The feasibility needs to be investigated though.
For a randomly picked public key that would work, but, are you sure the curves do not intersect at any point?
(Yeah, I know that if you find an intersection that doesn't mean you know the discrete log, but, still, could it not be some attack vector where you misclassify the curve and then somewhere along the way, unmarshaling fails for an element in the curve and you panic?)
There was a problem hiding this comment.
Yes, we can introduce a yaml file to further configure the MSP. I agree.
And, right, the intersection issue is there.
So, either we introduce the configuration file or the capability sets the curve used for idemix. I'm fine either way.
There was a problem hiding this comment.
on second thoughts; I've been looking over the code in Fabric that loads the MSP (Idemix or otherwise) - the structure there does appear to point to using a config property of the MSP - but one that only would be used if the channel capability were at the correct level.
If I've understood correctly wouldn't that give different scopes for which curve is used? Does a configuration file in the idemix msp directory mean the choice of the curve is scope by the MSP, whereas with the channel capability that's on a channel level?
Adding an extra config file on top of all the config files is adding complexity - so adding to the channel config would from an engineering perspective seems to be better in my view - if it makes sense from a security perspective.
|
@ale-linux @adecaro @yacovm I'm closing this in favour of #20 |
3 participants
Note slight concern that some of func in the switch really should be in mathlib
Signed-off-by: Matthew B White whitemat@uk.ibm.com