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

support quick seach jwk by kid #64

Open
wbpcode opened this issue Feb 23, 2022 · 4 comments
Open

support quick seach jwk by kid #64

wbpcode opened this issue Feb 23, 2022 · 4 comments

Comments

@wbpcode
Copy link

wbpcode commented Feb 23, 2022

Now we will try to iterate all the jwks to find a matched jwk and then verify it. Can we create a hash table based on the kid? And if kid of jwt is not empty, we can try a quick search first before the iteration?

Status verifyJwtWithoutTimeChecking(const Jwt& jwt, const Jwks& jwks) {
  // Verify signature
  std::string signed_data =
      jwt.header_str_base64url_ + '.' + jwt.payload_str_base64url_;[Wayne Zhang, 4 years ago: • Initial code](https://sourcegraph.com/github.com/google/jwt_verify_lib/-/commit/d856e7692ce470a961ac15d635570ac5379036f3)
  bool kid_alg_matched = false;
  for (const auto& jwk : jwks.keys()) {
    // If kid is specified in JWT, JWK with the same kid is used for
    // verification.
    // If kid is not specified in JWT, try all JWK.
    if (!jwt.kid_.empty() && !jwk->kid_.empty() && jwk->kid_ != jwt.kid_) {
      continue;
    }

    // The same alg must be used.
    if (!jwk->alg_.empty() && jwk->alg_ != jwt.alg_) {
      continue;
    }
    kid_alg_matched = true;

    if (jwk->kty_ == "EC") {
      const EVP_MD* md;
      if (jwt.alg_ == "ES384") {
        md = EVP_sha384();
      } else if (jwt.alg_ == "ES512") {
        md = EVP_sha512();
      } else {
        // default to SHA256
        md = EVP_sha256();
      }

      if (verifySignatureEC(jwk->ec_key_.get(), md, jwt.signature_,
                            signed_data)) {
        // Verification succeeded.
        return Status::Ok;
      }
    } else if (jwk->kty_ == "RSA") {
      const EVP_MD* md;
      if (jwt.alg_ == "RS384" || jwt.alg_ == "PS384") {
        md = EVP_sha384();
      } else if (jwt.alg_ == "RS512" || jwt.alg_ == "PS512") {
        md = EVP_sha512();
      } else {
        // default to SHA256
        md = EVP_sha256();
      }

      if (jwt.alg_.compare(0, 2, "RS") == 0) {
        if (verifySignatureRSA(jwk->rsa_.get(), md, jwt.signature_,
                               signed_data)) {
          // Verification succeeded.
          return Status::Ok;
        }
      } else if (jwt.alg_.compare(0, 2, "PS") == 0) {
        if (verifySignatureRSAPSS(jwk->rsa_.get(), md, jwt.signature_,
                                  signed_data)) {
          // Verification succeeded.
          return Status::Ok;
        }
      }
    } else if (jwk->kty_ == "oct") {
      const EVP_MD* md;
      if (jwt.alg_ == "HS384") {
        md = EVP_sha384();
      } else if (jwt.alg_ == "HS512") {
        md = EVP_sha512();
      } else {
        // default to SHA256
        md = EVP_sha256();
      }

      if (verifySignatureOct(jwk->hmac_key_, md, jwt.signature_, signed_data)) {
        // Verification succeeded.
        return Status::Ok;
      }
    } else if (jwk->kty_ == "OKP" && jwk->crv_ == "Ed25519") {
      Status status = verifySignatureEd25519(jwk->okp_key_raw_, jwt.signature_,
                                             signed_data);
      // For verification failures keep going and try the rest of the keys in
      // the JWKS. Otherwise status is either OK or an error with the JWT and we
      // can return immediately.
      if (status == Status::Ok ||
          status == Status::JwtEd25519SignatureWrongLength) {
        return status;
      }
    }
  }

  // Verification failed.
  return kid_alg_matched ? Status::JwtVerificationFail
                         : Status::JwksKidAlgMismatch;
}
@wbpcode
Copy link
Author

wbpcode commented Feb 23, 2022

If the community thinks the work is worthwhile, I can give it a try.

@qiwzhang
Copy link
Contributor

In my view, such optimization is not very useful. JWKS is very small with only a few "kid" in general. If you can proof to me that there are many production JWKS with large amount of kids, then please go ahead with such optimization.

@wbpcode
Copy link
Author

wbpcode commented Feb 24, 2022

@qiwzhang
In our new gateway, we may create a jwk for every consumer which result in large amount of kids.
In this way, we can manage consumer authentication in a more granular and flexible way. For example, if a consumer's token is leaked, we can immediately invalidate the token by disabling the corresponding jwk, rather than waiting until the token expires itself.

And I think this optimisation should be very simple.

@qiwzhang
Copy link
Contributor

I see. It makes sense. Please go ahead.

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

No branches or pull requests

2 participants