Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secp256k1 - disable malleability check when verifying #244

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

mistermoe
Copy link
Member

@mistermoe mistermoe commented Oct 17, 2023

relevant context here: TBD54566975/web5-kt#87

the best course of action may be to provide callers with the option to enable/disable the malleability check. Just didn't have the time to plumb this optionality down through the crypto algorithms interface. Wanted to get this PR out so that Verifiable Credential signature verification doesn't 💩 out for creds signed in environments that don't guarantee a low-s signature.

cc: @andresuribe87

@codesandbox
Copy link

codesandbox bot commented Oct 17, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #244 (267c415) into main (f3c7184) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
- Coverage   90.99%   90.97%   -0.03%     
==========================================
  Files          67       67              
  Lines       12645    12648       +3     
  Branches     1260     1260              
==========================================
  Hits        11506    11506              
- Misses       1116     1119       +3     
  Partials       23       23              
Components Coverage Δ
api 94.06% <ø> (-0.27%) ⬇️
common 95.00% <ø> (ø)
credentials 92.77% <ø> (ø)
crypto 100.00% <100.00%> (ø)
dids 92.16% <ø> (ø)
agent 88.16% <ø> (ø)
identity-agent 59.05% <ø> (ø)
proxy-agent 58.59% <ø> (ø)
user-agent 57.36% <ø> (ø)

@mistermoe
Copy link
Member Author

@frankhinek can i get a stamp here?

Copy link
Contributor

@diehuxx diehuxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the very detailed write up in the linked web5-kt issue.

Copy link
Contributor

@frankhinek frankhinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@frankhinek frankhinek merged commit b4820ef into main Nov 2, 2023
22 of 23 checks passed
@frankhinek frankhinek deleted the secp256k1-verify branch November 2, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants