Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
==========================================
- Coverage 82.13% 82.00% -0.13%
==========================================
Files 61 61
Lines 1422 1434 +12
==========================================
+ Hits 1168 1176 +8
- Misses 254 258 +4
🚀 New features to boost your workflow:
|
tkrajacic
left a comment
tkrajacic
left a comment
There was a problem hiding this comment.
This should now refer to the latest swift-crypto release 4.1.0 that includes the required APIs
| let keys = try await JWTKeyCollection() | ||
| .add(eddsa: EdDSA.PublicKey(x: eddsaPublicKeyBase64, curve: .ed25519), kid: "public") | ||
| .add(eddsa: EdDSA.PrivateKey(d: eddsaPrivateKeyBase64, curve: .ed25519), kid: "private") | ||
| let signingCollection = try await JWTKeyCollection() | ||
| .add(eddsa: EdDSA.PrivateKey(d: eddsaPrivateKeyBase64, curve: .ed25519)) | ||
| let verifyingCollection = try await JWTKeyCollection() | ||
| .add(eddsa: EdDSA.PublicKey(x: eddsaPublicKeyBase64, curve: .ed25519)) |
There was a problem hiding this comment.
Because before it seemed like it was using the public key to verify but it wasn't. It wasn't wrong per se but a bit misleading
| /// In JWT, EdDSA public keys are represented as a single x-coordinate and are used for verifying signatures. | ||
| /// Currently, only the ``EdDSACurve/ed25519`` curve is supported. | ||
| public struct PublicKey: EdDSAKey { | ||
| public struct PublicKey: EdDSAKey, Equatable { |
There was a problem hiding this comment.
Out of curiosity, why can't we also make them Hashable?
There was a problem hiding this comment.
We could if there's a use case for it. I wouldn't have made them Equatable either if
- We didn't need it in the tests
- RSA and ECDSA weren't also already
Equatable
There was a problem hiding this comment.
I think it is generally useful to make all your types Hashable, if they are. We can't foresee every usecase, but if someone wants to put them in a Set… there you go.
There was a problem hiding this comment.
I think all keys should conform to Hashable because… they are
There was a problem hiding this comment.
Equatable means "represents a value which can be meaningfully compared for sameness with another value".
Identifiable means "represents a value which has unique identity even if it compares the same with another value".
Hashable means "is Equatable and represents a value which can be quickly and efficiently reduced to a hashing key with appropriate properties".
There are very few things which are semantically Equatable without being semantically Hashable. In Swift, "not Hashable" is effectively used to mean "this type should not be treated as having sufficiently unique representation to be used as a dictionary key or to be uniqued by set inclusion".
As regards asymmetric encryption keys, Hashable is by this definition not semantically appropriate. You should not be keying a dictionary by a crypto key or trying to construct unique sets of crypto keys. In the infosec world, these would definitely qualify as potentially dangerous behaviors (in terms of reducing security).
4 participants
This adds PEM representation support for EdDSA keys, it's now possible to create EdDSA keys with PEM and get their PEM representation.
This also adds the missing
Equatableconformance to EdDSA keys and fixes some tests.