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

validation rule for jwt_secrets.rsa_public_key is inconsistent between the write side and consume side #13710

Open
1 task done
jizhilong opened this issue Sep 25, 2024 · 4 comments · May be fixed by #13717
Open
1 task done

Comments

@jizhilong
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Kong version ($ kong version)

2.2.2+

Current Behavior

No response

Expected Behavior

No response

Steps To Reproduce

No response

Anything else?

  • on the write side, jwt_secrets.rsa_public_key MUST be content of a plain text pem file when asymmetric crypto is used.
    entity_checks = {
      { conditional = { if_field = "algorithm",
                        if_match = {
                          match_any = { patterns = { "^RS256$",
                                                     "^RS384$",
                                                     "^RS512$",
                                                     "^PS256$",
                                                     "^PS384$",
                                                     "^PS512$",
                                                     "^EdDSA$",
                                                     }, },
                        },
                        then_field = "rsa_public_key",
                        then_match = {
                          required = true,
                          custom_validator = validate_ssl_key,
                        },
                      },
  • while on the consume side, jwt_secrets.rsa_public_key MAY get base64 decoded when plugin_conf.secret_is_base64 is switched on, with no regard to whether the crypto used is symmetric or asymmetric
  local jwt_secret_value = algorithm ~= nil and algorithm:sub(1, 2) == "HS" and
                           jwt_secret.secret or jwt_secret.rsa_public_key

  if conf.secret_is_base64 then
    jwt_secret_value = jwt:base64_decode(jwt_secret_value)
  end

In scenarios with algorithm='RS256' and conf.secret_is_base64=true, jwt:base64_decode(jwt_secret_value) will always fail, which lead to issues like #2891 .

We can work around this problem by switching off secret_is_base64, just like @smileMrLee did in #2891 . Or we can make a fix to jwt/handler.lua by check the necessity of base64 decoding with regard to both algorithm and conf.secret_is_base64:

  local is_symmetric_algorithm = algorithm ~= nil and algorithm:sub(1, 2) == "HS"
  local jwt_secret_value =  is_symmetric_algorithm and jwt_secret.secret or jwt_secret.rsa_public_key

  if is_symmetric_algorithm and conf.secret_is_base64 then
    jwt_secret_value = jwt:base64_decode(jwt_secret_value)
  end
@jizhilong jizhilong changed the title validation rule for jwt_secrets.rsa_public_key is inconsistent between the write side and consuming side validation rule for jwt_secrets.rsa_public_key is inconsistent between the write side and consume side Sep 25, 2024
@samugi
Copy link
Member

samugi commented Sep 27, 2024

Hello @jizhilong, thank you for opening this issue. It appears you have a solution coded for this problem, would you consider opening a pull request? Thank you!

jizhilong added a commit to jizhilong/kong that referenced this issue Sep 28, 2024
@jizhilong jizhilong linked a pull request Sep 28, 2024 that will close this issue
2 tasks
@jizhilong
Copy link
Author

@samugi glad to help, the PR is ready at #13717 now

@jizhilong
Copy link
Author

@samugi can you help to review the PR, thanks.

@samugi
Copy link
Member

samugi commented Nov 11, 2024

@jizhilong thank you so much for your contribution! We will review it and provide a feedback soon.

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

Successfully merging a pull request may close this issue.

2 participants