Convert EC signature data and use unified verify #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #4 +/- ##
==========================================
- Coverage 84.2% 83.83% -0.38%
==========================================
Files 150 150
Lines 5781 5771 -10
==========================================
- Hits 4868 4838 -30
- Misses 913 933 +20
Continue to review full report at Codecov.
|
eJamesLin
reviewed
Nov 28, 2018
|
|
||
| // For byte >= 0x80 (first bit is 1), prefix 0x00 to make Security framework happy. | ||
| if rBytes.first! >= UInt8(0x80) { | ||
| rBytes = [0x00] + rBytes |
There was a problem hiding this comment.
Guess this is doing the Signed big-endian encoding of minimal length 👍
Reference: https://crypto.stackexchange.com/a/1797 🙇
There was a problem hiding this comment.
It depends on the implementation detail of the security framework/algorithm we are using. Here for Security.framework, we need to do so, otherwise, the signature is not accepted.
rmundo
approved these changes
Nov 28, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the workaround used for ECDSA verification. Previously, there was a mistake on extra bit prefixing (we thought we need to prefix 0x00 when the first bit is above than 0x70, but actually it should be 0x80).
It turns out that the
. ecdsaSignatureMessageX962SHA256algorithm works well for the newSecKeyVerifySignatureAPI, as long as we convert the raw {r,s} signature data to DER-encoded format. So it is no need to make the verifying method seperated anymore.