Skip to content

Conversation

@bbockelm
Copy link
Contributor

This commit adds some bugfixes required for supporting IAM; there's nothing specific to that particular issuer, but rather some technical issues with scitokens-cpp:

  1. Incorrect handling of the padding in the base64 pieces of the public keys.
  2. Do not require the (optional) alg claim in the public key; instead, try to bootstrap this from the (required) kty if alg isn't available.

The contents of the public key JSON are base64 encoded using
the standard JWT rules.

This fixes the fact we did not internal pad the values inside
the public keys, causing base64 decoding failures for specific
issuers (such as our test IAM endpoint).  This prevented us
from verifying any SciToken from endpoints using the affected
public keys.
It's not mandatory to have an `alg` claim in the JWKS; we can
derive the information we need from the `kty` (key type) claim.
The java library used by IAM does not advertise `alg`.

There's potentially more work to do here if we want to support
additional RSA signing algorithms.
Copy link
Contributor

@djw8605 djw8605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar issue with the incorrectly padded base64. I interpreted it as incorrect base64, and didn't make a change to the scitokens code. Though, I'm fine with this as well.

@bbockelm bbockelm merged commit e25fc51 into scitokens:master Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants