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

bug: KeyStore.toJWKS(true) crashed if keystore includes some public keys #42

Closed
2 tasks done
Alexsey opened this issue Sep 15, 2019 · 4 comments
Closed
2 tasks done
Labels
wontfix This will not be worked on

Comments

@Alexsey
Copy link
Contributor

Alexsey commented Sep 15, 2019

Describe the bug

If the KeyStore instance includes at least one public key then .toJWKS(true) would throw an error public key cannot be exported as private

To Reproduce

const jose = require('@panva/jose')

const privateKey = `-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQCqGKukO1De7zhZj6+H0qtjTkVxwTCpvKe4eCZ0FPqri0cb2JZfXJ/DgYSF6vUp
wmJG8wVQZKjeGcjDOL5UlsuusFncCzWBQ7RKNUSesmQRMSGkVb1/3j+skZ6UtW+5u09lHNsj6tQ5
1s1SPrCBkedbNf0Tp0GbMJDyR4e9T04ZZwIDAQABAoGAFijko56+qGyN8M0RVyaRAXz++xTqHBLh
3tx4VgMtrQ+WEgCjhoTwo23KMBAuJGSYnRmoBZM3lMfTKevIkAidPExvYCdm5dYq3XToLkkLv5L2
pIIVOFMDG+KESnAFV7l2c+cnzRMW0+b6f8mR1CJzZuxVLL6Q02fvLi55/mbSYxECQQDeAw6fiIQX
GukBI4eMZZt4nscy2o12KyYner3VpoeE+Np2q+Z3pvAMd/aNzQ/W9WaI+NRfcxUJrmfPwIGm63il
AkEAxCL5HQb2bQr4ByorcMWm/hEP2MZzROV73yF41hPsRC9m66KrheO9HPTJuo3/9s5p+sqGxOlF
L0NDt4SkosjgGwJAFklyR1uZ/wPJjj611cdBcztlPdqoxssQGnh85BzCj/u3WqBpE2vjvyyvyI5k
X6zk7S0ljKtt2jny2+00VsBerQJBAJGC1Mg5Oydo5NwD6BiROrPxGo2bpTbu/fhrT8ebHkTz2epl
U9VQQSQzY1oZMVX8i1m5WUTLPz2yLJIBQVdXqhMCQBGoiuSoSjafUhV7i1cEGpb88h5NBYZzWXGZ
37sJ5QsW+sJyoNde3xH8vdXhzU7eT82D6X/scw9RZz+/6rCJ4p0=
-----END RSA PRIVATE KEY-----`

const publicKey = `-----BEGIN PUBLIC KEY-----
MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCqGKukO1De7zhZj6+H0qtjTkVxwTCpvKe4eCZ0
FPqri0cb2JZfXJ/DgYSF6vUpwmJG8wVQZKjeGcjDOL5UlsuusFncCzWBQ7RKNUSesmQRMSGkVb1/
3j+skZ6UtW+5u09lHNsj6tQ51s1SPrCBkedbNf0Tp0GbMJDyR4e9T04ZZwIDAQAB
-----END PUBLIC KEY-----`

const store = new jose.JWKS.KeyStore

store.add(jose.JWK.asKey(privateKey))
store.add(jose.JWK.asKey(publicKey))

console.log(store.toJWKS(true))

Expected behaviour
It should be stringified with one private and one public key (preferable)
OR
Key Store should not allow adding both private and public key at the same time

Environment:

  • @panva/jose version: [1.9.1]
  • node version: [e.g. v12.10.0]

Additional context
Add any other context about the problem here.

  • the bug is happening on latest @panva/jose too.
  • i have searched the issues tracker on github for similar issues and couldn't find anything related.
@Alexsey Alexsey added the bug Something isn't working label Sep 15, 2019
@panva
Copy link
Owner

panva commented Sep 15, 2019

That’s expected behaviour and not a bug. Only private keys can be exported this way. Encountering a public one is indicative of a mixed keystore and its safer not to export anything.

@panva panva closed this as completed Sep 15, 2019
@panva panva added wontfix This will not be worked on and removed bug Something isn't working labels Sep 15, 2019
@Alexsey
Copy link
Contributor Author

Alexsey commented Sep 15, 2019

If mixed keystores are ok then - there should be no problem with exposing them. If mixed keystores are not ok - the error should be raised as early as possible - at the moment when the keystore is becoming mixed

The current behavior is problematic because it means that if I'm trying to do .toJWKS(true) on a keystore I must always wrap it in try/catch because it may throw because it has been corrupted and the error gives no idea when did that happen. This is not a good design

On the other hand, how exposing a public key can be unsafe?

EDIT: Adding of a public key to a keystore is preventing from obtaining its private keys. So the element of a collection affects the whole collection. This is not a behavior that you would expect from any kind of collection

@panva
Copy link
Owner

panva commented Sep 16, 2019

@Alexsey I hear you, v1.9.2 addresses this.

@Alexsey
Copy link
Contributor Author

Alexsey commented Sep 16, 2019

Cool, thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants