Fix bugs and add token timeout-in-cache test

This commit is contained in:
Junhyun Shim 2022-07-11 16:20:58 +02:00
parent 7f5049239d
commit 545a9a8043
7 changed files with 84 additions and 24 deletions

View File

@ -115,6 +115,9 @@ std::pair<StringRef, bool> decode(Arena& arena, StringRef base64UrlStr) {
}
auto out = new (arena) uint8_t[decodedLen];
auto actualLen = decode(base64UrlStr.begin(), base64UrlStr.size(), out);
if (actualLen == -1) {
return { StringRef(), false };
}
ASSERT_EQ(decodedLen, actualLen);
return { StringRef(out, decodedLen), true };
}

View File

@ -342,7 +342,7 @@ public:
Future<Void> multiVersionCleanup;
Future<Void> pingLogger;
std::unordered_map<Standalone<StringRef>, Standalone<StringRef>> publicKeys;
std::unordered_map<Standalone<StringRef>, PublicKey> publicKeys;
};
ACTOR Future<Void> pingLatencyLogger(TransportData* self) {
@ -1560,9 +1560,7 @@ FlowTransport::FlowTransport(uint64_t transportId, int maxWellKnownEndpoints, IP
self->multiVersionCleanup = multiVersionCleanupWorker(self);
if (g_network->isSimulated()) {
for (auto const& p : g_simulator.authKeys) {
Standalone<StringRef> pk;
pk = p.second.toPublicKey().writeDer(pk.arena());
self->publicKeys.emplace(p.first, pk);
self->publicKeys.emplace(p.first, p.second.toPublicKey());
}
}
}
@ -1939,12 +1937,12 @@ HealthMonitor* FlowTransport::healthMonitor() {
return &self->healthMonitor;
}
Optional<StringRef> FlowTransport::getPublicKeyByName(StringRef name) const {
Optional<PublicKey> FlowTransport::getPublicKeyByName(StringRef name) const {
auto iter = self->publicKeys.find(name);
if (iter != self->publicKeys.end()) {
return iter->second;
}
return Optional<StringRef>{};
return {};
}
NetworkAddress FlowTransport::currentDeliveryPeerAddress() const {
@ -1955,11 +1953,14 @@ bool FlowTransport::currentDeliveryPeerIsTrusted() const {
return g_currentDeliverPeerAddressTrusted;
}
void FlowTransport::addPublicKey(StringRef name, StringRef key) {
Standalone<StringRef>& pk = self->publicKeys[name];
pk = StringRef(pk.arena(), key);
void FlowTransport::addPublicKey(StringRef name, PublicKey key) {
self->publicKeys[name] = key;
}
void FlowTransport::removePublicKey(StringRef name) {
self->publicKeys.erase(name);
}
void FlowTransport::removeAllPublicKeys() {
self->publicKeys.clear();
}

View File

@ -2,6 +2,7 @@
#include "fdbrpc/TokenCache.h"
#include "fdbrpc/TokenSign.h"
#include "flow/MkCert.h"
#include "flow/ScopeExit.h"
#include "flow/UnitTest.h"
#include "flow/network.h"
@ -200,14 +201,18 @@ bool TokenCacheImpl::validateAndAdd(double currentTime,
for (auto tenant : t.tenants.get()) {
c.tenants.push_back_deep(c.arena, tenant);
}
StringRef signature(c.arena, signature);
cache.insert(signature, c);
StringRef sig(c.arena, signature);
cache.insert(sig, c);
return true;
}
}
bool TokenCacheImpl::validate(TenantNameRef name, StringRef token) {
auto sig = authz::jwt::signaturePart(token);
if (sig.empty()) {
TEST(true); // Token is ill-formed
return false;
}
auto cachedEntry = cache.get(sig);
double currentTime = g_network->timer();
NetworkAddress peer = FlowTransport::transport().currentDeliveryPeerAddress();
@ -225,7 +230,7 @@ bool TokenCacheImpl::validate(TenantNameRef name, StringRef token) {
auto& entry = cachedEntry.get();
if (entry->expirationTime < currentTime) {
TEST(true); // Read expired token from cache
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "Expired");
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "ExpiredInCache");
return false;
}
bool tenantFound = false;
@ -235,7 +240,7 @@ bool TokenCacheImpl::validate(TenantNameRef name, StringRef token) {
break;
}
}
if (tenantFound) {
if (!tenantFound) {
TEST(true); // Valid token doesn't reference tenant
TraceEvent(SevWarn, "TenantTokenMismatch").detail("From", peer).detail("Tenant", name.toString());
return false;
@ -247,8 +252,16 @@ namespace authz::jwt {
extern TokenRef makeRandomTokenSpec(Arena&, IRandom&, authz::Algorithm);
}
TEST_CASE("/fdbrpc/authz/TokenCache") {
TEST_CASE("/fdbrpc/authz/TokenCache/BadTokens") {
std::pair<void (*)(Arena&, IRandom&, authz::jwt::TokenRef&), char const*> badMutations[]{
{
[](Arena&, IRandom&, authz::jwt::TokenRef&) { FlowTransport::transport().removeAllPublicKeys(); },
"NoKeyWithSuchName",
},
{
[](Arena&, IRandom&, authz::jwt::TokenRef& token) { token.keyId.reset(); },
"NoKeyId",
},
{
[](Arena&, IRandom&, authz::jwt::TokenRef& token) { token.expiresAtUnixTime.reset(); },
"NoExpirationTime",
@ -278,13 +291,18 @@ TEST_CASE("/fdbrpc/authz/TokenCache") {
"NoTenants",
},
};
auto const pubKeyName = "somePublicKey"_sr;
auto privateKey = mkcert::makeEcP256();
auto const numBadMutations = sizeof(badMutations) / sizeof(badMutations[0]);
for (auto repeat = 0; repeat < 50; repeat++) {
auto arena = Arena();
auto privateKey = mkcert::makeEcP256();
auto& rng = *deterministicRandom();
auto validTokenSpec = authz::jwt::makeRandomTokenSpec(arena, rng, authz::Algorithm::ES256);
validTokenSpec.keyId = pubKeyName;
for (auto i = 0; i < numBadMutations; i++) {
FlowTransport::transport().addPublicKey(pubKeyName, privateKey.toPublicKey());
auto publicKeyClearGuard =
ScopeExit([pubKeyName]() { FlowTransport::transport().removePublicKey(pubKeyName); });
auto [mutationFn, mutationDesc] = badMutations[i];
auto tmpArena = Arena();
auto mutatedTokenSpec = validTokenSpec;
@ -298,6 +316,48 @@ TEST_CASE("/fdbrpc/authz/TokenCache") {
}
}
}
if (TokenCache::instance().validate("TenantNameDontMatterHere"_sr, StringRef())) {
fmt::print("Unexpected successful validation of ill-formed token (no signature part)\n");
ASSERT(false);
}
if (TokenCache::instance().validate("TenantNameDontMatterHere"_sr, "1111.22"_sr)) {
fmt::print("Unexpected successful validation of ill-formed token (no signature part)\n");
ASSERT(false);
}
if (TokenCache::instance().validate("TenantNameDontMatterHere2"_sr, "////.////.////"_sr)) {
fmt::print("Unexpected successful validation of unparseable token\n");
ASSERT(false);
}
fmt::print("TEST OK\n");
return Void();
}
TEST_CASE("/fdbrpc/authz/TokenCache/GoodTokens") {
// Don't repeat because token expiry is at seconds granularity and sleeps are costly in unit tests
auto arena = Arena();
auto privateKey = mkcert::makeEcP256();
auto const pubKeyName = "somePublicKey"_sr;
FlowTransport::transport().addPublicKey(pubKeyName, privateKey.toPublicKey());
auto publicKeyClearGuard = ScopeExit([pubKeyName]() { FlowTransport::transport().removePublicKey(pubKeyName); });
auto& rng = *deterministicRandom();
auto tokenSpec = authz::jwt::makeRandomTokenSpec(arena, rng, authz::Algorithm::ES256);
tokenSpec.expiresAtUnixTime = static_cast<uint64_t>(g_network->timer() + 2.0);
tokenSpec.keyId = pubKeyName;
auto signedToken = authz::jwt::signToken(arena, tokenSpec, privateKey);
if (!TokenCache::instance().validate(tokenSpec.tenants.get()[0], signedToken)) {
fmt::print("Unexpected failed token validation, token spec: {}, now: {}\n",
tokenSpec.toStringRef(arena).toStringView(),
g_network->timer());
ASSERT(false);
}
threadSleep(3.5);
if (TokenCache::instance().validate(tokenSpec.tenants.get()[0], signedToken)) {
fmt::print(
"Unexpected successful token validation after supposedly expiring in cache, token spec: {}, now: {}\n",
tokenSpec.toStringRef(arena).toStringView(),
g_network->timer());
ASSERT(false);
}
fmt::print("TEST OK\n");
return Void();
}

View File

@ -489,10 +489,6 @@ bool verifyToken(StringRef signedToken, PublicKey publicKey) {
return verifyStringSignature(b64urlTokenPart, sig, publicKey, keyAlgNid, digest);
}
bool verifyToken(StringRef signedToken, StringRef publicKeyDer) {
return verifyToken(signedToken, PublicKey(DerEncoded{}, publicKeyDer));
}
TokenRef makeRandomTokenSpec(Arena& arena, IRandom& rng, Algorithm alg) {
if (alg != Algorithm::ES256) {
throw unsupported_operation();

View File

@ -292,10 +292,11 @@ public:
bool currentDeliveryPeerIsTrusted() const;
NetworkAddress currentDeliveryPeerAddress() const;
Optional<StringRef> getPublicKeyByName(StringRef name) const;
Optional<PublicKey> getPublicKeyByName(StringRef name) const;
// Adds or replaces a public key
void addPublicKey(StringRef name, StringRef key);
void addPublicKey(StringRef name, PublicKey key);
void removePublicKey(StringRef name);
void removeAllPublicKeys();
private:
class TransportData* self;

View File

@ -132,7 +132,7 @@ StringRef signaturePart(StringRef token);
bool parseToken(Arena& arena, TokenRef& tokenOut, StringRef signedTokenIn);
// Verify only the signature part of signed token string against its token part, not its content
bool verifyToken(StringRef signedToken, StringRef publicKeyDer);
bool verifyToken(StringRef signedToken, PublicKey publicKey);
} // namespace authz::jwt

View File

@ -582,8 +582,7 @@ ACTOR Future<ISimulator::KillType> simulatedFDBDRebooter(Reference<IClusterConne
WLTOKEN_RESERVED_COUNT,
&allowList);
for (const auto& p : g_simulator.authKeys) {
Arena tmp;
FlowTransport::transport().addPublicKey(p.first, p.second.toPublicKey().writeDer(tmp));
FlowTransport::transport().addPublicKey(p.first, p.second.toPublicKey());
}
Sim2FileSystem::newFileSystem();