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

Remove reliance on x5c property at /jwks #2716

Closed
patcon opened this issue May 11, 2022 · 2 comments
Closed

Remove reliance on x5c property at /jwks #2716

patcon opened this issue May 11, 2022 · 2 comments
Assignees
Labels
tooling Tooling, automation and CI to support development.

Comments

@patcon
Copy link
Contributor

patcon commented May 11, 2022

The staging /jwks endpoint: https://te-auth.id.tbs-sct.gc.ca/oxauth/restv1/jwks

Success criteria: Finding another way to check token sigs without using 5xc property.

This would allow us to take a step toward replacing our custom auth app (for local development and CI) with one of the more turn-key mock oauth2 server.

I recommend we remove our usage of the poorly supported (as per a few mock oauth server projects) and unadvised (as per Google Engineering and also in-practice in their infra) openid feature (the x5c property).

Benefits

This would allow us to more easily benefit from migrating to a mock oauth2 server (for local development and CI). Migrating to a mock server would allow us to abandon our own auth app, and the special-case auth code paths within api app.

Relevant code

Where we process the x5c property into a config object we use to later check token sigs:

$certificateChain = array_values(data_get($value, 'x5c'))[0];
$config = Configuration::forAsymmetricSigner(
$signer,
InMemory::empty(), // Private key is only used for generating tokens, which is not being done here, therefore empty is used.
InMemory::plainText('-----BEGIN CERTIFICATE-----' . "\n" . $certificateChain . "\n" . '-----END CERTIFICATE-----'),
);

Email from Doug Harris of SiC:

The “x5c” property contains the same public key represented by the “n” and “e” properties, just presented as an X.509 certificate for any software out there that might prefer the older format. It is completely redundant and ignored by most software I’ve worked with.

As long as your software is still checking the signature on our ID tokens the normal way you can ignore the “x5c”.

Added this to icebox for now. cc @tristan-orourke

@patcon patcon added the tooling Tooling, automation and CI to support development. label May 11, 2022
@patcon
Copy link
Contributor Author

patcon commented May 15, 2022

This is noted missing support from the library we're using: lcobucci/jwt#32 (comment)

This is a library suggested for it: https://github.com/acodercat/php-jwk-to-pem

Or this is perhaps better, as a more minimal dep is required, which doesn't pull in phpseclib:
lcobucci/jwt#32 (comment)

@yonikid15
Copy link
Contributor

This is resolved in #2813

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Tooling, automation and CI to support development.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants