From 94f1e209d9af339f9dfc7e17abe922340a3ef0a1 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 11 Sep 2019 09:49:38 -0500 Subject: [PATCH 1/2] Fix handling of base64 padding. 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. --- src/scitokens_internal.cpp | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index b90a301..b33c084 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -189,10 +189,28 @@ struct local_base64url : public jwt::alphabet::base64url { }; +// Assuming a padding, decode +std::string b64url_decode_nopadding(const std::string &input) +{ + std::string result = input; + switch (result.size() % 4) { + case 1: + result += "="; // fallthrough + case 2: + result += "="; // fallthrough + case 3: + result += "="; // fallthrough + default: + break; + } + return jwt::base::decode(result); +} + + std::string es256_from_coords(const std::string &x_str, const std::string &y_str) { - auto x_decode = jwt::base::decode(x_str); - auto y_decode = jwt::base::decode(y_str); + auto x_decode = b64url_decode_nopadding(x_str); + auto y_decode = b64url_decode_nopadding(y_str); std::unique_ptr ec(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1), EC_KEY_free); if (!ec.get()) { @@ -232,8 +250,8 @@ es256_from_coords(const std::string &x_str, const std::string &y_str) { std::string rs256_from_coords(const std::string &e_str, const std::string &n_str) { - auto e_decode = jwt::base::decode(e_str); - auto n_decode = jwt::base::decode(n_str); + auto e_decode = b64url_decode_nopadding(e_str); + auto n_decode = b64url_decode_nopadding(n_str); std::unique_ptr e_bignum(BN_bin2bn(reinterpret_cast(e_decode.c_str()), e_decode.size(), nullptr), BN_free); std::unique_ptr n_bignum(BN_bin2bn(reinterpret_cast(n_decode.c_str()), n_decode.size(), nullptr), BN_free); From 6f0b88a61ca3fdd8d606f803b13a0eadeb0fed2f Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 11 Sep 2019 10:10:37 -0500 Subject: [PATCH 2/2] Look at the key type to determine the expected algorithm. 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. --- src/scitokens_internal.cpp | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index b33c084..345165b 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -417,10 +417,33 @@ Validator::get_public_key_pem(const std::string &issuer, const std::string &kid, auto key_obj = find_key_id(keys, kid); auto iter = key_obj.find("alg"); + std::string alg; if (iter == key_obj.end() || (!iter->second.is())) { - throw JsonException("Key is missing algorithm name"); - } - auto alg = iter->second.get(); + auto iter2 = key_obj.find("kty"); + if (iter2 == key_obj.end() || !iter2->second.is()) { + throw JsonException("Key is missing key type"); + } else { + auto kty = iter2->second.get(); + if (kty == "RSA") { + alg = "RS256"; + } else if (kty == "EC") { + auto iter3 = key_obj.find("crv"); + if (iter3 == key_obj.end() || !iter3->second.is()) { + throw JsonException("EC key is missing curve name"); + } + auto crv = iter2->second.get(); + if (crv == "P-256") { + alg = "EC256"; + } else { + throw JsonException("Unsupported EC curve in public key"); + } + } else { + throw JsonException("Unknown public key type"); + } + } + } else { + alg = iter->second.get(); + } if (alg != "RS256" and alg != "ES256") { throw UnsupportedKeyException("Issuer is using an unsupported algorithm"); }