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

verifyKey is not taken into consideration #149

Open
jakobsa opened this issue Oct 5, 2017 · 3 comments
Open

verifyKey is not taken into consideration #149

jakobsa opened this issue Oct 5, 2017 · 3 comments
Labels

Comments

@jakobsa
Copy link

jakobsa commented Oct 5, 2017

Info Value
Platform Name ios
Platform Version 10
CocoaLumberjack Version 2.3.0
Integration Method cocoapods
Xcode Version Xcode 8.3
Repro rate all the time (100%)
JWT Version 3-beta4

Issue Description and Steps

I have to verify a JWT token using RS256. The public key material is received from an endpoint as modulus and exponent. I could not use any of the provided key extractors for key material in that format. I had to create the JWTCryptoKeyPublic instance myself and found the verifyKey block useful as it should allow for directly setting the public key the verifier should use.

I was surprised to see that it would not work, and after digging into the sources I found that secretData needed to be set (here is why: JWTCoding+VersionThree:560).

SecretData referred to as key will not be used by RSBaseAlgorithms down the callstack as self.verifyKey is preferred then. (JWTAlgorithmRSBase:127-129).

To summarize.

Holders configured like that do not work:

JWTAlgorithmRSFamilyDataHolder* verifyDataHolder = [JWTAlgorithmRSFamilyDataHolder new];
verifyDataHolder.algorithmName(JWTAlgorithmNameRS256).verifyKey(publicKey);

Holders configured like that will work:

JWTAlgorithmRSFamilyDataHolder* verifyDataHolder = [JWTAlgorithmRSFamilyDataHolder new];
verifyDataHolder.algorithmName(JWTAlgorithmNameRS256).secretData([NSData new]).verifyKey(publicKey);
@jakobsa jakobsa changed the title verifyKey is not taken into cond verifyKey is not taken into consideration Oct 5, 2017
@lolgear
Copy link
Collaborator

lolgear commented Oct 5, 2017

@jakobsa Hi!
It is definitely a bug or a big bone! :)
What did you expect and how you would like to solve this?

The core point now is that holders MUST have secretData as a core point of signature purposes. (not good point :/)
Yes, they are too coupled to main HS algorithm. ( which intended to have secretData ).

If you would like, we could make a list of appropriate considerations and desires to further API 3.0.

One of the points: maybe algorithms should hold secret data with which they should work?

@jakobsa
Copy link
Author

jakobsa commented Oct 5, 2017

@lolgear

For me only token verification was relevant, so I didn't consider the implications of signing much.
I am not deep enough into the sources to really make qualified suggestions but I will give it a shot as sometimes a outsiders view can be refreshing (or at least funny ;) ).

My suggestions are only valid if the following assumptions are true:

  • SecretData is an arbitrary data format for key material used by dedicated KeyExtractors only.
  • SecretData is always converted into an internal key format before it is used for verifying / signing.
  • verifyKey is the result of that conversion (for verification only).

Suggestion:

  • add signingKey as property to holders
  • prefer verifyKey and signingKey over key extraction from secretData (as it already provides the internal key format)
  • do the keyExtraction (using secretData, yielding verifyKey + signingKey) as high up in the callstack as possible and only use verifyKey + signingKey down from there.

@lolgear
Copy link
Collaborator

lolgear commented Oct 21, 2019

@jakobsa Hi, could you check latest master? It has been fixed, I guess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants