[util] clear OpenSSL error queue on jwt-cpp activity
As it turns out, jwt-cpp doesn't clean up OpenSSL error queue in case
of errors. While I think it's a good idea to correct that improper
OpenSSL API usage in the jwt-cpp library itself (and I'm going to work
on a patch for jwt-cpp library upstream), I updated Kudu's jwt-util
code to report on those errors and clean the error queue as an interim
solution.
I haven't added any tests, but a few tests in a follow-up patch would
fail without these changes.
This is a follow-up to 5a3d116f302bde07e86bf80c237f8a595d5003b4.
Change-Id: I1a18a0d78bca0ea32878dc3825523e1bd6ecb019
Reviewed-on: http://gerrit.cloudera.org:8080/19895
Tested-by: Alexey Serbin <alexey@apache.org>
Reviewed-by: Wenzhe Zhou <wzhou@cloudera.com>
Reviewed-by: Zoltan Chovan <zchovan@cloudera.com>
Reviewed-by: Attila Bukor <abukor@apache.org>
diff --git a/python/kudu/tests/test_client.py b/python/kudu/tests/test_client.py
index 02da29d..3978dc1 100755
--- a/python/kudu/tests/test_client.py
+++ b/python/kudu/tests/test_client.py
@@ -901,7 +901,7 @@
require_authentication=True, jwt=jwt)
jwt = self.get_jwt(valid=False)
- error_msg = ('FATAL_INVALID_JWT: Not authorized: Verification failed, error: ' +
+ error_msg = ('FATAL_INVALID_JWT: Not authorized: JWT verification failed: ' +
'failed to verify signature: VerifyFinal failed')
with self.assertRaisesRegex(kudu.KuduBadStatus, error_msg):
client = kudu.connect(self.master_hosts, self.master_ports,
diff --git a/src/kudu/util/jwt-util-test.cc b/src/kudu/util/jwt-util-test.cc
index b7d181c..892743c 100644
--- a/src/kudu/util/jwt-util-test.cc
+++ b/src/kudu/util/jwt-util-test.cc
@@ -699,11 +699,9 @@
status = JWTHelper::Decode(token, decoded_token);
EXPECT_OK(status);
status = jwt_helper.Verify(decoded_token.get());
- ASSERT_FALSE(status.ok());
- ASSERT_TRUE(status.ToString().find(
- "JWT algorithm 'rs512' is not matching with JWK algorithm 'rs256'")
- != std::string::npos)
- << " Actual error: " << status.ToString();
+ ASSERT_TRUE(status.IsNotAuthorized()) << status.ToString();
+ ASSERT_STR_CONTAINS(status.ToString(),
+ "JWT algorithm 'rs512' is not matching with JWK algorithm 'rs256'");
}
TEST(JwtUtilTest, VerifyJwtFailKeyNotFound) {
@@ -728,10 +726,8 @@
status = JWTHelper::Decode(token, decoded_token);
EXPECT_OK(status);
status = jwt_helper.Verify(decoded_token.get());
- ASSERT_FALSE(status.ok());
- ASSERT_TRUE(
- status.ToString().find("Invalid JWK ID in the JWT token") != std::string::npos)
- << " Actual error: " << status.ToString();
+ ASSERT_TRUE(status.IsNotAuthorized()) << status.ToString();
+ ASSERT_STR_CONTAINS(status.ToString(), "Invalid JWK ID in the JWT token");
}
TEST(JwtUtilTest, VerifyJwtTokenWithoutKeyId) {
@@ -798,9 +794,8 @@
status = JWTHelper::Decode(token, decoded_token);
EXPECT_OK(status);
status = jwt_helper.Verify(decoded_token.get());
- ASSERT_FALSE(status.ok());
- ASSERT_TRUE(status.ToString().find("Unsecured JWT") != std::string::npos)
- << " Actual error: " << status.ToString();
+ ASSERT_TRUE(status.IsNotAuthorized()) << status.ToString();
+ ASSERT_STR_CONTAINS(status.ToString(), "Unsecured JWT");
}
TEST(JwtUtilTest, VerifyJwtFailExpiredToken) {
@@ -829,10 +824,8 @@
status = JWTHelper::Decode(token, decoded_token);
EXPECT_OK(status);
status = jwt_helper.Verify(decoded_token.get());
- ASSERT_FALSE(status.ok());
- ASSERT_TRUE(status.ToString().find("Verification failed, error: token expired")
- != std::string::npos)
- << " Actual error: " << status.ToString();
+ ASSERT_TRUE(status.IsNotAuthorized()) << status.ToString();
+ ASSERT_STR_CONTAINS(status.ToString(), "JWT verification failed: token expired");
}
namespace {
diff --git a/src/kudu/util/jwt-util.cc b/src/kudu/util/jwt-util.cc
index 209d01c..b391d5e 100644
--- a/src/kudu/util/jwt-util.cc
+++ b/src/kudu/util/jwt-util.cc
@@ -290,12 +290,25 @@
// EXTRACT_VALUE(Bool, bool)
};
+namespace {
+
+// Utility function to handle exceptions from the jwt-cpp library.
+Status HandleEx(const char* const msg, const std::exception& e) {
+ const auto err = GetOpenSSLErrors();
+ return Status::NotAuthorized(err.empty()
+ ? Substitute("$0: $1", msg, e.what())
+ : Substitute("$0: $1 ($2)", msg, e.what(), err));
+}
+
+} // anonymous namespace
+
//
// JWTPublicKey member functions.
//
// Verify JWT's signature for the given decoded token with jwt-cpp API.
Status JWTPublicKey::Verify(
const DecodedJWT& decoded_jwt, const std::string& algorithm) const {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
// Verify if algorithms are matching.
if (algorithm_ != algorithm) {
return Status::NotAuthorized(
@@ -306,14 +319,8 @@
try {
// Call jwt-cpp API to verify token's signature.
verifier_.verify(decoded_jwt);
- } catch (const jwt::error::rsa_exception& e) {
- return Status::NotAuthorized(Substitute("RSA error: $0", e.what()));
- } catch (const jwt::error::token_verification_exception& e) {
- return Status::NotAuthorized(
- Substitute("Verification failed, error: $0", e.what()));
} catch (const std::exception& e) {
- return Status::NotAuthorized(
- Substitute("Verification failed, error: $0", e.what()));
+ return HandleEx("JWT verification failed", e);
}
return Status::OK();
}
@@ -321,6 +328,7 @@
// Create a JWKPublicKey of HS from the JWK.
Status HSJWTPublicKeyBuilder::CreateJWKPublicKey(
const JsonKVMap& kv_map, unique_ptr<JWTPublicKey>* pub_key_out) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
// Octet Sequence keys for HS256, HS384 or HS512.
// JWK Sample:
// {
@@ -355,8 +363,7 @@
return Status::InvalidArgument(Substitute("Invalid 'alg' property value: '$0'", algorithm));
}
} catch (const std::exception& e) {
- return Status::NotAuthorized(
- Substitute("Failed to initialize verifier, error: $0", e.what()));
+ return HandleEx("failed to initialize verifier", e);
}
*pub_key_out = std::move(jwt_pub_key);
return Status::OK();
@@ -365,6 +372,7 @@
// Create a JWKPublicKey of RSA from the JWK.
Status RSAJWTPublicKeyBuilder::CreateJWKPublicKey(
const JsonKVMap& kv_map, unique_ptr<JWTPublicKey>* pub_key_out) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
// JWK Sample:
// {
// "kty":"RSA",
@@ -411,11 +419,8 @@
} else {
return Status::InvalidArgument(Substitute("Invalid 'alg' property value: '$0'", algorithm));
}
- } catch (const jwt::error::rsa_exception& e) {
- return Status::NotAuthorized(Substitute("RSA error: $0", e.what()));
} catch (const std::exception& e) {
- return Status::NotAuthorized(
- Substitute("Failed to initialize verifier, error: $0", e.what()));
+ return HandleEx("failed to initialize verifier", e);
}
*pub_key_out = std::move(jwt_pub_key);
@@ -425,6 +430,7 @@
// Convert JWK's RSA public key to PEM format using OpenSSL API.
Status RSAJWTPublicKeyBuilder::ConvertJwkToPem(
const string& base64_n, const string& base64_e, string& pub_key) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
string str_n;
if (!WebSafeBase64Unescape(base64_n, &str_n)) {
return Status::InvalidArgument("malformed 'n' key component");
@@ -456,6 +462,7 @@
// Create a JWKPublicKey of EC (ES256, ES384 or ES512) from the JWK.
Status ECJWTPublicKeyBuilder::CreateJWKPublicKey(
const JsonKVMap& kv_map, unique_ptr<JWTPublicKey>* pub_key_out) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
// JWK Sample:
// {
// "kty":"EC",
@@ -529,11 +536,8 @@
DCHECK(algorithm == "es512");
jwt_pub_key = new ES512JWTPublicKey(algorithm, pub_key);
}
- } catch (const jwt::error::ecdsa_exception& e) {
- return Status::NotAuthorized(Substitute("EC error: $0", e.what()));
} catch (const std::exception& e) {
- return Status::NotAuthorized(
- Substitute("Failed to initialize verifier, error: $0", e.what()));
+ return HandleEx("failed to initialize verifier", e);
}
pub_key_out->reset(jwt_pub_key);
@@ -545,6 +549,7 @@
const string& base64_x,
const string& base64_y,
string& pub_key) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
string ascii_x;
if (!WebSafeBase64Unescape(base64_x, &ascii_x)) {
return Status::InvalidArgument("malformed 'x' key component");
@@ -826,6 +831,7 @@
// Decode the given JWT token.
Status JWTHelper::Decode(const string& token, UniqueJWTDecodedToken& decoded_token_out) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
try {
// Call jwt-cpp API to decode the JWT token with default jwt::json_traits
// (jwt::picojson_traits).
@@ -843,17 +849,18 @@
VLOG(3) << msg.str();
#endif
} catch (const std::invalid_argument& e) {
- return Status::NotAuthorized(
- Substitute("Token is not in correct format, error: $0", e.what()));
+ return HandleEx("token is not in correct format", e);
} catch (const std::runtime_error& e) {
- return Status::NotAuthorized(
- Substitute("Base64 decoding failed or invalid json, error: $0", e.what()));
+ return HandleEx("base64 decoding failed or invalid JSON", e);
+ } catch (const std::exception& e) {
+ return HandleEx("unexpected error while decoding JWT", e);
}
return Status::OK();
}
// Validate the token's signature with public key.
Status JWTHelper::Verify(const JWTDecodedToken* decoded_token) const {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
DCHECK(initialized_);
const auto& decoded_jwt = decoded_token->decoded_jwt_;
@@ -923,20 +930,18 @@
return status;
}
} catch (const std::bad_cast& e) {
- return Status::NotAuthorized(
- Substitute("Claim was present but not a string, error: $0", e.what()));
+ return HandleEx("claim was present but not a string", e);
} catch (const jwt::error::claim_not_present_exception& e) {
- return Status::NotAuthorized(
- Substitute("Claim not present in JWT token, error $0", e.what()));
+ return HandleEx("claim not present in JWT token", e);
} catch (const std::exception& e) {
- return Status::NotAuthorized(
- Substitute("Token verification failed, error: $0", e.what()));
+ return HandleEx("token verification failed", e);
}
return Status::OK();
}
Status JWTHelper::GetCustomClaimUsername(const JWTDecodedToken* decoded_token,
const string& jwt_custom_claim_username, string& username) {
+ SCOPED_OPENSSL_NO_PENDING_ERRORS;
DCHECK(!jwt_custom_claim_username.empty());
const auto& decoded_jwt = decoded_token->decoded_jwt_;
try {
@@ -952,12 +957,13 @@
return Status::NotAuthorized(
Substitute("Claim '$0' is empty", jwt_custom_claim_username));
}
-
} catch (const std::runtime_error& e) {
- return Status::NotAuthorized(
- Substitute("Claim '$0' was not present, error: $1", jwt_custom_claim_username,
- e.what()));
+ return HandleEx(Substitute("claim '$0' was not present",
+ jwt_custom_claim_username).c_str(), e);
+ } catch (const std::exception& e) {
+ return HandleEx("unexpected error while processing JWT claim", e);
}
+
return Status::OK();
}
