Skip to content
This repository was archived by the owner on Mar 8, 2024. It is now read-only.

Conversation

@nhartner
Copy link
Collaborator

@nhartner nhartner commented Sep 9, 2020

High Level Overview of Change

Change default curve and algorithm to P-256 / ES256 because secp256k1 / ES256k does not have wide support by JWS libraries.

Context of Change

Per jwt.io, ES256k is not supported by the majority of JWT libraries, and none for .Net, Go, Rust, and browser-based JS libraries.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Before / After

Before: generate and sign commands would generate an sepc256k1 key and sign with ES256k.
After: generate and sign commands generate an P-256 key and sign with ES256.

Test Plan

Unit tests
Manually run the keys generate and sign commands and verify that the signed PayID has the expected curve and algorithm in the protected section.

@nhartner nhartner requested a review from 0xCLARITY as a code owner September 9, 2020 23:09
Copy link

@0xCLARITY 0xCLARITY left a comment

Choose a reason for hiding this comment

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

LGTM.

Just to double-check, although we sign with ES256, we can verify a signature with any supported algorithm, correct?

@nhartner
Copy link
Collaborator Author

Yes, this only changes the default for signing. PayIDs signed with other supported JWS curves and algorithm can still be verified.

@nhartner nhartner merged commit fd888e0 into master Sep 10, 2020
@nhartner nhartner deleted the default-P256-with-ES256 branch December 21, 2020 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants