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

Internal Server Error: JSON::JWK::Set::KidNotFound #64

Closed
tobiashuste opened this issue Jul 8, 2020 · 11 comments
Closed

Internal Server Error: JSON::JWK::Set::KidNotFound #64

tobiashuste opened this issue Jul 8, 2020 · 11 comments
Labels

Comments

@tobiashuste
Copy link

Currently, we are using GitLab version 13.1.3 which ships the omniauth_openid_connect gem in version 0.3.5. In GitLab 13.1.0 the omniauth_openid_connect package was upgraded from 0.3.3 to 0.3.5. (https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34030)

The previously working OpenID configuration now results in an Internal Server Error during the callback phase. Manually downgrading the package to the version 0.3.3 used in GitLab 13.1 fixes the issue temporarily for us.

Relevent stacktrace

JSON::JWK::Set::KidNotFound: JSON::JWK::Set::KidNotFound
  from json/jose.rb:29:in `with_jwk_support'
  from json/jws.rb:131:in `valid?'
  from json/jws.rb:26:in `verify!'
  from json/jws.rb:195:in `decode_compact_serialized'
  from json/jwt.rb:104:in `decode_compact_serialized'
  from json/jose.rb:63:in `decode'
  from openid_connect/response_object/id_token.rb:70:in `decode'
  from omniauth/strategies/openid_connect.rb:229:in `decode_id_token'
  from omniauth/strategies/openid_connect.rb:343:in `verify_id_token!'
  from omniauth/strategies/openid_connect.rb:223:in `access_token'
  from omniauth/strategies/openid_connect.rb:125:in `callback_phase'
  from omniauth/strategy.rb:238:in `callback_call'
  from omniauth/strategy.rb:189:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from omniauth/strategy.rb:192:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from omniauth/strategy.rb:192:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from omniauth/strategy.rb:192:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from omniauth/strategy.rb:192:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from omniauth/strategy.rb:192:in `call!'
  from omniauth/strategy.rb:169:in `call'
  from gitlab/metrics/elasticsearch_rack_middleware.rb:24:in `call'
  from gitlab/metrics/redis_rack_middleware.rb:22:in `call'
  from gitlab/middleware/rails_queue_duration.rb:29:in `call'
  from gitlab/metrics/rack_middleware.rb:17:in `block in call'
  from gitlab/metrics/transaction.rb:54:in `run'
  from gitlab/metrics/rack_middleware.rb:17:in `call'
  from gitlab/request_profiler/middleware.rb:17:in `call'
  from labkit/middleware/rack.rb:19:in `block in call'
  from labkit/context.rb:32:in `with_context'
  from labkit/middleware/rack.rb:18:in `call'
  from gitlab/middleware/go.rb:20:in `call'
  from gitlab/etag_caching/middleware.rb:13:in `call'
  from batch_loader/middleware.rb:11:in `call'
  from rack/attack.rb:97:in `call'
  from apollo_upload_server/middleware.rb:20:in `call'
  from gitlab/middleware/multipart.rb:125:in `call'
  from rack/attack.rb:111:in `call'
  from warden/manager.rb:36:in `block in call'
  from warden/manager.rb:34:in `catch'
  from warden/manager.rb:34:in `call'
  from rack/cors.rb:98:in `call'
  from rack/tempfile_reaper.rb:15:in `call'
  from rack/etag.rb:25:in `call'
  from rack/conditional_get.rb:25:in `call'
  from rack/head.rb:12:in `call'
  from action_dispatch/http/content_security_policy.rb:18:in `call'
  from gitlab/middleware/read_only/controller.rb:51:in `call'
  from gitlab/middleware/read_only.rb:18:in `call'
  from rack/session/abstract/id.rb:259:in `context'
  from rack/session/abstract/id.rb:253:in `call'
  from action_dispatch/middleware/cookies.rb:648:in `call'
  from gitlab/middleware/same_site_cookies.rb:27:in `call'
  from action_dispatch/middleware/callbacks.rb:27:in `block in call'
  from active_support/callbacks.rb:101:in `run_callbacks'
  from action_dispatch/middleware/callbacks.rb:26:in `call'
  from action_dispatch/middleware/actionable_exceptions.rb:17:in `call'
  from action_dispatch/middleware/debug_exceptions.rb:32:in `call'
  from action_dispatch/middleware/show_exceptions.rb:33:in `call'
  from gitlab/middleware/basic_health_check.rb:25:in `call'
  from rails/rack/logger.rb:37:in `call_app'
  from rails/rack/logger.rb:26:in `block in call'
  from active_support/tagged_logging.rb:80:in `block in tagged'
  from active_support/tagged_logging.rb:28:in `tagged'
  from active_support/tagged_logging.rb:80:in `tagged'
  from rails/rack/logger.rb:26:in `call'
  from action_dispatch/middleware/remote_ip.rb:81:in `call'
  from gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'
  from gitlab/middleware/request_context.rb:23:in `call'
  from request_store/middleware.rb:19:in `call'
  from action_dispatch/middleware/request_id.rb:27:in `call'
  from rack/method_override.rb:22:in `call'
  from rack/runtime.rb:22:in `call'
  from rack/timeout/core.rb:123:in `block in call'
  from rack/timeout/support/timeout.rb:19:in `timeout'
  from rack/timeout/core.rb:122:in `call'
  from /opt/gitlab/embedded/service/gitlab-rails/config/initializers/fix_local_cache_middleware.rb:9:in `call'
  from action_dispatch/middleware/executor.rb:14:in `call'
  from rack/sendfile.rb:111:in `call'
  from action_dispatch/middleware/host_authorization.rb:76:in `call'
  from gitlab/metrics/requests_rack_middleware.rb:60:in `call'
  from raven/integrations/rack.rb:51:in `call'
  from rails/engine.rb:527:in `call'
  from rails/railtie.rb:190:in `public_send'
  from rails/railtie.rb:190:in `method_missing'
  from gitlab/middleware/release_env.rb:12:in `call'
  from rack/urlmap.rb:68:in `block in call'
  from rack/urlmap.rb:53:in `each'
  from rack/urlmap.rb:53:in `call'
  from puma/configuration.rb:228:in `call'
  from puma/server.rb:691:in `handle_request'
  from puma/server.rb:481:in `process_client'
  from puma/server.rb:335:in `block in run'
  from puma/thread_pool.rb:138:in `block in spawn_thread'

OpenID configuration

  {
    'name' => 'openid_connect',
    'label' => 'Helmholtz AAI',
    'icon' => 'helmholtz.png',
    'args' => {
      'name' => 'openid_connect',
      'scope' => ['openid', 'profile', 'email', 'eduperson_principal_name'],
      'response_type' => 'code',
      'issuer' => 'https://login.helmholtz-data-federation.de/oauth2',
      'discovery' => true,
      'client_auth_method' => 'basic',
      'uid_field' => 'eduperson_principal_name',
      'send_scope_to_token_endpoint' => 'true',
      'client_options' => {
        'identifier' => '...',
        'secret' => '...',
        'redirect_uri' => 'https://gitlab.hzdr.de/users/auth/openid_connect/callback'
      }
    }
  }

I also experimented with other configuration options. Setting discovery to false and specifying all information manually. I also tried to set the JSON web set keys manually as well https://github.com/m0n9oose/omniauth_openid_connect/blob/ef2942047c866993d8323115c419371d75f05a60/lib/omniauth/strategies/openid_connect.rb#L270-L273. Unfortunately, none of the measures changed anything.

Possible fixes

I would be very happy if you could have a look into this issue!

For reference, this is the respective GitLab issue: https://gitlab.com/gitlab-org/gitlab/-/issues/225850

@davidbasalla
Copy link

I also ran into this issue today, going to revert to 0.3.3 for now

@TheTesla
Copy link

TheTesla commented Sep 1, 2020

I think I found the problem:

::OpenIDConnect::ResponseObject::IdToken.decode(id_token, public_key)

public_key is an jwt encoded array of dicts, where each dict represents one key containing a key id. id_token is also an array of dicts. There must be the key id kid named in the id_token.

@TheTesla
Copy link

TheTesla commented Sep 7, 2020

#68 solves the problem, but we must reintroduce signature verification again later.

@gcolson
Copy link

gcolson commented Oct 5, 2020

Hi,

I just run into this same issue while using 0.3.3...
Does that makes sense to you ? Could it be not related to this issue ?
I have pretty much the same stacktrace as @tobiashuste

@TheTesla
Copy link

Hi,

I just run into this same issue while using 0.3.3...
Does that makes sense to you ? Could it be not related to this issue ?
I have pretty much the same stacktrace as @tobiashuste

No, this must be something else. Is there an error "KidNotFound"?

@gcolson
Copy link

gcolson commented Nov 16, 2020

yep, same stacktrace as the one posted up there..

@stale
Copy link

stale bot commented Jan 16, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 16, 2021
@stale stale bot closed this as completed Jan 23, 2021
@maxbes
Copy link

maxbes commented Jan 26, 2021

We solved this issue on our Gitlab instance by switching the ID token signature method to RS256 instead of HS256 in our OP configuration.

It is normal for JWT signed with HS* to not have a kid because they are unambiguously signed by the client secret in the context of OpenID Connect. The lack of kid should not be a fatal error.

stanhu added a commit to stanhu/json-jwt that referenced this issue Apr 21, 2021
As defined in https://tools.ietf.org/html/rfc7515#section-4.1.4, the
`kid` (Key ID) parameter is optional and helps to disambiguate multiple
keys.

If there is only a single key and `kid` is not provided, then we can
just return the only key present.

Relates to omniauth/omniauth_openid_connect#64
@stanhu
Copy link
Contributor

stanhu commented Apr 21, 2021

I believe nov/json-jwt#92 is the proper fix for this issue.

stanhu added a commit to stanhu/json-jwt that referenced this issue Apr 21, 2021
As defined in https://tools.ietf.org/html/rfc7515#section-4.1.4, the
`kid` (Key ID) parameter is optional and helps to disambiguate multiple
keys.

If there is only a single key and `kid` is not provided, then we can
just return the only key present.

Relates to omniauth/omniauth_openid_connect#64
stanhu added a commit to stanhu/omniauth-openid-connect that referenced this issue Apr 23, 2021
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If
this happens, we now iterate through each key to find one that works.

Closes omniauth/omniauth_openid_connect#64
@stanhu
Copy link
Contributor

stanhu commented Apr 23, 2021

Got some feedback from the json-jwt maintainer, and this is another attempt at fixing this issue: #91

stanhu added a commit to stanhu/omniauth-openid-connect that referenced this issue Apr 24, 2021
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If a
missing `kid` exception is raised, we now iterate through each key to
find one that works.

Closes omniauth/omniauth_openid_connect#64
stanhu added a commit to stanhu/omniauth-openid-connect that referenced this issue Apr 24, 2021
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If a
missing `kid` exception is raised, we now iterate through each key to
find one that works.

Closes omniauth/omniauth_openid_connect#64
stanhu added a commit to stanhu/omniauth-openid-connect that referenced this issue Apr 24, 2021
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If a
missing `kid` exception is raised, we now iterate through each key to
find one that works.

Closes omniauth/omniauth_openid_connect#64
stanhu added a commit to stanhu/omniauth-openid-connect that referenced this issue Apr 24, 2021
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If a
missing `kid` exception is raised, we now iterate through each key to
find one that works.

Closes omniauth/omniauth_openid_connect#64
hhorikawa pushed a commit to netsphere-labs/omniauth-openid-connect that referenced this issue Jan 24, 2022
When a key ID (`kid`) is not included in the JWT, that means we don't
know anything about which signing key to use. The json-jwt library
expects the `kid` value to be present if a JWK Set is presented.  If a
missing `kid` exception is raised, we now iterate through each key to
find one that works.

Closes omniauth/omniauth_openid_connect#64
@TomNaessens
Copy link

@stanhu Is the fix you provided for this issue in #91 or nov/json-jwt#92 expected to get merged & released? Both maintainers don't seem very willing nor active 😦
Should we switch to the Gitlab fork? In case of the latter, is there any Omniauth 2.x support planned for that fork?

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

Successfully merging a pull request may close this issue.

7 participants