ECDSA Alg improvements #276
Conversation
|
@Spomky thanks a lot for your contribution, it's quite nice to remove that dependency. The only thing is that I'd prefer to keep the internal interface and the signature converter as an implementation of it. The main motivation is to isolate things and allow us to abstract that bit from the rest of the lib. If you prefer, I can take it from here (I don't want to ask too much of you 😁) |
|
I updated the PR and restored the PointsManipulator interface. I will also take time to add more tests for these algorithms. |
|
@Spomky thank you very much |
|
Hi @lcobucci , I added some tests taken from the RFC6979. Let me know if you have ideas to fix these metrics. I will modify my PR accordingly |
lcobucci
force-pushed
the
ECDSA
branch
2 times, most recently
from
9a47532 to
c913eff
Compare
There was a problem hiding this comment.
@Spomky I've rebased your branch and reorganised your commits a bit to make it easier to make modifications there.
These comments are minor things that I can adjust, if you agree and don't have time to do it 😄
We can adjust the MSI/Covered MSI for now and create the unit tests later on (specially for the exceptions raised in the new file) - don't worry too much about coverage.
I have a tiny question (please forgive my lack of knowledge regarding crypto): how can we test that the binary manipulation that we now have is not susceptible to vector/timing attacks? Since we were previously using a lib before I didn't bother much with that but now that we have the code here it feels like I'm responsible for providing a reliable solution.
Thanks a lot for this contribution 👍
|
Great. Thank you for the review. |
|
Hi @lcobucci, I modified my PR few hours ago. |
|
@Spomky thanks a lot! I'm actually addressing those deprecations right now, so don't worry about them. Once I'm done you will be able to rebase your branch and sync it with |
|
Hi @lcobucci I hope everything is going well for you. This PR looks good to me. I don't see any optimization at the moment. |
|
@Spomky alright I wasn't sure if you had finished. Thank you for your contribution, I'd take a look on it this week and get it merged in 👍 All the best for you too! |
lcobucci
force-pushed
the
ECDSA
branch
3 times, most recently
from
af99f69 to
23f55cd
Compare
Detaching this library from any external package to perform ECDSA signature creation and verification.
Making things clearer and easier to understand.
Since we're just converting things from/to hexadecimal, we can simply rely on `bin2hex()/hex2bin()`. These are simpler and faster than `pack()/unpack()`.
Refactoring the class a bit to reduce the amount of mutations and make things a bit more understandable.
fgrosse/phpasn1dependency removedAsn1class removedand RFC7520Hi @lcobucci,
As discussed, here is a proposal to get ride of
fgrosse/phpasn1and fix trouble with that library.This could be a first step to help with #259 by backporting this solution.
The class
ECSignaturemay need to be improved as well.Feel free to propose correction on that class if so.