feat: enable ed25519 in jwkkid.BuildJWK() #3450
Conversation
Signed-off-by: Filip Burlacu <[email protected]>
pkg/doc/util/jwkkid/kid_creator.go
Outdated
| @@ -52,6 +52,8 @@ func CreateKID(keyBytes []byte, kt kms.KeyType) (string, error) { | |||
|
|
|||
| return x25519KID, nil | |||
| case kms.ED25519Type: // go-jose JWK thumbprint of Ed25519 has a bug, manually build it and build its resulting KID. | |||
| // TODO remove `case kms.ED25519Type` when go-jose fixes Ed25519 JWK thumbprint. | |||
There was a problem hiding this comment.
go jose already implemented the fix, need to update the dep and remove this case fully since j.Thumbprint() below will handle the correct ED25519 type
There was a problem hiding this comment.
What go-jose version added the fix? If we update, do we want to update to latest?
There was a problem hiding this comment.
It dosen't hurt to be at latest.. but I'll check the version when it was updated
There was a problem hiding this comment.
Weird, I could swear to have seen that they fixed it. It doesn't seem to be fixed yet: https://github.com/square/go-jose/blob/v2/jwk.go#L335
it still uses a bad ed25519 jwk string for thumbnail template (missing " before x)
There was a problem hiding this comment.
ahhh... the repo name is different: github.com/go-jose/go-jose
see the fix here: https://github.com/go-jose/go-jose/blob/v3/jwk.go#L341
commit: fa705ca80d23fff27c333fb6475d30047e032200
need to update the go-jose to this new repo instead, the newer v3 has the fix.
square/go-jose is the legacy repo
Codecov Report
@@ Coverage Diff @@
## main #3450 +/- ##
==========================================
+ Coverage 87.58% 87.59% +0.01%
==========================================
Files 346 346
Lines 47011 46992 -19
==========================================
- Hits 41174 41163 -11
+ Misses 4334 4327 -7
+ Partials 1503 1502 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| // if err != nil { | ||
| // return nil, fmt.Errorf("buildJWK: failed to build JWK from ed25519 key: %w", err) | ||
| // } | ||
| case kms.ED25519Type: |
There was a problem hiding this comment.
this is good to uncomment, but go-jose needs to be updated to pick up the fixed thumbprint computation as mentioned in the previous comment.
Signed-off-by: Filip Burlacu <[email protected]>
Signed-off-by: Filip Burlacu [email protected]