Simplify code (crypto/NewEllipticCurve) #122
Conversation
- using elliptic.P256() - using DecodeBytes (remove bytes.NewReader)
|
Hey, the big.Int library is var-time so it is susceptible to timing attacks and not suitable for cryptographic purposes. There was a proposal to add in const time ops a year ago, but it has not been added yet: golang/go#20654 |
|
@decentralisedkev I just replaced the code with the existing one in golang basic library I roughly understand what you are talking about, but the decision that was before, also uses big.Int .. ignoring the existing code in the golang base supply. I believe it would be more correct to use the golang library and expect a fix there if there is no better suggestion. |
|
I'm not sure I follow. The existing code used big.Int which is var-time, I was suggesting that instead of refactoring on top of it, that it be replaced with a constant time implementation. The golang elliptic curve implementations don't use big.Int as far as I know, here is the ed25519 curve implementation as reference: https://github.com/golang/crypto/blob/master/ed25519/internal/edwards25519/edwards25519.go |
|
@decentralisedkev I thought a little bit about your suggestion and think we need to wait for a fix in Golang. I'm no expert in cryptography, but I did a quick study of the subject you described. It seems to me that the proposed implementation is more correct than it was, but I can not now offer a better solution. |
|
@im-kulikov Sure, I think the only solutions would be to use a library with cgo, implement it natively or wait for Golang to implement it, I'm not quite sure when this will be though |
|
Oh.. I propose to use what is in PR and yet wait for the Golang implementation. Using CGO is not the best solution. |
|
@im-kulikov Oh I understand now. It will not be safe to use in a production environment though with the timing attack. I agree, using cgo may not be the best solution. However, it may be the only one available right now. What do you think? |
|
At the moment, we have the same not safe solution. Among other things, it is also self-written, that is, the probability of the problems described by you is much higher. Solution with cgo, IMO, I wouldn't consider. We should write tests to find bottlenecks and try to solve them with goasm.. this option me seems more go-way... How's that for an option? |
|
@decentralisedkev as I see, only |
|
I close, because it is left unanswered and there is no clarity whether it will be required in the future or not. |
cc @anthdm / @fyrchik / @aprasolova / @alexvanin