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

Expose ways to integrate new key types better #969

Closed
wants to merge 7 commits into from

Conversation

lestrrat
Copy link
Collaborator

Addresses #912

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop/v2@f589fb8). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 92bd45d differs from pull request most recent head f7ab98e. Consider uploading reports for the commit f7ab98e to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##             develop/v2     #969   +/-   ##
=============================================
  Coverage              ?   71.93%           
=============================================
  Files                 ?       95           
  Lines                 ?    13855           
  Branches              ?        0           
=============================================
  Hits                  ?     9967           
  Misses                ?     3054           
  Partials              ?      834           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lestrrat
Copy link
Collaborator Author

I don't like the API that exposes the hooks for the raw keys. I need to rethink it

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@@ -728,6 +495,20 @@ func CurveForAlgorithm(alg jwa.EllipticCurveAlgorithm) (elliptic.Curve, bool) {
return ecutil.CurveForAlgorithm(alg)
}

// KeySpec is a specification for additional key types
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't going to fly. Need to remove

@lestrrat
Copy link
Collaborator Author

Notes: need to fix x25519 for > go1.20

@lestrrat
Copy link
Collaborator Author

note: if we want to use crypto/ecdh for go >= 1.20. we're going to have to break back compat for x25519.Public/PrivateKeys, as ecdh.PublicKey/ecdh.PublicKey expects to be treated as pointers, whereas previously we used []byte, which meant that we didn't need to carry pointers. Aligning this with crypto/ecdh probably means we need to release v3

@lestrrat lestrrat closed this Sep 27, 2023
@lestrrat lestrrat deleted the flexible-raw-keys branch September 25, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant